Repository navigation
fix(crdt): accept old offline edits, merge nothing on a hook rejection, stop pulls skipping rows - #35
Merged
Merged
Conversation
juicycleff
added a commit
that referenced
this pull request
Oct 7, 2026
…ers per node (#36) * fix(crdt): keep a shadow row's own clock apart from its cursor and pull counters per node A shadow row now has two clocks. The state's own clock, stored in crdt_state, is what merges and LWW compare and what a pull hands out as the change HLC. The hlc_ts and hlc_counter columns are the cursor position pulls page by. Every writer so far kept them equal, and rows written that way read back exactly as before, so nothing moves until a writer asks for a different position through WriteFieldStateAt or WriteTombstoneAt. Tombstone rows store their delete clock in crdt_state as well, and ReadState keeps the latest delete clock across nodes, not whichever row it happened to read last. A counter row is pulled as one change per node it holds. The server merges an increment into whichever node's row has the newer clock, so a pull that only sent the row's own node dropped the other node's totals. That is why the Dart convergence test left device B at 7. * fix(crdt): restamp a row's cursor when a push changes it so late stamps reach every peer A device that was offline pushes changes stamped hours ago. The server merged them, but the row kept the old clock as its cursor position, so a peer that had already pulled past that clock never got the merged state. Now a push that changes a row's stored state moves the row to a fresh cursor position: past the plugin clock, past every position this process handed out, past the highest one in the table, and past the row's own clock. Only the position moves. The change keeps its own clock on the wire, so you get the record you would have got by pulling earlier, and every client picks the winner it would have picked then. An LWW value that loses writes nothing. Neither does an older tombstone or a retried push. Pulls and streams resume from cursor positions, including the multi-table window from #35. Allocation and the write share one lock per plugin, which also stops two concurrent pushes to one field from losing each other's merge.
juicycleff
added a commit
that referenced
this pull request
Oct 8, 2026
… peer (#37) * fix(crdt): keep a shadow row's own clock apart from its cursor and pull counters per node A shadow row now has two clocks. The state's own clock, stored in crdt_state, is what merges and LWW compare and what a pull hands out as the change HLC. The hlc_ts and hlc_counter columns are the cursor position pulls page by. Every writer so far kept them equal, and rows written that way read back exactly as before, so nothing moves until a writer asks for a different position through WriteFieldStateAt or WriteTombstoneAt. Tombstone rows store their delete clock in crdt_state as well, and ReadState keeps the latest delete clock across nodes, not whichever row it happened to read last. A counter row is pulled as one change per node it holds. The server merges an increment into whichever node's row has the newer clock, so a pull that only sent the row's own node dropped the other node's totals. That is why the Dart convergence test left device B at 7. * fix(crdt): restamp a row's cursor when a push changes it so late stamps reach every peer A device that was offline pushes changes stamped hours ago. The server merged them, but the row kept the old clock as its cursor position, so a peer that had already pulled past that clock never got the merged state. Now a push that changes a row's stored state moves the row to a fresh cursor position: past the plugin clock, past every position this process handed out, past the highest one in the table, and past the row's own clock. Only the position moves. The change keeps its own clock on the wire, so you get the record you would have got by pulling earlier, and every client picks the winner it would have picked then. An LWW value that loses writes nothing. Neither does an older tombstone or a retried push. Pulls and streams resume from cursor positions, including the multi-table window from #35. Allocation and the write share one lock per plugin, which also stops two concurrent pushes to one field from losing each other's merge. * fix(crdt): cut time travel on each row's own clock, not its cursor position ReadStateAt filtered rows on hlc_ts and hlc_counter. Since a push now restamps the position of every row it changes, a row merged at T+10s with its own clock at T+1s vanished from every cut before T+10s, taking all of its field state with it. You'd ask for the record at T+5s and get no views field at all. We now read the record's rows and keep those whose own clock (the field state HLC, or the tombstone's delete clock) is at or before the cut. That is what the cut meant before restamping existed. The in-memory shadow fake now honours a position cut when a query has one, so the test fails on the old query instead of passing by accident. ReadFieldHistory gets a doc note: its since, order and limit use positions while each entry reports the row's own clock. * fix(crdt): pull a counter row as one record carrying its full state A counter row went out as one record per node it held. One +2 on a counter that 500 devices had touched sent 500 records (about 89 KB) to every client on its next pull, and the Go Syncer paid a read and a write for each of them. Now a counter row is one record with State set to the stored PNCounterState, the same carrier sets, lists, text and documents already use. Go ApplyChange, crdt-js mergeFieldState and Dart applyChange all merge a state carrier before they look at counter_delta, so every current client still gets every node's totals and still converges after a late increment. The record keeps counter_delta for the row's own node, which is what a client that predates state carriers saw before. * fix(crdt): read each table's max cursor once per push and narrow the lock Three smaller fixes to the push path, all under the cursor write lock. Each changed row used to cost a maxCursor query on top of its read and its upsert, serialized across every client by one mutex. A push now reads each table's stored maximum once (cursorBatch). Within one process the allocator's high mark already covers every position it handed out, so the stored maximum only matters after a restart or for another instance, and reading it once per push still keeps a restarted server past its old rows. AfterMetadataWrite ran under the lock because of a deferred unlock. It now runs after the lock is released, so a slow hook stalls only its own push. A record delete read the whole record first, so a field row that failed to decode failed the delete. It now reads only the record's tombstone rows. The cursorAllocator comment now says what the lock does not cover: local writes through AfterMutation (the lost-update guarantee is push against push only), other instances sharing the database, and which plugin hooks still run under it. * fix(crdt): restamp rows the Syncer pulls so a hub's clients receive them A hub pulls from upstream with a Syncer and serves its own clients with a SyncController. The Syncer wrote each pulled row at its own clock, and ordinary polling lag puts that clock behind the hub clients' cursors, so on a hub almost any upstream row could be missed by its clients. This was true before restamping too; it is the same bug as the late stamp. mergeRemoteChange now does what a push does. It takes the plugin's cursor write lock around the read, merge and write, writes nothing when the merge leaves the state unchanged, and writes a changed row at a fresh position from the shared allocator. A delete is written only when it is newer than the stored one, also at a fresh position. The AfterInboundChange hook runs after the lock is released.
This was referenced Oct 8, 2026
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
1 participant
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.
Three fixes to the CRDT sync server. Each one loses or refuses data a client sent in good faith, and each comes with a test.
Old offline edits are accepted again
Drift validation compared a change's HLC against server time in both directions, so a change stamped more than
MaxHLCDrift(an hour by default) in the past was rejected. That's exactly what a device produces after a few days offline: you edit, the edits wait in the queue, and when you reconnect every one of them bounces. Now only changes stamped too far in the future are refused. Older changes are always accepted, since they're just late.TestValidateChangeRecord_OldOfflineChangeAcceptedcovers it.A hook rejection merges nothing
HandlePushranBeforeInboundChangechange by change, merging as it went. When a hook rejected change 5 of 10, changes 1 to 4 were already merged, but the client was told the whole push failed, so its idea of what the server holds was wrong. The hooks now run over the whole batch first, and a rejection fails the push with nothing applied. A hook that returns nil still skips just that change, as before.TestSyncController_HandlePush_HookRejectionMergesNothingcovers it.A multi-table pull no longer skips rows
HandlePullandStreamChangesSinceread each table with its own page limit, then moved the cursor to the newest HLC across all of them. If one table filled its page and another table had a newer row, the cursor jumped past the first table's unread rows, and the next pull never saw them.readChangesWindownow cuts the window at the earliest full-page end across the tables, so the next pull resumes there. The comparison uses (timestamp, counter) only, matching the shadow-table cursor.TestSyncController_HandlePull_MultiTableBacklogNotSkippedcovers it.Running it
go test -race ./crdt/...Wire shapes don't change, and neither do the merge or apply rules. Clients built against the current server keep working. The Dart client in #34 runs its convergence and conformance tests against this branch as well as
main, and passes on both.One server gap these fixes don't touch: a late-stamped merge of a counter or set still isn't redelivered to peers that already pulled past its HLC. #34 describes it, and has a skipped test for it.