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

Switch ws-kalive-ms from fixed interval to sliding #455

Open
Tracked by #456
ptaoussanis opened this issue Nov 13, 2024 · 0 comments
Open
Tracked by #456

Switch ws-kalive-ms from fixed interval to sliding #455

ptaoussanis opened this issue Nov 13, 2024 · 0 comments

Comments

@ptaoussanis
Copy link
Member

ptaoussanis commented Nov 13, 2024

Sente has a keep-alive mechanism for both the client and server, controlled by the :ws-kalive-ms options.

These are intended to provide an upper bound on idle time, but the current design does not actually achieve this.

The current design:

  • Every ws-kalive-ms (fixed interval) check if there's been any activity in the previous window
  • If yes: noop
  • If no: send a ping

The problem:

When a pong response is received, it'll be in the next window - often very shortly after the ping was sent. This can lead to unintentionally long periods of connection idleness, e.g.:

Period1 (t0+Δ->t1): no activity           => PING at t1 (t0+1Δ) <-
Period2 (t1+Δ->t2): pong received at t1+ε => noop at t2 (t0+2Δ)
Period3 (t2+Δ->t3): no activity           => PING at t3 (t0+3Δ) <-
Period4 (t3+Δ->t4): pong received at t3+ε => noop at t4 (t0+4Δ)
Period5 (t4+Δ->t5): no activity           => PING at t5 (t0+5Δ) <-

The part I'd foolishly missed: if the last activity was the pong at t1+ε, then the interval t3 - (t1+ε) approaches (i.e. twice the desired upper bound).

The most obvious solution would be to switch to a sliding window rather than a fixed interval - but there may also be alternative (simpler?) solutions. Most simply probably just halving the default ws-kalive-ms and documenting the current behaviour - but let's evaluate our options.

PRs or ideas welcome, otherwise I'll take a look at doing this myself next time I'm on batched Sente work 👍

Big thanks to @p-himik for identifying this bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant