Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dialGRPCConn should default recv/send msg size to max int not max int32 #184

Open
saurik opened this issue Jan 2, 2022 · 0 comments · May be fixed by #185
Open

dialGRPCConn should default recv/send msg size to max int not max int32 #184

saurik opened this issue Jan 2, 2022 · 0 comments · May be fixed by #185

Comments

@saurik
Copy link

saurik commented Jan 2, 2022

In dialGRPCConn there is code that is clearly intended to remove any default call limits on either sending or receiving data over the resulting GRPC client connection, but as it uses math.MaxInt32 despite the value being of type int I've actually run into this limit :(. Inside of the GRPC library itself, they seem to use int(^uint(0) >> 1) in a place to get the maximum possible value... maybe that can be used instead?

go-plugin/grpc_client.go

Lines 36 to 38 in 56f9791

opts = append(opts,
grpc.WithDefaultCallOptions(grpc.MaxCallRecvMsgSize(math.MaxInt32)),
grpc.WithDefaultCallOptions(grpc.MaxCallSendMsgSize(math.MaxInt32)))

ALTERNATIVELY, can GRPCBroker.Dial be given a way to override the grpc.DialOptions? It calls through to dialGRPCConn, but seems to be on of the few places in the library where the grpc.DialOptions are not configurable (whether directly or indirectly). I kind of feel like the intent of this code was actually fine for me, if only it used the correct 64-bit value; but, if there were merely an override, that would be sufficient.

func (b *GRPCBroker) Dial(id uint32) (conn *grpc.ClientConn, err error) {

return dialGRPCConn(b.tls, netAddrDialer(addr))

func dialGRPCConn(tls *tls.Config, dialer func(string, time.Duration) (net.Conn, error), dialOpts ...grpc.DialOption) (*grpc.ClientConn, error) {

(My motivation for this is that there is a 3GB result that comes to me through one of these go-plugin GRPC channels in Avalanche. In case you care, and to provide the reference there to this issue here, that issue is ava-labs/avalanchego#984.)

@StephenButtolph StephenButtolph linked a pull request Jan 3, 2022 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant