Skip to content

Conversation

@VeskeR
Copy link
Contributor

@VeskeR VeskeR commented Jan 16, 2026

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

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
Copy link

Copilot AI left a 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
Copy link

Copilot AI Jan 16, 2026

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).

Suggested change
**** @(RTO5a2b)@ This clause has been deleted
**** @(RTO5a2b)@ This clause has been deleted.

Copilot uses AI. Check for mistakes.
lawrence-forooghian added a commit to ably/ably-cocoa-plugin-support that referenced this pull request Jan 21, 2026
Needed for implementing RTO4d from [1].

[1] ably/specification#416
lawrence-forooghian added a commit to ably/ably-cocoa that referenced this pull request Jan 21, 2026
lawrence-forooghian added a commit to ably/ably-cocoa-plugin-support that referenced this pull request Jan 21, 2026
Needed for implementing RTO4d from [1].

[1] ably/specification#416
lawrence-forooghian added a commit to ably/ably-cocoa that referenced this pull request Jan 21, 2026
lawrence-forooghian added a commit to ably/ably-liveobjects-swift-plugin that referenced this pull request Jan 21, 2026
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
lawrence-forooghian added a commit to ably/ably-liveobjects-swift-plugin that referenced this pull request Jan 21, 2026
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
lawrence-forooghian added a commit to ably/ably-liveobjects-swift-plugin that referenced this pull request Jan 21, 2026
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
lawrence-forooghian added a commit to ably/ably-liveobjects-swift-plugin that referenced this pull request Jan 21, 2026
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
lawrence-forooghian added a commit to ably/ably-liveobjects-swift-plugin that referenced this pull request Jan 21, 2026
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
lawrence-forooghian added a commit to ably/ably-liveobjects-swift-plugin that referenced this pull request Jan 21, 2026
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
lawrence-forooghian added a commit to ably/ably-liveobjects-swift-plugin that referenced this pull request Jan 21, 2026
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
lawrence-forooghian added a commit to ably/ably-liveobjects-swift-plugin that referenced this pull request Jan 21, 2026
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
@lawrence-forooghian
Copy link
Collaborator

lawrence-forooghian commented Jan 21, 2026

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:

  • does it in a concise fashion
  • avoids describing Realtime implementation details (e.g. caching), and instead focuses on the contract with clients

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 incorrect to apply the buffered operations after a discontinuity (as opposed to just stating that they're "no longer relevant") — I think this requires more explanation than could be concisely expressed in the spec, and I'll elaborate on this below; we'd then remove the explanation from RTO4d
  • the best-effort behaviour currently described in RTO5f, and then remove that spec point

Why it would be an error to apply buffered operations after a discontinuity

Consider the case where the client ends up reconnecting to some different region r_reconnect. Upon attaching in r_reconnect, you're potentially attaching to r_reconnect's message stream at a position where the messages for some given region r are r-regional-order-earlier than the originating-in-r messages that you've buffered.

It's thus possible that there exists some operation m, originating in r, which has not yet been applied to the post-discontinuity sync sequence and which is r-regional order-earlier than the originating-in-r messages that you've buffered. If we were to apply the pre-discontinuity BufferedObjectOperations to the post-discontinuity sync sequence then, upon post-discontinuity receipt of m, we would discard it due to the siteTimeserials check, causing m to — incorrectly — never be applied to our local objects data.

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 m1 from the same region:

Why we clear buffered ops on discontinuity drawio(1)

lawrence-forooghian added a commit to ably/ably-liveobjects-swift-plugin that referenced this pull request Jan 21, 2026
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
Copy link
Collaborator

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

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants