-
Notifications
You must be signed in to change notification settings - Fork 307
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
Expose API to send pings #1
Comments
To keep the API simpler and prevent it from becoming a wrapper around the net.Conn, its probably best to let applications implement ping/pongs on their own and have canonical examples of how to do this. |
No idea what I meant by preventing it from becoming a wrapper around net.Conn. |
Given browsers don't implement |
See https://bugs.chromium.org/p/chromium/issues/detail?id=706002 and https://groups.google.com/a/chromium.org/forum/#!topic/net-dev/2RAm-ZYAIYY The consensus has been its not worth the expense and TCP keep alives are enough, which I agree with. |
It also wouldn't really make sense over HTTP/2 |
This isn't quite accurate, browsers do respond to TCP keepalive is not end-to-end and doesn't inform you of the client still being alive, simply that a possible proxy still exists. |
That is true, but I'd argue most WebSocket servers do not send ping/pongs and just use TCP keep alive and things work fine. HTTP/1.1 persistent connections also just use TCP keep alive without major issues. I could be wrong but I believe net/http's HTTP/2 server also does not use HTTP/2 keep alives because TCP keep alives are cheaper and enough. Its not end to end but most in between proxies also use TCP keep alives because they do not want to keep unnecessary connections up as well. The other issue I have with this feature is I don't think I should give it an option because most of the time, TCP keep alives will be enough and it won't make sense over HTTP/2 as HTTP/2 has protocol level pings and wouldn't work at all with WASM. I also do not want to make it automatic as it can be expensive. We can always revisit this later. |
Although, I'm not 100% sure. I'll reopen this for now for further discussion. |
For my use case, we are running a few IoT devices connected through websocket. We can't guarantee the quality of the connection at all times, so we deal with a fair number of reconnects. Using the ping/pong on the server allow us to free the resources allocated to the connections and be aggressive with ping/pongs. The whole point is we want our server to know in a timely manner if a device got disconnected. Not sure if KeepAlive helps with that purpose. |
TCP keep alive is exactly for that purpose, it should be good enough for what you're doing. |
No one has commented on this in a while so going to close assuming the lack of a ping/pong API is fine. |
TCP Keepalive only operates on connections specifically marked "idle" and has no user application level, it only has a kernel level. Not to mention analytics for latency between server and client collected from ping/pong. |
Not necessarily true. E.g. with this library, your application could be deadlocked but you'd still respond to pings because this library spawns a readLoop goroutine. One also ought to have maximum time spent waiting for the next message anyway. So if the application is not responding/connected, the connection will be dced. In this library, this would correspond with the timeout on the context passed to c.Reader. E.g. a reasonable timeout might be 15 minutes which would bound the time between message reads to max 15 minutes. So in practice, I'm not sure if this would matter.
With Go at least, by default net/http sets a keep alive of 3 minutes so it shouldn't report after 2 hours.
Solid point, I didn't think about this. Will reopen. |
Could add an API like: type Conn struct {
// Receive on Pongs to receive WebSocket pongs.
// The library will never block on sending to Pongs.
// As such, it has a buffer of 1. If Pongs are not read
// further pongs will just be dropped.
// The string is the pong payload.
Pongs <-chan string
...
}
// Ping will send a WebSocket ping to the remote end with the given
// payload.
func (c *Conn) Ping(ctx context.Context, payload string) error {
} |
Could also take the HTTP/2 approach: c3mb0/net@780a5dd |
https://stackoverflow.com/questions/23238319/websockets-ping-pong-why-not-tcp-keepalive
The text was updated successfully, but these errors were encountered: