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

Increase grpc max recv message size #2775

Merged
merged 1 commit into from Oct 31, 2018

Conversation

dperny
Copy link
Collaborator

@dperny dperny commented Oct 31, 2018

Skipping the usual boilerplate, because this is a kludge, everyone involved knows its a kludge, and we'll revert it in the next release.

Increases the maximum recieved message size for gRPC client connections to math.MaxInt32. This means that large controlapi List requests will be proxied correctly.

Increases the maximum recieved message size for gRPC client connections
to math.MaxInt32. This means that large controlapi List requests will be
proxied correctly.

Signed-off-by: Drew Erny <drew.erny@docker.com>
@andrewhsu
Copy link
Member

@dperny if this codeline is only accessible behind an api that has the proper creds, then i think the increase of the max message size is ok

@anshulpundir
Copy link
Contributor

Looks like this only affects the connections between the managers. Also, this is reverting back grpc to the same behavior as previous releases. These help mitigate the risk somewhat @andrewhsu

@codecov
Copy link

codecov bot commented Oct 31, 2018

Codecov Report

Merging #2775 into master will increase coverage by 0.02%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##           master   #2775      +/-   ##
=========================================
+ Coverage   61.97%     62%   +0.02%     
=========================================
  Files         137     137              
  Lines       22047   22050       +3     
=========================================
+ Hits        13664   13672       +8     
+ Misses       6912    6906       -6     
- Partials     1471    1472       +1

Copy link
Member

@andrewhsu andrewhsu left a comment

Choose a reason for hiding this comment

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

LGTM

@andrewhsu
Copy link
Member

nitpick: update the comment in the code to something along the lines of #2775 (comment)

@anshulpundir anshulpundir merged commit 8d8689d into moby:master Oct 31, 2018
thaJeztah added a commit to thaJeztah/docker that referenced this pull request Nov 1, 2018
Changes included;

- moby/swarmkit#2735 Assign secrets individually to each task
- moby/swarmkit#2759 Adding a new `Deallocator` component
- moby/swarmkit#2738 Add additional info for secret drivers
- moby/swarmkit#2775 Increase grpc max recv message size
  - addresses moby#37941
  - addresses moby#37997
  - follow-up to moby#38103

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
docker-jenkins pushed a commit to docker/docker-ce that referenced this pull request Nov 12, 2018
Changes included;

- moby/swarmkit#2735 Assign secrets individually to each task
- moby/swarmkit#2759 Adding a new `Deallocator` component
- moby/swarmkit#2738 Add additional info for secret drivers
- moby/swarmkit#2775 Increase grpc max recv message size
  - addresses moby/moby#37941
  - addresses moby/moby#37997
  - follow-up to moby/moby#38103

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Upstream-commit: be3843c8c8fb30b4a604dae9d0dad3d393db717c
Component: engine
adhulipa pushed a commit to adhulipa/docker that referenced this pull request Apr 11, 2019
Changes included;

- moby/swarmkit#2735 Assign secrets individually to each task
- moby/swarmkit#2759 Adding a new `Deallocator` component
- moby/swarmkit#2738 Add additional info for secret drivers
- moby/swarmkit#2775 Increase grpc max recv message size
  - addresses moby#37941
  - addresses moby#37997
  - follow-up to moby#38103

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
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.

None yet

3 participants