Corrupt-log containment in RocksTransactionLogStore.getRange is per-drain, so one bad entry head-of-line-blocks a replication stream forever

[heskew]

Summary

A corrupt transaction-log entry permanently head-of-line-blocks a replication stream, because the corrupt-entry containment added in RocksTransactionLogStore.getRange is scoped to a single drain. The failedIterators WeakSet is constructed inside getRange and keyed on iterator object identity, so every fresh getRange builds new iterators, re-reads the same corrupt log, and re-throws. There is no durable exclusion, no recovery, and no health signal — the stream simply never advances past the bad entry.

Observed on 5.1.x: the same corrupt log re-erroring on a fixed cadence for five days while replication for that database made no forward progress.

Mechanism

resources/RocksTransactionLogStore.ts (getRange):

const failedIterators = new WeakSet<IterableIterator<TransactionEntry>>();   // per-call
...
const safeNext = (iterator, log?) => {
    if (failedIterators.has(iterator)) return { value: undefined, done: true };
    try { ... } catch { failedIterators.add(iterator); ... }
};

Within one getRange this behaves as intended: the corrupt log is marked failed, subsequent retry-polls skip it, other peer logs keep draining, and the worker stays healthy (the original goal — avoiding an uncaughtException out of the aggregate iterator).

Across calls it provides nothing. The WeakSet and the iterators both die with the call, so the next drain re-opens the same log, hits the same entry, and logs the same error. Combined with the replay guard — which stops iteration at a corrupt entry rather than skipping or excluding the log — nothing after the bad entry is ever delivered.

Impact

• Replication for the affected database stops advancing permanently. Records written after the corrupt entry never reach the peer.
• The failure is warn/error log spam only. cluster_status still shows the connection connected: true; there is no "this stream is wedged" signal, so the condition is invisible to monitoring and can persist for days.
• Recovery today is manual and requires knowing to look at the transaction-log files at all.

Expected

Corrupt-log exclusion should survive beyond a single getRange: track the failed (log, position) durably for the store so later drains skip it, and/or advance past the corrupt entry rather than stopping. Either way the condition needs to surface as a health signal — an operator should not have to grep logs to discover that a replication stream has been dead for days.

Prior discussion

This exact gap was raised in review on the PR that introduced the containment ("the aggregate iterator should also remove/exclude the corrupt log so later drains do not keep retrying it"). That PR was later closed, and only the per-call containment landed via a separate commit, so the durable-exclusion half was never implemented.

Related

• Torn audit entry throws RangeError out of transaction-log iteration instead of degrading #1977 — torn audit entry throws RangeError out of transaction-log iteration instead of degrading. Complementary: that asks the audit reader to degrade the way the replay reader already does. This issue is about the replay reader's degradation being non-durable — it stops cleanly, but forever.
• Transaction-log replay: corrupt length-prefix entry throws uncaught RangeError and aborts startup instead of being skipped #1135 — corrupt length-prefix entry aborting startup (closed; introduced the stop-at-corrupt-entry behavior).

Filed from a field incident; cluster and host identifiers omitted.

[heskew]

A concrete fix path already exists in the file — an outside review pointed this out and it checks out at v5.2.0.

resources/RocksTransactionLogStore.ts already has a log-exclusion mechanism: options.excludeLogs?: string[] (line 229), consulted when selecting logs to drain (line 286) and actively mutated during iteration (lines 371-385, splice/push by log name). So "exclude this log from draining" is already an expressible concept here.

The gap is purely durability of scope. Both pieces of state are per-call:

• failedIterators is constructed inside getRange (line 264) and keyed on iterator object identity (272, 276), so new iterators on the next drain are unknown to it.
• excludeLogs arrives on the caller's options object, so whatever is pushed at 385 lives only as long as that call's options.

That suggests a smaller fix than re-architecting the guard: persist the failed-log identity at store scope (rather than per-getRange) and seed excludeLogs from it, so a log known to fail framing is skipped on subsequent drains instead of re-read. A health signal on that persisted set would also close the "no alarm" half of this issue.

Worth noting the boot replay path does not appear to use excludeLogs at all, so it re-reads a corrupt log on every replay cycle.

[heskew]

Verified: the boot replay path never excludes a failed log.

resources/replayLogs.ts:95:

