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

grpc-js: Propagate keepalive throttling throughout channel #2363

Conversation

murgatroid99
Copy link
Member

This fixes a bug in the keepalive implementation that did not conform to this part of the keepalive spec:

When a client receives a GOAWAY with error code ENHANCE_YOUR_CALM and debug data equal to ASCII "too_many_pings", it should log the occurrence at a log level that is enabled by default and double the configure KEEPALIVE_TIME used for new connections on that channel.

Currently, it only doubles the keepalive time for the subchannel that received the GOAWAY.

This fix is modeled heavily on grpc/grpc#23313. I made the following changes:

  • The channel now tracks the current keepalive time, including throttling modifications.
  • The subchannel now passes its keepalive time along with every connectivity state update. This mostly won't do anything except right after a connection drops with a "too many pings" error, but it's always there to make the types and logic simpler.
  • When the channel gets a subchannel from the pool, it wraps it before passing it to the LB policy, and tracks all of those wrapped subchannels. When the wrapped subchannel gets a connectivity state update, it updates the channel's keepalive time, and that update is propagated to all of the wrapped subchannels.

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

2 participants