Skip to content

Improve websocket liveness checks and idle timeout handling - #5517

Open
aasoni wants to merge 4 commits into
masterfrom
aasoni/ws-idle-timeout-rework
Open

Improve websocket liveness checks and idle timeout handling#5517
aasoni wants to merge 4 commits into
masterfrom
aasoni/ws-idle-timeout-rework

Conversation

@aasoni

@aasoni aasoni commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Description of Changes

  • Idle timeout no longer requires a pong to restart: any data received from the client (including Pong frames) counts as activity and extends the idle deadline. Per review feedback, send progress does not count as activity, completing a write only proves the socket (or LB) buffer isn't full, not that the client is alive, and testing showed counting it did not help the disconnection issue under investigation.

  • On idle timeout, initiate a proper close handshake carrying an "idle timeout" reason instead of abruptly tearing down the connection, so well-behaved clients can tell why they were disconnected. The connection is torn down if the handshake does not complete within a 10s grace period.

  • Fix the kick log message to report the channel's configured capacity instead of its remaining capacity (always 0 at that point).

API and ABI breaking changes

No

Expected complexity level and risk

3

Testing

These changes were made on the BitCraft cluster and were tested by hundreds of players connecting and subscribing to large amounts of data. I am fairly confident the change doesn't break things. I am less confident it's the disconnection silver bullet, but it may help.

@aasoni
aasoni force-pushed the aasoni/ws-idle-timeout-rework branch from e4fc66d to 234f6c9 Compare July 10, 2026 14:30
@aasoni
aasoni requested review from jsdt and kim July 10, 2026 14:32

@jsdt jsdt left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I drafted a PR on top of this branch to make the idle tracking more straightforward (IMO): #5521

I discussed it in another comment, but I don't think we want to bump the idle timeout on writes.

Comment thread crates/core/src/client/client_connection.rs Outdated
Comment thread crates/client-api/src/routes/subscribe.rs Outdated
@kim

kim commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

@aasoni If it is still to be merged, can you update this branch + resolve conflicts?

@aasoni
aasoni force-pushed the aasoni/ws-idle-timeout-rework branch 2 times, most recently from d7eeca2 to d893391 Compare August 12, 2026 09:54
@aasoni
aasoni requested a review from jsdt August 12, 2026 11:16
@aasoni
aasoni force-pushed the aasoni/ws-idle-timeout-rework branch from d893391 to 1af4d2b Compare August 12, 2026 11:18
@aasoni
aasoni enabled auto-merge August 12, 2026 19:54
auto-merge was automatically disabled August 13, 2026 14:46

Pull Request is not mergeable

aasoni and others added 3 commits August 18, 2026 11:25
- Idle timeout no longer requires receive activity alone: the send loop
  extends the idle deadline whenever it makes write progress
  (rate-limited to once per second).

- On idle timeout, initiate a proper close handshake carrying an
  "idle timeout" reason instead of abruptly tearing down the
  connection, so well-behaved clients can tell why they were
  disconnected. The connection is torn down if the handshake does not
  complete within a 10s grace period.

- The "websocket connection aborted" warning and ws_clients_aborted
  metric now fire only when the client actor task is actually aborted
  or panics, not on every normal disconnect.

- Fix the kick log message to report the channel's configured capacity
  instead of its remaining capacity (always 0 at that point).
Instead of passing around part of a watch to extend the timeout, this
change adds a `last_activity` field to `ActorState`. It can be updated
by calling `record_activity()` on the actor state, and you can get an
idle timeout future by calling `idle_timer` on the actor state. IMO this
is a bit more straightforward, and it reduces how many arguments we are
passing around.
Per review feedback: completing a ws.feed only means the socket buffer
(including any LB/proxy buffers) isn't full, not that the client is
making progress, so a dead client could be kept alive for a long time
by small periodic messages. Testing also showed this did not help the
disconnection issue being investigated.

Only data received from the client now counts as activity. Also update
docs that still described the old behavior, and fix the unawaited
tokio::time::advance calls in the last-activity test.
@aasoni
aasoni force-pushed the aasoni/ws-idle-timeout-rework branch from 5613d72 to e9ee86e Compare August 18, 2026 09:25
@aasoni
aasoni enabled auto-merge August 18, 2026 09:25
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.

3 participants