test(cluster): prove mid-log txnlog tear recovery end-to-end

[kriszyp]

End-to-end coverage for harper#2016 — Mid-log corrupt transaction-log frame silently truncates replay and replication — acknowledged writes lost and harper#2063 — Corrupt-log containment in RocksTransactionLogStore.getRange is per-drain.

Why this exists

Both fix PRs — harper#2087 and rocksdb-js#750 — state the same coverage ceiling in their own descriptions: every test drives synthetic iterators or hand-built buffers, and nothing reopens a genuinely damaged log on disk and watches a real consumer replicate past the break. This is that test.

What it does

Two nodes. B's resume cursor is established on a healthy stream (10 rows), B goes offline, 50 more rows are written to A and acknowledged, then A is stopped and a frame 20 frames from the end of its data/local log is torn on disk. Both nodes come back and B resumes from its pre-tear cursor.

The oracle is the last row written, not a count: a stream that stopped at the tear never delivers the tail, however many rows it managed first.

Measured

Engine	Rows on B
rocksdb-js 2.7.0 (released)	39 / 60	fails — stops at the torn frame
rocksdb-js #750 build	60 / 60	passes — resyncs, nothing lost

That is the fails-on-base check: the test reproduces the defect on today's released engine and passes on the fixed one, so it is not vacuous.

Worth a reviewer's attention

It skips on a released engine. The gate is the CorruptFrameError export, which only the fixed engine has. Until the rocksdb-js bump lands, this is a skip in CI — deliberate, so it can merge ahead of the engine, but it means CI green here is not evidence the test runs. HARPER_TXNLOG_TEAR_FORCE=1 runs it anyway, which is how the base column above was produced.

It tears the frame by rewriting the declared length, not by truncating. Framing is walked from the file header using the 13-byte header layout, duplicated as constants from rocksdb-js's transaction_log_file.h. If that layout changes, this test breaks at the magic-number assertion rather than silently testing nothing.

It covers the unreadable tear shape only, and that is the narrower half. A partial append usually leaves a readable torn frame — the declared length overruns into the next frame but still fits the file — which the reader yields as a well-formed entry with a garbage payload. Framing recovery cannot help there. Measured on this same harness, that shape leaves B at 39/60 even on a fixed engine, wedged on repeated decode failures rather than on the framing break. Filed as #669 — A readable torn transaction-log frame poisons the replication stream. Reviewers should read that before treating #2016/#2063 as closed by the two fix PRs.

Independent pre-push review ran at the current head. Claude and Gemini completed the final delta review. The review retained a semantics question: the upstream framing fix says the broken frame itself can be lost, while this harness previously measured 60/60 on the fixed engine and the Aug 21 review asks to pin that observed zero-loss behavior.

For the human reviewer

decision — tear-shape-scope — cover the unreadable shape and file the readable one vs block this test until the readable shape also passes — reversible; the alternative leaves the proven half of the fix unproven while #669 is designed

decision — engine-gate-by-export — gate on the CorruptFrameError export vs a version comparison on the installed engine — reversible; the export is the actual capability, a version string is a proxy that a local link or prerelease breaks

Generated with Claude Opus 5.

🤖 Generated with Claude Code

_{Review-Coverage: authored=codex; ran=claude,gemini; declined=cursor-grok,cursor-composer,domain; rounds=2 @ a42e810}

_{Human-Review-Need: 4 @ a42e810}

[gemini-code-assist]

Code Review

This pull request adds a new integration test, txnlogTearReplication.test.mjs, to verify that replication resyncs past a mid-log transaction-log tear instead of stalling. The review feedback suggests wrapping individual cleanup steps in separate try-catch blocks within the after hook to ensure that a failure in one step does not prevent subsequent cleanup steps from executing.

[claude]

Delta review — commits 0c1e2fe + a42e810 (since last review 2026-08-07)

One file changed (integrationTests/cluster/txnlogTearReplication.test.mjs), two post-review commits. Prior review found no blockers; this pass covers what changed.

---

What the two commits do

0c1e2fe (Address txnlog tear test review feedback)

1. Tightened the tearFrame precondition from frames.length > framesFromEnd + 2 to frames.length > BATCH_ONE + framesFromEnd + 2. This is strictly correct — the old guard only verified there were enough frames total; the new guard verifies there are enough frames past B's resume cursor, which is what the test actually requires.
2. Updated the skip-gate comment from resyncPosition to CorruptFrameError, matching the real export name that is now tested.
3. Changed waitForRows(ctx.nodeB, TOTAL - 1) to waitForRows(ctx.nodeB, TOTAL) and missing.length <= 1 to missing.length === 0. This pins zero-loss semantics rather than allowing one row to be lost to the torn frame.
4. Expanded cleanup from .catch(() => {}) (Gemini's inline comment) to separate try/catch {} blocks so each step is attempted independently.

a42e810 (Log txnlog tear cleanup failures)
Expanded the silent catch {} in cleanup to catch (error) { console.error(…) }, making teardown failures diagnosable in CI logs.

---

Findings

1. Zero-loss assertion — decision confirmation requested (advisory)

The PR description explicitly flags this as the key open decision: "the upstream framing fix says the broken frame itself can be lost, while this harness previously measured 60/60 on the fixed engine and the Aug 21 review asks to pin that observed zero-loss behavior."

The final assertion is:

ok(
    missing.length === 0,
    `resync should preserve every acknowledged row, but B is missing ${missing.length}: ...`
);

This includes the row in the torn frame — the one whose payload bytes the test rewrites. The rocksdb-js #750 PR description says the engine reports where framing resumes after a tear (so entries after the break are recovered), but does not guarantee the torn frame's own payload is reproduced. If the implementation skips the broken frame's payload (which would be semantically correct for a framing-recovery-only fix), this assertion fails on a correctly-fixed engine.

The comment added in 0c1e2fe reads "including the row in the torn frame" and the author's measured result is 60/60. That is consistent with the engine performing a full-copy resync for the torn segment — which would recover the torn frame's row from the receiving node's perspective. This is a legitimate design decision — the test is pinning a measured, desired behavior — but it should receive explicit human reviewer acknowledgement before merge, because it encodes an assumption about the engine's resync mechanism that is not obvious from the framing-fix spec alone.

2. readFrames treats zero float64 timestamp as end-of-log (low, informational)

readFrames breaks when view.getFloat64(position) === 0. The comment says this is a "zero-timestamp end-of-entries marker." If rocksdb-js ever writes a valid entry with a Unix epoch timestamp (zero), the walker would stop early. In practice this cannot happen on a node that is at least a few seconds old, and the log is read while A is stopped and quiescent, so no race is possible. No action required.

3. Cleanup order: A is stopped before B (informational)

The after() loop stops A then B sequentially. Between those two stops, B's replication client will attempt a reconnect to A and generate connection-refused log noise. This is cosmetic only and consistent with other cluster tests in this repo.

---

Status

No blockers. The Gemini inline comment about cleanup silently swallowing errors is fully resolved — failure logging is now explicit and per-node. The tearFrame precondition tightening closes a real gap. The one item asking for human attention is the zero-loss assertion semantics (finding 1 above), which is already documented in the PR body and is a reviewer decision, not a defect.

_{Reviewed at head a42e810 (merge commit 3fad0f6). Changed files: 1. Findings: 1 advisory (decision confirmation), 2 informational.}

[ldt1996]

The engine gate holds up: CorruptFrameError is merged and exported on rocksdb-js main, so this arms itself on the next pin bump, and the header constants match transaction_log_file.h. The measured 39/60 on the released engine is arithmetically consistent with the tear position (60 minus the 20 trailing frames minus the torn one), which corroborates the placement. Both decision points: accept; the export is the capability, and covering the unreadable half now with #669 tracking the readable half is right, though #669 should stay the blocker on calling #2016/#2063 closed.

Two asks:

1. The frame-count guard permits a vacuous pass: frames.length > FRAMES_AFTER_TEAR + 2 lets the torn frame sit inside the batch-one prefix if the log ever comes up shorter than expected, and a tear behind B's cursor is never read, so the test would pass without exercising resync. frames.length > BATCH_ONE + FRAMES_AFTER_TEAR + 2 pins the tear past the cursor.

2. The final assertion allows missing.length <= 1, but your measured fixed-engine run was 60/60. If the fixed engine reliably re-delivers the torn frame's row, assert 0 so the test pins it; if the torn row is legitimately lost on the replica, a comment should say what eventually heals it. As written the bound quietly codifies losing one acknowledged row.

Nit: the doc comment says the gate is resyncPosition; the code gates on CorruptFrameError.

Lavinia, via Claude (I reviewed this before posting)

[kriszyp]

Addressed the Aug 21 review asks in 0c1e2fe/a42e8105:

• the frame-count guard now requires enough frames to place the tear beyond BATCH_ONE plus the preserved tail;
• convergence waits for all 60 rows and the final oracle requires zero missing acknowledged rows;
• the suite doc now names the actual CorruptFrameError capability gate;
• stop/teardown cleanup steps are independently guarded and log failures.

Local checks: lint and 626 unit tests pass. The focused integration entry is capability-skipped on the pinned rocksdb-js 2.7.0, as expected; the full cluster gate was attempted but could not complete on this shared worker because the loopback pool was occupied.