quarkusio / quarkus

Quarkus: Supersonic Subatomic Java.
https://quarkus.io
Apache License 2.0
13.82k stars 2.69k forks source link

websocket-next extension should be able to automatically broadcast pings #39862

Closed edeandrea closed 6 months ago

edeandrea commented 7 months ago

Description

This enhancement stems from this conversation: https://quarkusio.zulipchat.com/#narrow/stream/187030-users/topic/websockets-next

A typical WebSocket server will periodically send pings to its clients. A browser client will automatically respond with a pong when it receives a ping.

Since this is something that pretty much every websocket server should do, shouldn't the extension handle this? Other WebSocker server frameworks (like SockJS for example) do this automatically.

Implementation ideas

Maybe its opt-in (config property? annotation? Both?) where the user has control over the interval by which to send the ping message?

quarkus-bot[bot] commented 7 months ago

You added a link to a Zulip discussion, please make sure the description of the issue is comprehensive and doesn't require accessing Zulip

This message is automatically generated by a bot.

mkouba commented 7 months ago

A typical WebSocket server will periodically send pings to its clients. A browser client will automatically respond with a pong when it receives a ping.

Hm, I'm not an expert here but I think that it should be an opt-in feature because not all servers need it. Also if your application handles a LOT of connected clients then automatic pings may cause significant performance drop.

Since this is something that pretty much every websocket server should do, shouldn't the extension handle this? Other WebSocker server frameworks (like SockJS for example) do this automatically.

Do you have a link where the SockJS implementation/configuration is described?

gsmet commented 7 months ago

I think it's worth having a look at how other servers handle this. I know that when I used SSE in Quarkus GitHub App, I had to implement the ping/pong mechanism as otherwise things were VERY unreliable.

mkouba commented 7 months ago

BTW a simple blocking version of this functionality (after https://github.com/quarkusio/quarkus/pull/39858 was merged) implemented using the quarkus-scheduler could look like:

@Inject
OpenConnections connections;

@Scheduled(every = "5s")
void sendPing() {
   Buffer ping = Buffer.buffer("ping");
   for (WebSocketConnection c : connections) {
      c.sendPingAndAwait(ping);
   }
}
edeandrea commented 7 months ago

I totally agree it should be opt-in. And the user should have some control over the interval (by config property or something).

mkouba commented 7 months ago

Since this is something that pretty much every websocket server should do, shouldn't the extension handle this? Other WebSocker server frameworks (like SockJS for example) do this automatically.

Do you have a link where the SockJS implementation/configuration is described?

@edeandrea?

mkouba commented 7 months ago

CC @cescoffier

cescoffier commented 6 months ago

@mkouba About sockJS: https://vertx.io/docs/vertx-web/java/#_sockjs.

So, on the cloud, load balancers tend to aggressively recycle TCP connections when they "can" (remember that connection is their concurrency limit). To achieve this, most of them follow a rule: cut the connection if there is no application level frame within 10s.

When implementing an SSE, I use a Multi "merge" trick, sending empty JSON objects (or whatever format) and ignoring them on the client side.

For web sockets, I'm wondering if the ping/pong mechanism is enough. Indeed, it's not at the application level, but at level 4. We should try to reproduce this first.

@mkouba, the openshift sandbox follows the same strategy, so if we can keep a long-running connection just using the ping/pong on the openshift sandbox, we should be good to go.

mkouba commented 6 months ago

@mkouba About sockJS: https://vertx.io/docs/vertx-web/java/#_sockjs.

So these docs do not say anything about server -> client ping/pong implementation/configuration. Which is what I was trying to find out.

@mkouba, the openshift sandbox follows the same strategy, so if we can keep a long-running connection just using the ping/pong on the openshift sandbox, we should be good to go.

I'll try it and we'll see ;-).

mkouba commented 6 months ago

For the record - I tested the quarkus.websockets.next.server.auto-ping-interval with haproxy.router.openshift.io/timeout-tunnel on OpenShift and the auto ping from the server was enough to keep the connection alive.

IMPLEMENTATION NOTE: We do not broadcast the ping but instead schedule a timer for each connection separately.