fix(p2p): replication reliability — durable checkpoints, reconnection, convergent LWW#54
Open
mads-jm wants to merge 7 commits into
Open
fix(p2p): replication reliability — durable checkpoints, reconnection, convergent LWW#54mads-jm wants to merge 7 commits into
mads-jm wants to merge 7 commits into
Conversation
Persist per-peer/per-collection replication checkpoints to a debounced JSON file under userData so a relaunch resumes incrementally instead of full-resyncing every collection (#40). Stop silently dropping changes on pull timeout: a timed-out pull now rejects and does not advance the checkpoint, and the responder advances to the newest returned doc's updatedAt rather than wall-clock now. Act on the previously-dropped pull-response on the requester side. Replace fixed 10s relay retries with exponential backoff + jitter, a single alternate-relay fallback, and a liveness heartbeat for half-open links (#41). Wire protocol id unchanged. Covered by checkpoint-store, backoff, relay-manager and replication suites (#32).
Replace the brittle string comparison of updatedAt in replication-handler with epoch-ms parsing (lww.ts). Unparseable/missing timestamps resolve to oldest rather than throwing. Equal-timestamp ties are broken by a deterministic contentKey projection that strips id, updatedAt/addedAt and RxDB-internal underscore fields identically on both sides, so two peers comparing a transmitted data payload against a full toJSON elect the SAME winner and converge (acceptance #42). Covered by lww suite incl. a cross-peer convergence test (#32).
Route serve-stream writes through a per-stream serialized queue that awaits the libp2p stream drain signal before overrunning the write buffer, so a slow reader paces the sender instead of triggering the stream reset that made >10MB transfers flaky. Removes the standing backpressure TODO (#47). Covered by a large-file (>10MB) reassembly + sha256 integrity test and a drain-wait unit test (#32).
Add handshake capability-exchange tests (success, capability-mismatch surfaced not rejected, empty-stream no-op) on the shared in-memory libp2p harness, closing the last untested protocol path (#32).
Update the RxDB-Replication concept page to reflect durable checkpoints, the non-silent pull-timeout policy, relay backoff/fallback/heartbeat, and skew-aware LWW with the convergent tie-break. Records the explicit decision NOT to bump the wire-protocol version (stays /whatnext/rxdb-replication/1.0.0) per the epic's no-silent-bump constraint.
The sender strips device-local fields (localFilePath, localFileSize, albumArtLocalPath, coverArtLocalPath) before transmission, but the receiver computed the equal-timestamp content tie-break key over the unstripped stored doc. On an exact-ms tie the existing-side key carried localFilePath (which sorts before name), so the incoming version won on both peers -- they swapped user content and oscillated indefinitely, violating cross-peer convergence (AC #42). Introduce a single DEVICE_LOCAL_FIELDS constant in schemas.ts consumed by both the sender strip and lww.contentKey so the two field sets cannot drift. Receiver-side only: wire format and protocol id are unchanged.
|
The Architect handoff notes are working artifacts of the agent cycle, not project source. They were the only add/add conflict when integrating the Wave-1 lanes, and they do not belong in the reviewable diff. Removing them here keeps the PR diff to real changes; .claude/ scratch is gitignored on mvp.
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.
Short Description
Hardens RxDB-over-libp2p replication from happy-path to reliable across all five fragile surfaces the state-of-the-union flagged: durable checkpoints (no more full resync per launch), pull timeout/backoff + relay reconnection, skew-aware LWW with a convergent tie-break, backpressure-safe file-chunk sends, and the P2P test coverage that was entirely absent.
Background
The replication layer was architecturally sound but operationally brittle, with zero tests. The subtle one is the LWW tie-break: on an exact-millisecond
updatedAttie we fall back to a content key. The sender strips device-local fields (localFilePath,localFileSize,albumArtLocalPath,coverArtLocalPath) from the transmitted payload, but the receiver computed the key overexisting.toJSON(), which kept them — so the two sides hashed different field sets and, because'l' < 'n', the existing side always lost, swapping user content on every tie. That's a non-convergent oscillation and a direct violation of "identical on both peers". It's fixed by sourcing the device-local field list from a single shared constant (DEVICE_LOCAL_FIELDSinschemas.ts) consumed by both the sender-strip and the tie-break, so the two can't drift apart again.What Has Changed
checkpoint-store.ts(new) persists per-collection/peer checkpoints, killing the full-resync-every-launch behavior (was in-memory atp2p-service.ts:71). Path re-derived from platform userData conventions in the utility processbackoff.ts(new); pull no longer resolves empty-on-timeout silently;relay-manager.tsgains backoff + reconnection and agetConnections-based heartbeat for dropped/half-open links (p2p-config.tstunables)lww.tstie-break stripsDEVICE_LOCAL_FIELDSfrom both sides before the content key (replication-handler.ts+useSessionReplication.tsshare the constant)file-transfer.tschunk writes respect stream backpressure (was unhandled at:463)replication.test.ts,handshake.test.ts,relay-manager.test.ts,backoff.test.ts,checkpoint-store.test.ts,file-transfer.test.ts, plus a sharedharness.ts; convergence regression cases inlww.test.ts(incl. one with all four device-local fields populated)RxDB-Replication.mdupdated for the new behavior and the tie-break field exclusionSteps for Reviewing
protocols/replication.tsand confirm no message type/shape/version changed — only behavior. ✅ inspector independently confirmed (no diff on message types /ipc-protocol)schemas.tsDEVICE_LOCAL_FIELDS→ confirm it's the single source consumed by bothuseSessionReplication.ts(sender-strip) andlww.ts(tie-break). This is the convergence fix.lww.test.tsconvergence cases → confirm they'd fail without the fix (the stored fixture now carrieslocalFilePath, which is exactly what let the original bug through).checkpoint-store.ts→ durability + graceful degradation to full resync if the userData path can't be derived.relay-manager.ts→ reconnection/backoff/heartbeat.cd app && npm test— 254 pass. ✅ independently re-run;src/typecheck clean. ✅Checklist
src/; inspector verdict APPROVE after fix-passRxDB-Replication.md(the standing P2P condition)needs-p2p-review)chore/fix-dev-env-lintCloses #40
Closes #41
Closes #42
Closes #47
Closes #32
Comments
Carry-forwards (non-blocking):
WhatNext(degrades gracefully to full resync); main could inject the path later.