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

chore: increase grpc max call send & receive msg size #192

Merged
merged 2 commits into from
Jan 24, 2023

Conversation

bsushmith
Copy link
Member

@bsushmith bsushmith commented Dec 14, 2022

  • Increase the grpc max call send & receive msg size to 32MB from default 4MB.

@coveralls
Copy link

coveralls commented Dec 14, 2022

Pull Request Test Coverage Report for Build 3947610556

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 86.06%

Totals Coverage Status
Change from base Build 3936296991: 0.0%
Covered Lines: 4982
Relevant Lines: 5789

💛 - Coveralls

config.grpcAddr(),
grpc.WithTransportCredentials(insecure.NewCredentials()),
grpc.WithDefaultCallOptions(
grpc.MaxCallRecvMsgSize(GRPCMaxClientSendSize),
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to increase RecvMsg as well? All of our insert APIs do not seem to need it.
wdyt?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure about this.
Increased the RecvMsg as well, as asset payload can be huge - but haven't faced this scenario yet.

@@ -30,6 +30,10 @@ import (
"google.golang.org/protobuf/encoding/protojson"
)

const (
GRPCMaxClientSendSize = 32 << 20
Copy link
Contributor

Choose a reason for hiding this comment

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

should we make it configurable as well ? wdyt?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. I think we should be able to configure it also if needed.

Copy link
Member Author

Choose a reason for hiding this comment

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

made both values to be configurable

* add support for user configurability of max call recv and send size in grpc config
@bsushmith bsushmith force-pushed the incr-grpc-max-msg-size branch from 79369e1 to e278504 Compare January 18, 2023 09:45
@ravisuhag ravisuhag merged commit 7fc4a82 into main Jan 24, 2023
@ravisuhag ravisuhag deleted the incr-grpc-max-msg-size branch January 24, 2023 06:48
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.

5 participants