Update node socket registration format #1132
Conversation
ian-shim
force-pushed
the
node-v2-sockets
branch
6 times, most recently
from
d2c1485 to
a2c8896
Compare
ian-shim
requested review from
jianoaix,
cody-littley,
pschork and
mooselumph
|
|
||
| v2DispersalPort := ctx.GlobalString(flags.V2DispersalPortFlag.Name) | ||
| if v2DispersalPort == "" { | ||
| if v2Enabled { |
There was a problem hiding this comment.
making v2 dispersal port required if v2 is enabled
| @@ -218,7 +224,7 @@ var ( | |||
| Required: false, | |||
| EnvVar: common.PrefixEnvVar(EnvVarPrefix, "ENABLE_GNARK_BUNDLE_ENCODING"), | |||
| } | |||
| EnableV2Flag = cli.BoolFlag{ | |||
| EnableV2Flag = cli.BoolTFlag{ | |||
There was a problem hiding this comment.
defaulting this to true
| logger.Fatalf("Could not start tcp listener: %v", err) | ||
| } | ||
|
|
||
| opt := grpc.MaxRecvMsgSize(config.GRPCMsgSizeLimitV2) |
There was a problem hiding this comment.
reducing msg size for v2 dispersal requests
| s = strings.Split(s[0], ":") | ||
| if len(s) != 2 { | ||
| err = fmt.Errorf("invalid socket address format: %s", socket) | ||
| if len(s) == 3 { |
There was a problem hiding this comment.
Then the error throws in L63
ian-shim
force-pushed
the
node-v2-sockets
branch
from
a2c8896 to
077c7d8
Compare
ian-shim
force-pushed
the
node-v2-sockets
branch
from
077c7d8 to
b9635a6
Compare
| DispersalPort string | ||
| RetrievalPort string | ||
| V2DispersalPort string |
There was a problem hiding this comment.
Would it be possible to instead use int types for ports? We just need to change the flag type from string to int to support this. I've made this change in a few places and everything compiles and runs just fine.
There was a problem hiding this comment.
Yes, I think that should have been the way to go, but I'm hesitant to change the flags from string to int because I'm not sure how every operator is configuring the values and if changing the type will be backward compatible
ian-shim
force-pushed
the
node-v2-sockets
branch
from
b9635a6 to
e28b801
Compare
ian-shim
force-pushed
the
node-v2-sockets
branch
from
e28b801 to
29bba0b
Compare
Why are these changes needed?
Decoupling v1 and v2 dispersal ports and integrating v2 port to the existing socket registration format (i.e.
<host>:<dispersalPort>;<retrievalPort>;<v2DispersalPort>)TODO: reachability check for v2 dispersal port
Checks