Uncommitted transaction-log reads bound corruption checks by mapped capacity, so a torn frame with a plausible length goes undetected #749

Summary

An uncommitted transaction-log read bounds its per-entry corruption checks by the mapped capacity of the memory map rather than by the written extent. The map is pre-extended far past the data, so a torn frame only trips the check when its declared length exceeds that whole capacity. Any smaller garbage length is accepted, and the reader advances position by it — walking over valid entries, or handing back a misframed one, without ever reporting corruption.

Where

src/transaction-log-reader.ts, in the query() iterator:

const limit = readUncommitted ? logBuffer!.length : Math.min(size, logBuffer!.length);
if (position + TRANSACTION_LOG_ENTRY_HEADER_SIZE > limit) { /* corrupt */ }
const length = dataView.getUint32(position + 8);
if (position + TRANSACTION_LOG_ENTRY_HEADER_SIZE + length > limit) { /* corrupt */ }

For a committed read limit is the committed watermark, which is a true entry boundary, so the checks are tight. For readUncommitted it is logBuffer.length — the mapped capacity.

Why it matters

readUncommitted is the boot-replay path (Harper's replayLogs.ts reads with startFromLastFlushed: true, readUncommitted: true), which is precisely the reader that has to cope with a log damaged by an interrupted append.

The field error in HarperFast/harper#1977 declared a length of 1,688,494,450 — comfortably past any mapped capacity, so it was caught. A partial write that leaves a plausible-looking smaller length is not: the frame is silently accepted, and iteration continues from a position that is no longer an entry boundary.

This also gates corrupt-frame resync (HarperFast/harper#2016, #2063): recovery can only run where detection fires, so on this path resync is inert for the class of corruption whose declared length happens to land under the mapped capacity.

Why it is not simply tightened

The looser bound is deliberate — an uncommitted reader is allowed to read past the size it captured at query time, because a concurrent append may have grown the file since. Replacing logBuffer.length with the extent captured at query start would report healthy, concurrently-appended entries as corrupt.

Fetching the current extent per entry is also not acceptable: getLogFileSize crosses into native and takes the store mutex, so it must not sit on the healthy read path (this was measured as a regression in PR review and moved off that path).

Suggested direction

Check against a cached written extent and refresh it only on an apparent overrun:

1. bound the checks by min(cachedWrittenExtent, logBuffer.length), seeded once per query();
2. when an entry appears to overrun that bound, re-read getLogFileSize once and re-test before declaring corruption;
3. only report a corrupt frame if it still overruns.

A concurrent append then costs one refresh rather than a false corruption report, the healthy path costs nothing per entry, and detection becomes as tight on the uncommitted path as it already is on the committed one.

Related

• A failed transaction-log append orphans its partial bytes, baking a mid-file framing break into the log #748 — the write-side producer of these mid-file breaks
• Mid-log corrupt transaction-log frame silently truncates replay and replication — acknowledged writes lost harper#2016, Corrupt-log containment in RocksTransactionLogStore.getRange is per-drain, so one bad entry head-of-line-blocks a replication stream forever harper#2063 — the reader-side consequences

🤖 Generated with Claude Code