HarperFast / HarperFast/harper

Unify the audit replay-floor contract: replication's base-copy decision, the RocksDB dedup guard, and getAuditFloor answer the same question three different ways

Open
#2,593 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
89
Forks
10
Avg merge
2d 6h
Merged PRs (30d)
200

Description

Three places in the codebase now answer the same question — *can a consumer holding cursor `T` be caught up from retained audit history, or does it need a full re-read?* — and each answers it with its own definition of the floor. They agree today only because each one happens to be conservative in the safe direction. Nothing enforces that, and no test compares them.

### The three implementations

| # | Site | Floor it computes | Unknown/empty case |
|---|---|---|---|
| 1 | `harper-pro` `replication/replicationConnection.ts:295` `shouldForceBaseCopyForRetention`, fed by the inline scan at `:3638` | `max(oldest retained entry of the peer's log, Date.now() - auditRetention)`, scanned live, scoped to one log (or all non-excluded logs) | `oldestRetainedTime ?? 0` → falls back to the nominal time cutoff |
| 2 | `harper` `resources/Table.ts:3303` `dedupVersionCouldBeRetained` (on `main`) | oldest retained entry of `options.nodeId`'s log, scanned live; falls back to `Date.now() - auditRetention` | nominal time cutoff |
| 3 | `harper` `resources/auditStore.ts` `getAuditFloor`, exposed as `Table.oldestRetainedAuditTime()` (#2458) | persisted, database-wide floor, raised write-ahead before every prune; on RocksDB it tracks the configured horizon rather than retained reality | `Infinity` — fails closed, no cursor reads as safe |

They differ on four axes that matter:

- **Scope.** 1 and 2 are per-log (per source node); 3 is database-wide. A database-wide floor can be far above a given node's log floor.
- **Durability.** 3 survives restart and covers purges that happened before this process existed. 1 and 2 only see what is on disk at the moment they scan, so a log that was purged to empty and then re-appended reads as fully retained.
- **Fail-closed vs fail-open.** 3 returns `Infinity` when it cannot trust the floor. 1 and 2 degrade to the nominal cutoff, which is a guess.
- **RocksDB fidelity.** 3 is deliberately pessimistic there (whole-log-file purge granularity means the floor must be written before the prune). 1 and 2 read the oldest surviving entry, which on RocksDB is routinely *below* 3's floor.

### Why this is worth fixing, and why the naive fix is wrong

The direction of the divergence is the concern. Sites 1 and 2 can certify a cursor as replayable that the persisted floor says is stale — a fail-open answer relative to the contract #2458 establishes. The dedup guard (2) is explicitly best-effort and converges via the full-copy record, so it is not a data-loss path today; replication (1) is the one where a wrong "yes, replay incrementally" means silently skipping purged entries.

But simply pointing replication at `oldestRetainedAuditTime()` would be a regression in the other direction: the database-wide RocksDB floor would force base copies for peers whose own per-node log is entirely intact. That is a real cost (bounded base copy of a whole database) paid for a floor that overstates what was actually removed. **The scopes have to be reconciled before the call sites are merged**, which is why this is its own issue rather than a follow-up commit on #2458.

### Proposed work

1. **Write down the contract once** — in `auditStore.ts`, next to `getAuditFloor`: what time domain the cursor is in, what scope the answer covers, and which direction the answer is allowed to be wrong in.
2. **Give the shared helper an optional log/node scope**, so it can answer both "is this cursor replayable from *this peer's* log" and "…from anything in this database". The answer should be the max of: the persisted floor (scoped as recorded), the oldest retained entry in the requested scope, and the nominal `Date.now() - auditRetention` cutoff — with the fail-closed `Infinity` preserved when the floor is untrustworthy.
3. **Move all three callers onto it**, keeping site 2's best-effort semantics explicit rather than accidental.
4. **Tests at the boundary**, asserting the same decision from all callers: a cursor exactly at the floor, one just below, after a prune, after a restart, on an empty log, and on a floorless (pre-upgrade) database. Both storage engines — the LMDB and RocksDB floors are derived differently and that is the whole point.

Coordinate with #2448, which makes `Table.subscribe` consume the floor: that is the second core caller and it should land on the shared helper, not on a third variant.

### Related

- #2458 — records the write-ahead audit retention floor (site 3)
- #2448 — consume the floor in `Table.subscribe` for MQTT durable resume
- harper-pro#277 — the replication base-copy-on-retention decision (site 1)

Contributor guide

Open the contributing guide

Research direction

Start with getAuditFloor in resources/auditStore.ts and dedupVersionCouldBeRetained in resources/Table.ts, then compare the replication call site in harper-pro. Read the floor contract from #2458 and coordinate with #2448; done means one scoped helper serves all callers, preserves fail-closed behavior, and boundary tests cover both storage engines and restart or empty-log cases.

Written by the indexing model from the issue text.

Assessment

Tech stack
node.js, typescript
Domain
databases
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.