Repository navigation
Deliver replicated messages to subscribers once after a restart or replication reconnect - #1362
Conversation
30d9df8 to
6338aeb
Compare
…oss resumes A checkpoint-grade route resumes below the origin's certified floor, so the origin re-sends part of its log the receiver already applied. Two nodes, numbered QoS 0 messages published on A, clean-session QoS 0 subscribers on B and A; A restarts, B's HTTP workers restart, and B restarts. Each case asserts no message reaches either subscriber twice, nothing sent after the interrupted side is back is lost on B, and B logged each of A's messages once. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…the restart job Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… logged Submodule core -> 6a0669337 (HarperFast/harper#4339, stacked on #4384). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…a floor-only resume, B's log covering the run The log oracle now refuses an empty result and reads only a log B has, each case waits for a floor below B's position so a floor-only resume must re-send, and the suite checks B recorded one. A restarted node that is not listening yet no longer fails the reconnect poll, and a node that started is torn down even when the other did not. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…d check certification after the first resume only startHarper replaces its context's harper object, so the nodes assigned before starting had no credentials or URLs; each start is now merged into the node ctx already holds, as startNode does elsewhere. A certified start keeps the lowest value, so only the first resume shows it was floor-only. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
6338aeb to
8c9d9a1
Compare
| @@ -1 +1 @@ | |||
| Subproject commit fe3b268e4be92658d7509a4e21f020632c8efc79 | |||
| Subproject commit 6a066933752eb7fd0d3166588efc56cf5dbd15ba | |||
There was a problem hiding this comment.
[major] Preserve the current base’s core fixes when repinning
The actual submodule delta from fe3b268e to 6a066933 changes 65 files and removes eight base-side commits, including restore-on-restart (#3987), the mTLS live-subscription fix (#4069), and analytics optimizations. This is a runtime rollback, not just ancestry divergence: core/dataLayer/rocksdbBackup.ts:643-663 no longer reads on_restart, and core/resources/Resource.ts:1217-1218 again refuses a refreshed principal without a role. Consequently, restoring a component-held database with on_restart=true loses its supported scheduling path, and certificate principals admitted without an hdb_user lose their subscriptions again. Rebase the core companions onto the current base core and repin to a commit containing both sets of changes. Confirm the resulting core tree retains these fixes before landing.
| ok(certifiedFrom > 0, 'B resumed from A floor-only'); | ||
| }); | ||
|
|
||
| test("the receiving node's HTTP workers restart", () => |
There was a problem hiding this comment.
[minor] The worker-restart case does not exercise replay when replication uses the pool
With the replication pool enabled, this case does not exercise replication replay: restartHttpWorkers only rolling-restarts HTTP workers, while integrationTests/cluster/clusterShared.mjs:248-251 documents that pool-on CI legs put replication on replication.threads workers and replication/DESIGN.md:109 states those pool workers own the sockets. Trigger: run the cluster suite with HARPER_CONFIG setting replication.threads > 0, remove core's isMessageLogged drop, and this case still passes because B's A-socket never disconnects, never resumes floor-only, and never re-streams a message; only the origin-restart and node-restart cases still cover the drop. Fix by making the pool case restart the replication owner (or pin this case pool-off / branch on replicationPoolActive()), so the named scenario reaches the resume it claims to test.
| deepEqual(duplicates(onB), [], 'message numbers the subscriber on B received more than once'); | ||
| deepEqual(duplicates(onA), [], 'message numbers the subscriber on A received more than once'); | ||
| deepEqual(missingFrom(onB, backUpN, publisher.n), [], `numbers from ${backUpN} B never delivered`); | ||
| const logged = await postToFixture(B, 'LoggedMessages', { peer: A.hostname, topic }); |
There was a problem hiding this comment.
[minor] The log-count assertion can conceal messages missing during B’s restart
The log completeness check cannot detect messages dropped from the segment a clean-session subscriber legitimately missed. In the node-restart case the subscriber check starts at resubscription (integrationTests/cluster/replicatedMessageRedelivery.test.mjs:152 and integrationTests/cluster/replicatedMessageRedelivery.test.mjs:162), so messages A published while B was down are covered only by LoggedMessages, but that check counts distinct versions for the whole topic against publisher.n - backUpN; pre-interruption entries count toward the threshold, so if replay drops D of the down-time messages the distinct count is n - D and the assertion still passes whenever D <= backUpN, which already includes both the pre-interruption count and the down-time count itself. Trigger: make B's resume drop the messages committed on A during B's restart; the subscriber never sees them, no duplicate is logged, and the suite passes despite B's origin log missing that segment. Check a segment-specific oracle instead, for example the distinct versions at or after the interruption point, not a topic-wide count.
⊙ Problem
On 5.4, a node that restarts, restarts its HTTP workers or reconnects replication delivers replicated MQTT messages to its live subscribers a second time. A checkpoint-grade route resumes exactly below the origin's certified floor (#1092), which trails what the node applied by up to an
originFloorsinterval, so the origin re-sends that part of its log. Record writes in it are dropped as duplicates; publishes were logged and delivered again.💡 Solution
replication/DESIGN.mdthat the leading-duplicate skip ties only stored records, so publishes rely on core's check.⚖️ Alternatives
✅ Verification
replicatedMessageRedelivery.test.mjs: a publisher on A, clean-session QoS 0 subscribers on B and A. A restarts, B's HTTP workers restart, B restarts. Each case asserts no duplicates on either node, nothing sent after the interrupted side is back is lost on B, and B logged each of A's messages once. It needs a second loopback address, so it has not run locally; CI runs it.Nothing sent after B was back was lost in any run; a node restart still delivered the messages sent while B was down.
Performance: no cost on in-order publishes: HarperFast/harper#4339 is stacked on HarperFast/harper#4384, so only a message at or below its topic's record version reads the log (measured on #4339: 17.9–18.0 µs per replicated publish vs main 18.0–18.2).
PR QA: skipped @ 8c9d9a1 — dispatch is not available in this session
🤖 Generated by Claude Opus 5.5 (Claude Code); posted via @ldt1996.
Release target: v5.4 — fixes a regression in the 5.4 betas from #1092's floor-only resume; 5.3 has none
Depends-on: HarperFast/harper#4384, HarperFast/harper#4339
Related PRs: #1358 independent
Complexity: medium
Deferred findings
3054506b6609bc66minor data-integrity —core— Core publish dedupe may drop a second same-topic publish sharing one transaction version, or re-deliver after receiver log pruning — NOT FILEDReview-Coverage: authored=claude; ran=codex,gemini,gemini-carbon; late=gemini-carbon:adjudicated; adjudicated=domain; blocked=cursor-composer(not-installed); declined=cursor-grok,muse; rounds=3; full=1 @ 8c9d9a1
Review-Attention: read ~3m (decisions: publish-dedupe-layer, perf-cost, shared-nodes-across-cases) @ 8c9d9a1