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

feat: limit grpc message size #2459

Merged

Conversation

TheWaWaR
Copy link
Contributor

I hereby agree to the terms of the GreptimeDB CLA

What's changed and what's your intention?

Limit grpc client/server message size

Please explain IN DETAIL what the changes are in this PR and why they are needed:

  • Make grpc client max_decoding_message_size/max_encoding_message_size configurable
  • Make grpc server max_decoding_message_size/max_encoding_message_size configurable

Checklist

  • I have written the necessary rustdoc comments.
  • I have added the necessary unit tests and integration tests.

Refer to a related PR or issue link (optional)

#2220

Those options are for:
* Limit receiving(decoding) message size
* Limit sending(enoding) message size
@TheWaWaR
Copy link
Contributor Author

Please note that I don't apply recv/send message limit to meta server, since I think it may not needed. If it's needed please mention it and tell which service need the limitation:

  • HeartbeatServer
  • RouterServer
  • StoreServer
  • ClusterServer
  • LockServer
  • DdlTaskServer
  • Admin

@TheWaWaR TheWaWaR force-pushed the limit-grpc-message-size branch 2 times, most recently from 9fa234b to 4a74c34 Compare September 21, 2023 05:36
@TheWaWaR TheWaWaR force-pushed the limit-grpc-message-size branch from 4a74c34 to 1487d0f Compare September 21, 2023 06:23
@codecov
Copy link

codecov bot commented Sep 21, 2023

Codecov Report

Merging #2459 (1487d0f) into develop (df08771) will decrease coverage by 0.41%.
Report is 5 commits behind head on develop.
The diff coverage is 77.77%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2459      +/-   ##
===========================================
- Coverage    84.85%   84.45%   -0.41%     
===========================================
  Files          727      726       -1     
  Lines       114977   115040      +63     
===========================================
- Hits         97568    97158     -410     
- Misses       17409    17882     +473     

Copy link
Collaborator

@fengjiachun fengjiachun left a comment

Choose a reason for hiding this comment

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

LGTM

@MichaelScofield MichaelScofield added this pull request to the merge queue Sep 22, 2023
Merged via the queue into GreptimeTeam:develop with commit 621c6f3 Sep 22, 2023
13 checks passed
paomian pushed a commit to paomian/greptimedb that referenced this pull request Oct 19, 2023
* feat: add two grpc config options

Those options are for:
* Limit receiving(decoding) message size
* Limit sending(enoding) message size

* test: add integration tests for message size limit
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.

3 participants