HarperFast / HarperFast/rocksdb-js

Transaction-log iterator reports a purged-segment gap as end-of-log, so a lagging consumer cannot tell divergence from being caught up

Open
#805 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
21
Forks
2
Avg merge
2d 9h
Merged PRs (30d)
36

Description

## Summary

`TransactionLog.query()`'s iterator returns `{ done: true }` when the next sequence file cannot be mapped. That is byte-identical to what it returns on reaching the end of the log, so a consumer that has fallen more than the retention window behind reads "no more entries" for a range that was reclaimed — with no error, and no way to distinguish the two.

- [`src/transaction-log-reader.ts:396-402`](https://github.com/HarperFast/rocksdb-js/blob/main/src/transaction-log-reader.ts#L396-L402) — the advance path, on rolling off the end of the current segment.
- [`src/transaction-log-reader.ts:236-244`](https://github.com/HarperFast/rocksdb-js/blob/main/src/transaction-log-reader.ts#L236-L244) — the initial-map path; its comment already names "purged" as one cause.

## The reader can already tell them apart

At the advance branch, `loadLastPosition()` has just supplied `latestLogId`, and the branch is only entered when `latestLogId > logBuffer.logId`. So "the segment I need is gone, but a newer one exists" is distinguishable from "nothing newer has been written yet" with no extra syscall and no new cache machinery.

## What this is *not*

Worth stating, because it is the intuitive first guess and it is wrong: **an unlinked-but-mapped segment is safe.** The inode outlives the unlink until the last mapping drops; `_logBuffers` holds `WeakRef`s ([`transaction-log-reader.ts:428-455`](https://github.com/HarperFast/rocksdb-js/blob/main/src/transaction-log-reader.ts#L428-L455)), and the single strong reference (`_currentLogBuffer`) is only taken when `latestLogId === logId` — the newest segment, which retention never makes eligible. Nothing pins a purged inode, a reader finishes what it has mapped, and space is reclaimed on unmap. No cross-worker cache invalidation is required.

The gap is strictly the segment a lagging consumer **has not mapped yet**: `open()` on an unlinked path is `ENOENT`, and there is nothing for inode semantics to preserve.

## Impact

Harper shares one reusable iterator per (database, peer) for replication catch-up. A peer offline longer than `logging.auditRetention` resumes, reaches the purged prefix, gets `done`, and is treated as caught up — a silently divergent replica. Harper records the gap as a metric (`txnlogReplayGapBytes`) but nothing escalates to a full copy, so the divergence is unbounded until someone notices out-of-band.

Reachable today via the boot purge (a restart after a peer has been down longer than retention), but rare because Harper's RocksDB retention is effectively one-shot per boot — HarperFast/harper#2140. It becomes structural with HarperFast/harper#2338, which puts retention on a continuous cadence; that PR carries this as an open pre-merge decision.

This is **not** closed by #799. That change keeps `txn.state`, its cached handle, the writer, and the contiguous reader in one sequence space, which bounds retention against the *writer* side. It does not bound against an arbitrarily lagging *consumer*, so this survives it.

## Suggested fix

1. **Signal the discontinuity** — return a marker, or set a flag on the iterator, when the advance target is missing while a higher segment exists. The consumer can then escalate to a checkpoint/full copy instead of concluding it is caught up. Smaller change, and the information is already in hand at the branch.
2. **Skip to the oldest surviving segment** instead of hard-stopping at `logId + 1`, letting the consumer detect the sequence jump. Needs a "next existing logId >= x" primitive that does not exist today.

(1) is preferred: it gives replication an unambiguous trigger without changing what the iterator yields on the happy path.

A consumer that far behind should be recovering by full copy rather than log replay — so the ask here is an honest end-of-stream reason, not an interlock that holds retention for lagging readers.

Contributor guide

Open the contributing guide

Research direction

Start in src/transaction-log-reader.ts at the initial-map path around lines 236-244 and the advance path around lines 396-402; trace how query() handles a missing next sequence file after loadLastPosition() supplies latestLogId. Review the iterator and replication call path to define a distinct purged-gap outcome. Done means a lagging consumer can distinguish divergence from a normal end-of-log without changing the happy-path entries.

Written by the indexing model from the issue text.

Assessment

Tech stack
node.js, typescript
Domain
databases, distributed-systems
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.