-
Notifications
You must be signed in to change notification settings - Fork 3
[AIT-286] Fix BufferedObjectOperations clearing rules for LiveObjects sync #416
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
base: main
Are you sure you want to change the base?
Conversation
BufferedObjectOperations should only be cleared when ATTACHED with RESUMED flag not set is received or on sync completion, not when a new sync sequence ID arrives. This is because the server caches object state at channel attachment time, not at the start of a sync sequence. Resolves AIT-286
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes the BufferedObjectOperations clearing behavior for LiveObjects synchronization by ensuring operations are cleared at the correct time based on when the server caches object state.
Changes:
- Added RTO4d to specify that BufferedObjectOperations must be cleared when ATTACHED is received without the RESUMED flag, since the server caches object state at channel attachment time
- Added RTO5f as an explanatory note describing the expected and edge-case behaviors for server-initiated resyncs
- Deleted RTO5a2b which previously required clearing BufferedObjectOperations when a new sync sequence ID arrived
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| *** @(RTO5a2)@ If a new sequence id is sent from Ably, the client library must treat it as the start of a new objects sync sequence, and any previous in-flight sync must be discarded: | ||
| **** @(RTO5a2a)@ The @SyncObjectsPool@ list must be cleared | ||
| **** @(RTO5a2b)@ The @BufferedObjectOperations@ list must be cleared | ||
| **** @(RTO5a2b)@ This clause has been deleted |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The deleted clause statement should end with a period for consistency with other deleted clauses in the specification (see RSN4c, TS3, etc. in features.textile).
| **** @(RTO5a2b)@ This clause has been deleted | |
| **** @(RTO5a2b)@ This clause has been deleted. |
Needed for implementing RTO4d from [1]. [1] ably/specification#416
Needed for implementing RTO4d from [1]. [1] ably/specification#416
Needed for implementing RTO4d from [1]. [1] ably/specification#416
Needed for implementing RTO4d from [1]. [1] ably/specification#416
That is, do it when we get a discontinuity, not when a new sync sequence changes, per spec changes in [1]. [1] ably/specification#416
That is, do it when we get a discontinuity, not when a new sync sequence changes, per spec changes in [1]. [1] ably/specification#416
That is, do it when we get a discontinuity, not when a new sync sequence changes, per spec changes in [1]. [1] ably/specification#416
That is, do it when we get a discontinuity, not when a new sync sequence changes, per spec changes in [1]. [1] ably/specification#416
That is, do it when we get a discontinuity, not when a new sync sequence changes, per spec changes in [1]. All written by Claude. [1] ably/specification#416
That is, do it when we get a discontinuity, not when a new sync sequence changes, per spec changes in [1]. All written by Claude. [1] ably/specification#416
That is, do it when we get a discontinuity, not when a new sync sequence changes, per spec changes in [1]. All written by Claude. [1] ably/specification#416
That is, do it when we get a discontinuity, not when a new sync sequence changes, per spec changes in [1]. Integration tests ported from JS in [2] at 06b746a. All written by Claude. [1] ably/specification#416 [2] ably/ably-js#2150
Let's have a DR?In general, the spec usually describes the "what" and not the "why". When it does explain the "why", it usually:
I think that we'd benefit from having a short DR explaining the motivation behind this new behaviour. I think that the two things it should describe are:
Why it would be an error to apply buffered operations after a discontinuityConsider the case where the client ends up reconnecting to some different region It's thus possible that there exists some operation This diagram, showing the events from the point of view of a Realtime client, might explain it better; assume that all of the blue messages are from the same region, and that there exists another earlier message |
That is, do it when we get a discontinuity, not when a new sync sequence starts, per spec changes in [1]. Integration tests ported from JS in [2] at 06b746a. All written by Claude. [1] ably/specification#416 [2] ably/ably-js#2150
| *** @(RTO5a2)@ If a new sequence id is sent from Ably, the client library must treat it as the start of a new objects sync sequence, and any previous in-flight sync must be discarded: | ||
| **** @(RTO5a2a)@ The @SyncObjectsPool@ list must be cleared | ||
| **** @(RTO5a2b)@ The @BufferedObjectOperations@ list must be cleared | ||
| **** @(RTO5a2b)@ This clause has been deleted |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd say replaced, not deleted
BufferedObjectOperations should only be cleared when ATTACHED with RESUMED flag not set is received or on sync completion, not when a new sync sequence ID arrives. This is because the server caches object state at channel attachment time, not at the start of a sync sequence.
Resolves AIT-286
See ably-js implementation ably/ably-js#2150