allow configuring grpc max receive message size #738
Conversation
jasonwbarnett
force-pushed
the
jwb/add-flag-to-configure-grpc-max-recv-msg-size
branch
5 times, most recently
from
a5a4e79
to
6a4661f
Compare
jasonwbarnett
force-pushed
the
jwb/add-flag-to-configure-grpc-max-recv-msg-size
branch
from
6a4661f
to
920f2cf
Compare
| @@ -393,6 +393,8 @@ func startGrpcServer(c *config.Config, grpcServer **grpc.Server, | |||
|
|
|||
| opts = append(opts, grpc.ChainStreamInterceptor(streamInterceptors...)) | |||
| opts = append(opts, grpc.ChainUnaryInterceptor(unaryInterceptors...)) | |||
| log.Println("Setting gRPC Max Receive Message Size to:", c.GrpcMaxRecvMsgSize) | |||
There was a problem hiding this comment.
Is this line left over from debugging?
There was a problem hiding this comment.
No, this was left intentionally as it is generally helpful to communicate upon startup what the picked up/configured value is. Especially useful if two values are provided, one at the CLI and the other in the config.
|
Seems like Pants is implementing a principled fix in pantsbuild/pants#20708. How do you want to proceeed with this PR @jasonwbarnett ? |
|
@buchgr It's up to you. I don't know much about gRPC and when it is or isn't appropriate to increase the default message send/receive sizes. If there are cases where it's appropriate, I'd like to see this get merged. If there aren't ever scenarios where this is appropriate, it should probably just be closed without a merge. |
|
I think if we decide to land this PR, then the flag needs to have a prominent warning about ensuring that the client side configuration matches for all clients that use the cache. However, if pants only needs this for FindMissingBlobs, then that should be an easy fix on the pants side and we should not add this option to bazel-remote. |
Fixes #658