Repository navigation
Corrupt-log containment in RocksTransactionLogStore.getRange is per-drain, so one bad entry head-of-line-blocks a replication stream forever #2063
Description
Activity
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.tsalready 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:
failedIteratorsis constructed insidegetRange(line 264) and keyed on iterator object identity (272, 276), so new iterators on the next drain are unknown to it.excludeLogsarrives on the caller'soptionsobject, 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 seedexcludeLogsfrom 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
excludeLogsat all, so it re-reads a corrupt log on every replay cycle.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
excludeLogsis passed, and nothing calls the removal/addition path that mutates it (RocksTransactionLogStore.ts~371-385). Combined withfailedIteratorsbeing 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 asexcludeLogson eachgetRangecall. That keeps the state in the replay context where it is most meaningful and avoids couplinggetRangeto 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_statuscontinued to reportconnected: truethroughout, 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.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.cppin theMidFileCorruptionbranch: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
MidFileCorruptionclassification untouched; the log in your incident aged out of retention, so it was a rotated file thatrecoverTail()never scans; and a torn tail already never produces this wedge, becauserecoverTail()truncates it onmaintoday 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
excludeLogsexcludeLogsis 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/removeLoghave no production callers in harper or harper-pro today — onlyunitTests/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
errorrather thanwarn, 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,
sizeis not advanced and nothing truncates, andO_APPENDputs 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 tosizebefore 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.
- added 4 commits that reference this issue
on Aug 6, 2026 - added 12 commits that reference this issue
on Aug 19, 2026
Summary
A corrupt transaction-log entry permanently head-of-line-blocks a replication stream, because the corrupt-entry containment added in
RocksTransactionLogStore.getRangeis scoped to a single drain. ThefailedIteratorsWeakSetis constructed insidegetRangeand keyed on iterator object identity, so every freshgetRangebuilds 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):Within one
getRangethis 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 anuncaughtExceptionout of the aggregate iterator).Across calls it provides nothing. The
WeakSetand 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
warn/errorlog spam only.cluster_statusstill shows the connectionconnected: true; there is no "this stream is wedged" signal, so the condition is invisible to monitoring and can persist for days.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
RangeErrorout 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.Filed from a field incident; cluster and host identifiers omitted.