Bound sliding sync per-connection state under position replay#19924
Draft
erikjohnston wants to merge 2 commits into
Draft
Bound sliding sync per-connection state under position replay#19924erikjohnston wants to merge 2 commits into
erikjohnston wants to merge 2 commits into
Conversation
Clients may resend the same sliding sync position many times, e.g. if they repeatedly fail to process our responses. Each such request records a new connection position, but the clean-up of old positions only ran the first time a position was used: it lives in `get_and_clear_connection_positions`, whose result is cached, so replayed positions never triggered it. A wedged client replaying one position could thus accumulate unboundedly many rows in `sliding_sync_connection_positions` (and, via the per-position state, in `sliding_sync_connection_streams`/`_room_configs`). Seen in the wild: ~1k positions on one connection over a few hours. Fix by deleting all other positions for the connection whenever we persist a new one: only the position the request was based on (which the client may retry from, not yet having seen our response) and the newly created position remain valid. Also update the connection's `last_used_ts` when persisting, since for replayed positions the cached read no longer does so; otherwise a connection in constant use from the same position would be expired as idle. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
Clients may resend the same sliding sync position many times, e.g. if they repeatedly fail to process our responses. Each such request records a new connection position, but the clean-up of old positions only ran the first time a position was used: it lives in
get_and_clear_connection_positions, whose result is cached, so replayed positions never triggered it. A wedged client replaying one position could thus accumulate unboundedly many rows insliding_sync_connection_positions(and, via the per-position state, insliding_sync_connection_streams/_room_configs). Seen in the wild: ~1k positions on one connection over a few hours.Fix by deleting all other positions for the connection whenever we persist a new one: only the position the request was based on (which the client may retry from, not yet having seen our response) and the newly created position remain valid.
Also update the connection's
last_used_tswhen persisting, since for replayed positions the cached read no longer does so; otherwise a connection in constant use from the same position would be expired as idle.