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

Allow infinite pings #116

Closed
tamirOK opened this issue Jul 27, 2020 · 6 comments
Closed

Allow infinite pings #116

tamirOK opened this issue Jul 27, 2020 · 6 comments

Comments

@tamirOK
Copy link

tamirOK commented Jul 27, 2020

Hi!

I want my client to hold open connection to server infinitely long by doing pings. To do so, I have to provide correct values for Configuration object. Can you answer my questions please:

  • What is a difference between _keepalive_time and http2_min_sent_ping_interval_without_data settings? I have checked Connection._ping method and these options look very similar to me.
  • How can I remove limit on max number of sent pings?
  • Can I allow ping function to run forever?

Thanks!

@tamirOK
Copy link
Author

tamirOK commented Jul 27, 2020

I have read the definitions of the options here and I think there are couple of problems in the implementation:

  • Connection is always closed after _keepalive_timeout seconds, but it should close only when it didn't received an acknowledgement.
  • GRPC_ARG_HTTP2_MAX_PINGS_WITHOUT_DATA is allowed to be 0. However, I am receiving:
    ValueError: "_http2_max_pings_without_data" should be positive
  • _http2_min_sent_ping_interval_without_data should not affect client side.

@vmagamedov
Copy link
Owner

Connection is always closed after _keepalive_timeout seconds, but it should close only when it didn't received an acknowledgement.

– _keepalive_timeout is cancelled when we receive PING acknowledgment.

GRPC_ARG_HTTP2_MAX_PINGS_WITHOUT_DATA is allowed to be 0

– fixed

_http2_min_sent_ping_interval_without_data should not affect client side.

– please elaborate

@vmagamedov
Copy link
Owner

Please take in mind that non-default keep-alive settings can cause PING strikes as described in https://github.com/grpc/grpc/blob/master/doc/keepalive.md, currently this will cause a cancellation of all in-flight requests (h2 library doesn't handle GOAWAY frame correctly).

@vmagamedov
Copy link
Owner

This issue was closed unintentionally, please reopen it if current keep-alive implementation still doesn't conforms to the reference documentation.

@tamirOK
Copy link
Author

tamirOK commented Jul 27, 2020

Can I control GRPC_ARG_HTTP2_MAX_PING_STRIKES and GRPC_ARG_HTTP2_MIN_RECV_PING_INTERVAL_WITHOUT_DATA_MS for the server? If not, are these values equals to the default values from doc?

@vmagamedov
Copy link
Owner

grpclib currently doesn't implements strikes functionality, so it is important only if you're using grpcio and other official gRPC implementations on the server-side.

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

No branches or pull requests

2 participants