-
Notifications
You must be signed in to change notification settings - Fork 18k
x/net/http2: no public method to send PING frames #15475
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
Comments
I suggest that x/net/http2 should periodically send PING frames to the peer, and kick muted connections from the connection pool automatically. This feature should be off by default, and could be turned on by setting some kind of options. |
Like @blixt, I'm implementing APNS client and lack of access to PING frame is a bit of a show stopper for pushing to prod environment. It makes sense to have periodical ping as proposed by @zjx20, but it would be great to have also some kind of read-only access to connection stats for both PING and GOAWAY frames (to have connection behaviour in metrics). @bradfitz: As I'm on a bit tight schedule and not sure can wait for the official fix, could You recommend some temporary solution which will allow access to ping on client side? I'm still quite green in GO and can't figure out how to extend the connection without actually forking x/net/http2 and making exported getter for Framer in http2ClientConn. |
PING frames are also useful for measuring the true end-to-end RTT, which you can't reliably measure at the transport level due to split TCP in cellular networks. I don't have a need for this feature, but if it's implemented, I would vote for a blocking API so the caller can time the RTT. Something like: func (s *h2connection) Ping(opaque uint64) (uint64, error) If you need to PING periodically, you could always do so from a timer loop. |
@tombergan, what is the return |
That's right, I missed that the ACK must echo the opaque value from the original PING. The uint64 return value is not necessary. Further, the connection should probably manage opaque values so it can match pings with their ACKs: func (s *h2connection) Ping() error |
Any update on this? I think this ping method should get a context as first argument to let the caller manage deadline and cancellation. Something like: func (cc *ClientConn) Ping(ctx context.Context) error |
Here is an implementation proposal: https://go-review.googlesource.com/#/c/29965/ |
This new method sends a PING frame with random payload to the peer and wait for a PING ack with the same payload. In order to support cancellation and deadling, the Ping method takes a context as argument. Fixes golang/go#15475 Change-Id: I340133a67717af89556837cc531a885d116eba59 Reviewed-on: https://go-review.googlesource.com/29965 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
There is, as far as I can tell, no method for sending a PING frame to an open HTTP/2 connection right now. While in most cases there is no direct need to send PING frames, I've encountered multiple cases where it's important, lest you discard the connection and recreate it:
/ping
in this case.)So my suggestion is to somehow expose a way to send PING frames.
The text was updated successfully, but these errors were encountered: