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

GH-2319: Increase the grpc.MaxCallRecvMsgSize #3201

Merged
merged 1 commit into from
Sep 9, 2019

Conversation

stack72
Copy link
Contributor

@stack72 stack72 commented Sep 9, 2019

Fixes: #2319

In #2319, a user is hitting the gRPC limit on the message size the
server can receive when uploading ec2 user-data

This commit doubles the limit that can be sent from 1024*1024*4 to
1024*1024*8

@stack72 stack72 force-pushed the stack72/raise-grpc-max-message-size branch from 771d813 to b8fa891 Compare September 9, 2019 14:47
@stack72 stack72 requested a review from pgavlin September 9, 2019 15:32
@stack72 stack72 self-assigned this Sep 9, 2019
@@ -55,6 +55,9 @@ type plugin struct {
// pluginRPCConnectionTimeout dictates how long we wait for the plugin's RPC to become available.
var pluginRPCConnectionTimeout = time.Second * 10

// pluginRPCMaxMessageSize raises the gRPC Max Message size from `4194304` to `8388608`
var pluginRPCMaxMessageSize = 1024 * 1024 * 8
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why 2x vs say 100x?

Seems 2x won’t make a huge dent in how common it is to hit this.

Also - since we are doing gRPC within a single machine - I assume we are generally less sensitive to issues related to this limit?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was trying to be kinda conservative :D

The userdata limit is 16KB, anything higher than that and AWS will reject it anyway. If you want, I can of course make it 1024*1024*100 - I am happy to do that if you want

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we might as well make this pretty large if there are no adverse effects.

Copy link
Member

@pgavlin pgavlin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it the case that these limits are only enforced server-side?

@@ -55,6 +55,9 @@ type plugin struct {
// pluginRPCConnectionTimeout dictates how long we wait for the plugin's RPC to become available.
var pluginRPCConnectionTimeout = time.Second * 10

// pluginRPCMaxMessageSize raises the gRPC Max Message size from `4194304` to `8388608`
var pluginRPCMaxMessageSize = 1024 * 1024 * 8
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we might as well make this pretty large if there are no adverse effects.

Fixes: #2319

In #2319, a user is hitting the gRPC limit on the message size the
server can receive when uploading ec2 user-data

This commit doubles the limit that can be sent from `1024*1024*4` to
`1024*1024*8`
@stack72 stack72 force-pushed the stack72/raise-grpc-max-message-size branch from b8fa891 to 0c518cf Compare September 9, 2019 18:01
@stack72
Copy link
Contributor Author

stack72 commented Sep 9, 2019

@pgavlin yes it's only enforced ServerSide. I bumped the current value 100 times - that should see us through ;)

Copy link
Member

@pgavlin pgavlin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's hope gRPC never allocates a buffer of MaxCallRecvMsgSize ;)

@stack72 stack72 merged commit 8d1b725 into master Sep 9, 2019
@stack72 stack72 deleted the stack72/raise-grpc-max-message-size branch September 9, 2019 18:31
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 this pull request may close these issues.

Creating server hits gRPC limit
3 participants