for (const auditRecord of txnLog.getRange({ startFromLastFlushed: true, readUncommitted: true }) as any) {

No excludeLogs is passed, and nothing calls the removal/addition path that mutates it (RocksTransactionLogStore.ts ~371-385). Combined with failedIterators being constructed per-getRange (line 264), a log that fails framing is re-read on every replay cycle indefinitely.

An alternative fix worth considering, raised in outside review: rather than persisting failed-log identity at store scope, have the caller (replayLogs.ts) own a failed-log set and pass it as excludeLogs on each getRange call. That keeps the state in the replay context where it is most meaningful and avoids coupling getRange to store-level state. Either shape closes the gap; the caller-side version is the smaller change.

On severity — I would resist framing this as merely wasted retries. In the field this produced 11 days of replication divergence between two nodes that never self-corrected: cluster_status continued to report connected: true throughout, and the condition was only discovered because an unrelated deploy failed with a misleading error. It eventually cleared only when the affected log aged out of retention. The absence of any health signal is as much the problem as the retrying.

[kriszyp]

Took a run at this. Short version: #723 is not the root fix, the durable-exclusion remedy as specified would make things worse, and the real fix needs a change in each repo. PRs are up — rocksdb-js#750 and #2087.

Why #723 isn't it

Same file and same recovery scan, but a disjoint failure mode. #723 repairs the tail of the active file; this is a mid-file break, which rocksdb-js deliberately declines to repair. From transaction_log_file.cpp in the MidFileCorruption branch:

Leave the file intact: entries are still framed after the break, so truncating would discard committed/replicated transactions. […] Reads past this point will fail until the file is repaired.

And recovery only ever scans one file (transaction_log_store.cpp):

only the active (highest-sequence) file can carry a torn partial write […] rotated files are immutable and already complete

Three things settle it: #723 changes no reader code at all and leaves the MidFileCorruption classification untouched; the log in your incident aged out of retention, so it was a rotated file that recoverTail() never scans; and a torn tail already never produces this wedge, because recoverTail() truncates it on main today and the reader then stops cleanly on the zero timestamp.

One interaction worth knowing: #723 makes committed reads reach the recovered tail instead of a stale flushed position, so a pre-existing mid-log break becomes reachable at boot rather than after the next commitFinished(). Not a regression — it surfaced one commit later before — but this gets more visible once #723 lands.

The hazard in durable excludeLogs

excludeLogs is keyed on the log name — an entire per-node stream. 'local' is this node's own writes, which is what the corrupt log was in your incident. Durably excluding it stops every future entry on that stream from replicating: one lost frame becomes a permanently dead stream. It also recovers nothing, since everything after the break was already unreachable.

(Also: excludeLogs/addLog/removeLog have no production callers in harper or harper-pro today — only unitTests/resources/auditLog.test.js. Expressible, but untested as a corruption control.)

What the PRs do

The permanence is engine-side: the reader throws at the break and cannot advance, so the entries after it are unreachable forever — which is #2016, the data-loss twin of this issue. The resync machinery already existed and was being discarded: validFramingResumes() finds where framing resumes (8 consecutive frames, or a chain landing exactly on the end) but is used only to classify, throwing the offset away.

So: rocksdb-js reports the break with a resume offset and leaves its reader positioned there; harper resyncs past it, logs a mid-log break at error rather than warn, and accumulates breaks in a report keyed by (store path, log, file, offset) for the health signal — your second half. A 12-entry log with a broken second frame goes from 1 entry delivered to 11.

Your severity framing drove a real fix, by the way. My first cut keyed severity on whether the pass resynced, so a break that merely hit the resync cap logged with the benign torn-tail warn — re-hiding exactly the condition this issue is about. It keys on the break's own shape now.

Two things these PRs do not do

They don't stop the tear. The write-side producer is still live and is not #572 (that one was fixed in 2.0.0/1.4.1, well before your 5.1.x incident). A failed append leaves the bytes it already wrote on disk, unreported, size is not advanced and nothing truncates, and O_APPEND puts the next entry after them — baking a break into a file that later rotates and is never rescanned. Filed as rocksdb-js#748; the fix looks like truncating back to size before throwing.

No end-to-end proof. Nothing yet reopens a genuinely damaged log and watches a real consumer replicate past the break. Related: on the boot-replay path, detection is still bounded by the mmap's mapped capacity, so a torn frame with a plausible-looking length isn't detected at all — rocksdb-js#749. Replication broadcast uses committed reads and isn't affected.