Fix update_advances? to be exact for delete-bearing updates - #34
Merged
Conversation
`Doc#update_advances?` conservatively returned true for any update carrying a delete set, because deletes don't move the state vector, so the cheap state-vector probe couldn't prove a duplicate. A lost-ack retry of a deletion the server had already integrated was therefore re-recorded and re-broadcast every time -- durable spam that apps had to guard against with their own encode-and-compare around every write. For delete-bearing updates, compare the full encoded document state (which includes the delete set) before vs. after a trial apply on an isolated probe: a genuinely new deletion changes it (true); an already-applied retry re-encodes identically (false). Insert/format-only updates keep the cheaper state-vector path, so only delete-bearing frames -- a minority -- pay for the exact compare. The exactly-once guarantee is unchanged in the safe direction: a real deletion is never dropped. This lets yrby-actioncable settle a duplicate pure-delete frame as :applied (acked, not stored or relayed), so `sync.rb`'s `update_advances?` gate now does the right thing without app-level compensation. Tests: - protocol.rs: pure-delete retry does not advance; a delete bundled with a new insert still advances (and its byte-identical retry does not). - sync_test.rb: end-to-end ActionCable -- a lost-ack delete retry is acked but not re-recorded or re-broadcast, via a new real-Y.js DeleteRetry fixture (added to the fixture generator). Co-Authored-By: Claude Opus 4.8 <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.
Prod issue
On docs with pure-delete retries,
yrby-actioncablere-recorded and re-broadcast deletions the server had already integrated.Doc#update_advances?returnedtruefor any delete-bearing update, so a lost-ack retry of a deletion looked like it advanced the doc every time — durable spam (duplicate rows, extra broadcasts, history-job noise) that the app had to compensate for with its own encode-and-compare guard around every write.Root cause
Deletes don't move the state vector (a deletion tombstones an existing struct rather than adding one), so the cheap state-vector-growth probe couldn't tell a new deletion from an already-applied one. The old code took the safe-but-blunt route: any delete set ⇒
true(record it).Fix
Measure the real effect on an isolated probe (never mutating the real doc), then branch on how to compare:
true); an already-applied retry re-encodes byte-identically (false).Only delete-bearing frames — a minority — pay for the exact comparison. The exactly-once guarantee is unchanged in the safe direction: a real deletion is never dropped.
This flows straight through
sync.rb's existing gate:A duplicate pure-delete frame is now acked but not stored or relayed — so the app-level replay-and-compare guard can be removed.
Tests
protocol.rs— a pure-delete retry does not advance; a delete bundled with a new insert still advances (and its byte-identical retry does not).sync_test.rb— end-to-end ActionCable: a lost-ack delete retry is acked ([1,2,3]) but recorded/broadcast only once. Uses a newDeleteRetryfixture generated from real Y.js (added togenerate_fixtures.mjs; the Y.js-emitted delete bytes match yrby's byte-for-byte).Bumps yrby to 0.2.3.
🤖 Generated with Claude Code