Repository navigation
Stop repeated uploads of compacted history during sync - #74
Conversation
|
@greptileai Please review c808967. This fixes an observed production compacted-history retry storm after the browser upgrade. The new 128-message regression fails against the unmodified deployed relay: a later round asks the peer to resend discarded history. It passes with the per-connection acknowledgement view, including fresh-peer restore and unchanged live storage. All 68 relay tests and CI pass; please scrutinize range/rank correctness, peer isolation and cleanup. |
[Medium impact] The PR appears safe to merge; both previous findings are fixed and no new blocking issue was found. SummaryThe relay keeps compacted timestamps acknowledged by each connection in that connection’s sync view. It still stores and broadcasts only accepted payloads.
Diagram%%{init: {'theme': 'neutral'}}%%
flowchart TD
Peer[Peer uploads history] --> Guard[Check compaction tombstones]
Guard -->|Compacted timestamp| Ack[Remember timestamp for this connection]
Guard -->|New timestamp| Live[Write live payload]
Ack --> View[Connection's sync view]
Live --> View
Fresh[Fresh connection] --> LiveOnly[Live timestamps only]
Close[Connection closes] --> Clear[Release remembered timestamps]
Reviews (2) · Last reviewed commit: "fix: preserve partial range bounds and i..." · Reviewed by Greptile |
|
@greptileai Please review updated head 59134a4. Both findings are fixed with partial finite-range and incremental/duplicate/out-of-order oracle regressions. All 69 relay tests pass; the original deployed source fails the large-history repeated-round regression. Real current-browser SDK integration also passes all four transport scenarios. New CI is running; deployment remains pending. |
A long-lived client can repeatedly upload discarded history on later full sync rounds. The relay filters the payloads correctly, but its reconciliation index forgets that this connection already supplied those timestamps.
Keep a per-WebSocket, per-owner timestamp-only acknowledgement view alongside live storage. Range fingerprints, ranks and iteration include only compacted timestamps that this peer has actually supplied. Fresh peers retain the live-only view and never request unavailable payloads. Release acknowledgements on disconnect and owner deletion; keep quotas, owner locks, accepted-only broadcasts and database contents unchanged.
Validation: all 69 relay tests pass. The new regression uses both small and frame-sized histories plus a substantial rebuilt history, checks fresh restore and unchanged storage, and requires later full rounds on the same connection to settle without retransmission. The 128-message regression fails against the unmodified deployed relay and passes with this fix. Independent set-oracle tests cover range fingerprints, iteration, boundaries, duplicates and owner/session isolation.
This follows the prolonged syncing report after get-based PR #1698. Production diagnostics found sustained compacted-history filtering for a non-test owner; no user records or keys were read. Merged as
4f44ebb32d8c44d29503cb075d57ce48a7519b28and deployed with the maintained Node 24.22.2 image. All 69 tests also passed inside the replacement image. The previous image and verified online SQLite backups are retained; the database volume, configuration and gateway were preserved.Post-deployment acceptance: all 15 synthetic workflows passed against the actual production application and relay, covering all 57 registered data surfaces, profiles/deletions, stored chats, settings, encrypted Agent Access, offline browser restart, self-compaction/rebuild, fresh restore and owner isolation. The actual SDK replication state and header both reached
Synced, with no page errors. Both synthetic owners were cleaned to zero stored messages/bytes. Aggregate replay filtering stopped after the initial reconnect backlog, and the affected user confirmed normal sync completion. Greptile rated the reviewed source 5/5; CI and CodeQL passed.