[do not use] Raise gRPC limit to 16 MB #100
Conversation
wild-endeavor
requested review from
anandswaminathan,
EngHabu,
katrogan,
kumare3 and
matthewphsmith
as code owners
cmd/entrypoints/serve.go
Outdated
| @@ -205,7 +205,7 @@ func serveGatewayInsecure(ctx context.Context, cfg *config.ServerConfig) error { | |||
| }() | |||
|
|
|||
| logger.Infof(ctx, "Starting HTTP/1 Gateway server on %s", cfg.GetHostAddress()) | |||
| httpServer, err := newHTTPServer(ctx, cfg, authContext, cfg.GetGrpcHostAddress(), grpc.WithInsecure()) | |||
| httpServer, err := newHTTPServer(ctx, cfg, authContext, cfg.GetGrpcHostAddress(), grpc.WithInsecure(), grpc.WithDefaultCallOptions(grpc.MaxCallRecvMsgSize(16777216))) | |||
There was a problem hiding this comment.
Const variable please :)
There was a problem hiding this comment.
yes yes, but should we do this?
There was a problem hiding this comment.
What's the default?
Does this number mean anything in particular :-D ?
Are you trying to limit it or make it more permissible?
There was a problem hiding this comment.
what happens when a response exceeds the size? does this get logged? does an error get returned?
There was a problem hiding this comment.
Go to the link I'm about to send you on Slack.
There was a problem hiding this comment.
The default size limit for grpc is 4MB, which is where the 4194304 number comes from in the error message (4*2^20). This is just four times that so 16MB (so making it more permissible). wondering if the truncation change is equally quick, in which case we should do that instead? or another workaround that people would rather see?
Codecov Report
@@ Coverage Diff @@
## Lyft-Flyte #100 +/- ##
===========================================
Coverage 63.09% 63.09%
===========================================
Files 100 100
Lines 7061 7061
===========================================
Hits 4455 4455
Misses 2093 2093
Partials 513 513 Continue to review full report at Codecov.
|
wild-endeavor
changed the title
|
Congrats. PR: 100 |
Temporarily raising the gRPC limit to 16MBs. This PR should be reverted after the error truncation fix goes in.