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

v3rpc: Let clients establish unlimited streams #8238

Merged
merged 1 commit into from
Jul 12, 2017

Conversation

yudai
Copy link
Contributor

@yudai yudai commented Jul 10, 2017

Issue Observed

From etcd v3.2.0, custom clients can create up to 100 streams for watch and keepalive. This limitation can cause unexpected behavior to custom clinets/proxies that need to establish many streams.

Issue Detail

etcd v3.2.0 updated the go-grpc library and it had a (breaking) change in it, which caps the default max streams per client to 100 (grpc/grpc-go#1071).

The official client and proxies do not establish many streams for now. However, this limitation can have impact for third party libraries that need to establish more streams for some reason.

Proposal

In this commit, we simply set the number of streams to the max number. We may have configuration property for this as well.

From go-grpc v1.2.0, the number of max streams per client is set to 100
by default by the server side. This change makes it impossible
for third party proxies and custom clients to establish many streams.

Copy link
Contributor

@heyitsanthony heyitsanthony left a comment

Choose a reason for hiding this comment

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

please s/etcdserver/v3rpc in the commit title and this should be good to merge. Thanks!

From go-grpc v1.2.0, the number of max streams per client is set to 100
by default by the server side. This change makes it impossible
for third party proxies and custom clients to establish many streams.
@yudai
Copy link
Contributor Author

yudai commented Jul 11, 2017

Done!

@yudai yudai changed the title etcdserver: Let clients establish unlimited streams v3rpc: Let clients establish unlimited streams Jul 11, 2017
@xiang90
Copy link
Contributor

xiang90 commented Jul 12, 2017

thanks! merging.

@xiang90 xiang90 merged commit 0a2b580 into etcd-io:master Jul 12, 2017
@codecov-io
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (master@1010b82). Click here to learn what that means.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff            @@
##             master   #8238   +/-   ##
========================================
  Coverage          ?   76.3%           
========================================
  Files             ?     346           
  Lines             ?   26994           
  Branches          ?       0           
========================================
  Hits              ?   20598           
  Misses            ?    4914           
  Partials          ?    1482
Impacted Files Coverage Δ
etcdserver/api/v3rpc/grpc.go 100% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1010b82...52101e6. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants