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: Fix keepalive ping timing after inactivity #2513

Merged

Conversation

murgatroid99
Copy link
Member

This fixes the implementation of the keepalive spec, specifically the following paragraphs:

Since keepalive is not occurring on HTTP/2 connections without any streams, there will be a higher chance of failure for new RPCs following a long period of inactivity. To reduce the tail latency for these RPCs, it is important to not reset the keepalive time when a connection becomes active; if a new stream is created and there has been greater than 'keepalive time' since the last read byte, then a keepalive PING should be sent (ideally before the HEADERS frame). Doing so detects the broken connection with a latency of keepalive timeout instead of keepalive time + timeout.

keepalive time is ideally measured from the time of the last byte read. However, simplistic implementations may choose to measure from the time of the last keepalive PING ack (a.k.a., polling). Such implementations should take extra precautions to avoid issues due to latency added by outbound buffers, such as limiting the outbound buffer size and using a larger keepalive timeout. Implementations must not measure from the last keepalive PING sent, to avoid triggering abuse detection on servers that are configured with settings that exactly match behavior of the client.

@murgatroid99 murgatroid99 merged commit 18dacfa into grpc:@grpc/grpc-js@1.8.x Jul 24, 2023
1 check passed
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.

2 participants