add websocket pings/pongs to help close zombie connections#410
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
paulgb
approved these changes
Sep 15, 2025
paulgb
left a comment
paulgb
left a comment
Member
There was a problem hiding this comment.
LGTM. I think TCP does something similar already so I'm not sure if this is the problem, but it doesn't hurt and should at least give us some better insight into when a connection is dropped because of missed heartbeat.
rolyatmax
force-pushed
the
taylor/add-ping-pong-to-websocket-server
branch
from
September 15, 2025 19:36
bfc3616 to
a83bbd6
Compare
Member
Author
@paulgb yeah, TCP does, but my understanding is that these timeouts can be much longer. According to ChatGPT, Linux will wait for 2 hours of idleness before sending the first probe. This is also configurable though and could be worth setting on our proxies. |
There was a problem hiding this comment.
Pull Request Overview
This PR implements WebSocket ping/pong functionality to detect and close zombie connections that may have been lost without proper cleanup. The implementation sends periodic ping messages to clients and monitors for pong responses to determine connection health.
- Added ping/pong timeout constants with 20-second ping intervals and 40-second pong timeouts
- Modified the WebSocket message sending loop to include periodic ping messages with timeout detection
- Added pong message handling to update the last received pong timestamp
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
abyssparanoia
added a commit
to arcterus-jp/y-sweet
that referenced
this pull request
Feb 4, 2026
* derive Deserialize for ClientToken (jamsocket#403) makes it easier to consume the ClientToken for other rust libraries * Add support for Y_SWEET_MAX_BODY_SIZE (jamsocket#405) This PR adds support for `Y_SWEET_MAX_BODY_SIZE` environment variable to override the default axum websocket body size of 2MB * better websocket URL generation (jamsocket#406) * v0.9.0 (jamsocket#408) * add websocket pings/pongs to help close zombie connections (jamsocket#410) * fix some websocket server issues (jamsocket#411) One of these fixes an issue I introduced in my last PR. The other is pre-existing. * v0.9.1 (jamsocket#412) * Allow pycrdt up to and including 0.12.x (jamsocket#414) Fixes jamsocket#400 Tested locally: ``` uv run pytest ...... [100%] 6 passed in 20.34s ``` * only persist documents to S3 when dirty (jamsocket#416) Fix an issue where y-sweet was persisting documents to S3 every ~10 seconds even when no changes had occurred. From what I understand, the persistence worker wakes up both on the dirty signal and also on a `checkpoint_freq` interval and was always calling persist, regardless of the dirtiness, which would write it to S3 regardless of whether any changes had been made. The fix adds a check to only persist when the document is actually dirty. I'm still a little bit unclear behind the reasoning for the logic of signaling the worker both on a dirty callback and on a timer. So I wonder if we instead just start the worker once, wake every 10 seconds, check if dirty, persist if needed. * use SeqCst ordering for sync_kv dirty flag (jamsocket#417) Now that [we're not persisting unless the document is marked dirty](jamsocket#416), let's use an atomic ordering with stronger guarantees for the KV store's `dirty` flag. * Allow to disable GC within documents using env variable/CLI (jamsocket#422) ### References - closes jamsocket#421 ### Description Adds an option to disable GC within documents (distinct from GC of documents in multi-document mode) via: - CLI by passing `--skip-gc` to either of `y-sweet serve` or `y-sweet serve-doc` - `Y_SWEET_SKIP_GC` environment variable I was not sure if I should also add the environment variable to `y-sweet-worker` for feature parity but given that it is deprecated I went for a smaller changeset and only added it in the server. I wonder if the configuration for toggling GC of documents themselves (the `doc_gc` option which is not exposed by clap) would be advisable, but I think it is already possible to configure `checkpoint_freq_seconds` instead which can be set to a very high value that is probably better than turning it off altogether. * feat: divide before main stream * fix: lint --------- Co-authored-by: Theodor Tonum <theodor@tonum.no> Co-authored-by: Vinay M <rmdort@gmail.com> Co-authored-by: Taylor Baldwin <taylorbaldwin@gmail.com> Co-authored-by: Michał Krassowski <5832902+krassowski@users.noreply.github.com> Co-authored-by: Aleksey Smolenchuk <lxe@lxe.co>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.