Skip to content

test(cluster): pin the mid-log txnlog tear test to the fail-stop quarantine semantics - #802

Draft
kriszyp wants to merge 7 commits into
mainfrom
test/txnlog-tear-quarantine-semantics
Draft

test(cluster): pin the mid-log txnlog tear test to the fail-stop quarantine semantics#802
kriszyp wants to merge 7 commits into
mainfrom
test/txnlog-tear-quarantine-semantics

Conversation

@kriszyp

@kriszyp kriszyp commented Sep 2, 2026

Copy link
Copy Markdown
Member

Cluster Integration Tests 6/6 has been red on every Node leg since feat: Sync Core b15fceb (2026-08-31), which bumped @harperfast/rocksdb-js 2.7.1 → 2.8.0. 2.8.0 is the first release exporting CorruptFrameError (rocksdb-js#750), and that export was txnlogTearReplication.test.mjs's skip gate: every earlier green run shows the test as # SKIP, so it ran on main for the first time that day and has failed every run since (B holds 39/60 rows). Nothing regressed. The test asserted that replication resyncs past an unreadable mid-log frame and delivers everything behind it; that guarantee was reversed in harper#2087 to fail-stop/quarantine ("stop at the break, and discard the transaction it truncated rather than commit part of it"), whose description already says that harper-pro#670's premise "no longer holds and needs re-scoping to the quarantine semantics". This re-scopes the test to that policy, in harper-pro only. The tear construction was right all along.

Of the two framings the dispatch offered, neither held. Not (a), a regression in the resume path: it never existed on core mainendIteratorOnCorruptFrame latches on any RangeError and never calls next() again, which the 2.8.0 reader's contract leaves to the caller. Not (b), a defective tear: the engine's own report confirms the unreadable shape (113 unreadable bytes = one 13-byte header plus the 100-byte payload, framing resuming at exactly the next frame). The old 60-row oracle could not hold under any design on the table either: the torn row's bytes are unreadable, and un-latching the guard delivers r40…r59 with r39 lost. harper-pro#669 (the readable tear shape) is open and untouched by this change; harper-pro#667 (cluster_status surfacing) is open.

The oracle now pins: B holds exactly the rows whose frames precede the torn one, byte-exact with id-specific delimited payloads, while A still holds every acknowledged row after its restart.

Nothing behind the break replicates, including a write A acknowledges after both nodes restart, over a bounded 10 s window. A's hdb.log reports the break with the torn-frame and resumption offsets, keyed on the engine's message rather than its log level so it holds on main (warn) and under harper#2087 (error).

The repair arm first re-checks B after corrupt A has fully stopped, then repairs the frame and restarts A, and requires the quarantined write to arrive. That makes the repair a causal positive control rather than crediting it for delayed pre-repair delivery.

A missing CorruptFrameError export now fails setup instead of skipping, and the frame→row mapping is checked against every frame's bytes before tearing it. The header sizes come from the engine's exported constants and every direct operation is bounded by an AbortSignal. The review's one-engine-only self-check refers to the RocksDB mention: the on-disk transaction-log framing and CorruptFrameError are rocksdb-js's, and there is no LMDB transaction-log file to tear, so no LMDB arm exists.

Found while adding the repair control, and reported rather than asserted: after the repair, B receives the later transaction but never the remainder of the torn one (r39…r59). B committed the drained prefix with that transaction's version and its resume cursor is now exclusive of it, so a repaired log makes B consistent only for later transactions; the torn transaction comes back only with a re-clone, and nothing reports the gap. That is a replication resume defect adjacent to this test, left for its own issue.

Refs #669, #667, HarperFast/harper#2087, #670.

For the human reviewer

Framing-Verdict: chosen-approach-sound (7f3a95f8a97a). The planning review ran before implementation on a design note whose option set covered core resuming past the break, write-side prevention (rocksdb-js#748), a gate-only fix, deletion, waiting for #2087, and the reviewer-added Pro-triggered base copy after a reported break; each rejected on a stated fact.

  1. The exact-prefix oracle pins partial source-transaction application on B. The torn frame (r39) sits inside the 50-row source transaction, so "exactly r0…r38" pins that B committed r10…r38 of a transaction it never sees the rest of. Alternative: assert only that nothing at or past the break arrives, leaving room for replication to gain boundary tracking. Chose the exact prefix because it is what streaming replication does today and under #2087 (whose atomic discard is the crash-recovery replay arm), and a looser oracle lets a stream that lost acknowledged pre-break rows pass. Reversible by loosening one assertion; the day boundary tracking lands this file goes red and must be re-pointed, deliberately.
  2. The torn transaction's remainder after repair is reported, not asserted. After the repair B receives r60 but never r39…r59 (see the summary). Asserting absence locks in a resume defect; asserting arrival fails today. The count is printed so the shard's stdout shows it, and the defect goes to its own issue. Cheap to flip either way once that issue is decided.
  3. A write after the restart is pinned as quarantined, and the repair arm is its positive control. Step 7 writes r60 once both nodes are back and asserts it never reaches B within 10 s. A then stops and B is re-checked before repair; only then does the repaired restart have to deliver that same row. This deliberately rules out slow pre-repair delivery being credited to repair.
  4. The engine's log line is the contract for "reported". The pass condition is the exact break and resync offsets from rocksdb-js's CorruptFrameError message in A's hdb.log, not a structured surface: cluster_status surfacing (cluster_status reports a stream healthy after it has lost transaction-log entries — nothing consumes getCorruptFrameReports() #667) does not exist yet, so the log line is the only operator-visible signal to pin. When cluster_status reports a stream healthy after it has lost transaction-log entries — nothing consumes getCorruptFrameReports() #667 lands the assertion should move to it.
  5. Fail, don't skip, on a missing CorruptFrameError. The dependency is pinned exactly at 2.8.0 and the diagnostic is part of the asserted contract; a silent skip is how this test spent a week "green" while asserting a reversed design. If the pin is ever loosened to a caret range an older engine makes this a red suite rather than a skip, which is the intended failure mode. Reversible.
  6. One log frame per row is a hard precondition. tearFrame requires exactly 60 frames and checks each frame's bytes against its row, so a future non-row entry in the data/local log fails setup with a message naming both counts rather than silently tearing the wrong frame. The per-frame identity check would catch most of that on its own; the count keeps the failure message plain.
  7. The log oracle is operator-visible, not stream-attributed. The test accepts the CorruptFrameError report from either A incarnation because boot replay writes before the logging root moves. Alternative: require a new report only after B reconnects, which proves the live replication drain logged it. The current scope is the available operator signal; moving to a structured cluster_status signal remains harper-pro#667. A human may prefer the stricter live-stream attribution.
  8. This is a one-database stream test. It proves containment and repair for data, but does not add a healthy second replicated database to distinguish per-stream fail-stop from peer-wide teardown. That extension has a different test topology and is intentionally left out of this rebase; a human may choose to make it a follow-up.

Verification

Route (a): the existing integration test, re-scoped. All runs local with dist rebuilt first, rocksdb-js 2.8.0:

  • Rebase verification (2026-09-03): npm run test:integration -- integrationTests/cluster/txnlogTearReplication.test.mjs — passed 1 suite / 1 test in 75.97 s at 64efd4d0; frame 39 was torn and B again reported 0/21 rows of the torn transaction's remainder after repair. npx prettier --check integrationTests/cluster/txnlogTearReplication.test.mjs and npm run lint:required passed. npm run build emitted dist but exited nonzero on pre-existing TypeScript errors outside this diff in analytics/profile.ts, replication/replicationConnection.ts, and core/server/threads/socketRouter.ts.

  • harper-integration-test-run integrationTests/cluster/txnlogTearReplication.test.mjs — 10 standalone runs green across core 2e65550d7, harper#2087's head 181eac8da (checked out in the worktree's core, built, then restored), and this PR's pointer 9a3c75013; 29–47 s each (the old test failed at 39/60 after 104 s). Torn frame 39 = r39; A's log carries at position 1126 of log 1 … valid framing resumes at 1197, 113 byte(s) unreadable (offsets in hex, as the engine prints them: 0x1126 = 4390); after the repair B holds 0/21 rows of the torn transaction's remainder, every time.

  • Not vacuous: with the compiled endIteratorOnCorruptFrame patched to call next() again after a CorruptFrameError that has a resync position, the test fails at the prefix check with expected exactly 39 rows; missing 0 [], unexpected 20 [r40, …, r59]. Guard restored, dist rebuilt.

  • npm run test:integration:cluster locally: not completed. The suite ran files concurrently on a shared fleet box carrying two other sessions' integration suites (load average 17); four unrelated files failed and six test children then hung with their nodes dead, so the run was killed. Re-run standalone afterwards: addNodeStartTime and copyFinalizeWedgeRecovery pass (contention flakes); cloneReadinessKeySet (QA-762, LMDB leader) and connectedBitRestartChurn (QA-587) fail standalone on this box too, while every cluster shard other than 6/6 was green in the 2026-09-02 nightly, so they are box-specific and unrelated. The Cluster Integration Tests 6/6 shard is the CI evidence for the rest.

  • prettier --check and oxlint --deny-warnings on the file: clean.

Complexity: complicated

Generated with Claude Fable 5.1.

🤖 Generated with Claude Code

https://claude.ai/code/session_01NhscgD5o72hSikQXoEzoR5

Review-Coverage: authored=codex; ran=cursor-composer,gemini,claude; adjudicated=domain; declined=cursor-grok; rounds=2 @ 64efd4d

Human-Review-Need: 3 (decisions: partial-prefix-contract, repaired-remainder-contract, reporting-surface) @ 64efd4d

@kriszyp kriszyp added this to the v5.3 milestone Sep 2, 2026
@kriszyp
kriszyp requested a review from heskew September 2, 2026 18:43

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the integration test txnlogTearReplication.test.mjs to align with a policy change where replication stops and reports a mid-log transaction-log tear instead of attempting to resync past it. The test is refactored to dynamically import constants from @harperfast/rocksdb-js, introduce robust row and payload helper functions, and assert that replication halts at the break, logs the diagnostic, and successfully resumes after the log is repaired. Feedback is provided regarding the breakDiagnostic helper, which uses hexadecimal formatting for offsets; this should be verified against the engine's actual log format to prevent test failures if decimal offsets are used instead.

Comment thread integrationTests/cluster/txnlogTearReplication.test.mjs
@kriszyp
kriszyp marked this pull request as ready for review September 3, 2026 22:22
@kriszyp
kriszyp requested a review from a team as a code owner September 3, 2026 22:22
kriszyp and others added 7 commits September 3, 2026 17:21
… quarantine semantics

The test asserted that replication resyncs past an unreadable mid-log frame and
delivers everything behind it. That guarantee was reversed in harper#2087 to
fail-stop/quarantine, and the test only started running on main when the
rocksdb-js 2.8.0 bump (Sync Core b15fceb) un-skipped it, so the Cluster 6/6
job has been red since.

The oracle now pins the decided policy: B holds exactly the rows whose frames
precede the torn one, byte-exact; nothing behind the break replicates,
including a write A acknowledges after the restart; and A's log reports the
break with the offsets where it sits and where framing resumes. A missing
CorruptFrameError export fails setup instead of skipping, and the frame-to-row
mapping the oracle relies on is checked against each frame's bytes.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NhscgD5o72hSikQXoEzoR5
…indow, check A's table

Review round 1 (Cursor Composer): a restarted node writes boot-time replay lines to
the stopped incarnation's log dir before logging.root is repointed, so the break
diagnostic is looked for in both; the negative window now spans a reconnect cycle;
and A is checked after its restart to show only the log is torn, not the table.
Narrating step comments dropped (Codex nit).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NhscgD5o72hSikQXoEzoR5
…g tear

Adjudication asked for the stated recovery path to be exercised, not only a
negative window. Repairing the torn frame's length field and restarting A now
has to deliver the write that was quarantined before, which also makes the
earlier non-delivery attributable to the stop rather than to latency.

Measured while adding it: the repair does not bring back the remainder of the
torn transaction (r39..r59) -- B's cursor already sits at that transaction's
version and the resume is exclusive of it -- so the test reports that count and
leaves it unasserted instead of locking the loss in. Only a re-clone recovers
those rows today.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NhscgD5o72hSikQXoEzoR5
…ry direct operation

Round 2 (Cursor Composer): the file/entry header sizes were hand-synced to
rocksdb-js; they now come from its exported constants. Direct reads and inserts
carry an AbortSignal so a node that accepts the socket and never answers fails
the step instead of hanging the file until the suite timeout.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NhscgD5o72hSikQXoEzoR5
…on invariant holds everywhere

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NhscgD5o72hSikQXoEzoR5
Co-Authored-By: GPT-5 Codex <noreply@openai.com>
@kriszyp
kriszyp force-pushed the test/txnlog-tear-quarantine-semantics branch from 44a51f3 to 64efd4d Compare September 3, 2026 23:40
@kriszyp
kriszyp marked this pull request as draft September 3, 2026 23:43
@claude

claude Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Reviewed; no blockers found.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant