HarperFast / HarperFast/harper
Table.deleteHistory() on RocksDB scans the whole audit range and reports a false nonzero entriesDeleted
- Dominant language
- JavaScript
- Stars
- 89
- Forks
- 10
- Avg merge
- 2d 6h
- Merged PRs (30d)
- 200
Description
## What happens
`Table.deleteHistory(endTime)` on a RocksDB-backed table returns a **nonzero `entriesDeleted`** for a call that removed nothing, and spends CPU proportional to the audit log doing it.
`RocksTransactionLogStore.remove()` is an `async` no-op (`resources/RocksTransactionLogStore.ts:554-557`, carrying a TODO that it "can likely be removed once the call to purgeLogs() is added"). `Table.deleteHistory` (`resources/Table.ts`, the audit-range scan inside `deleteHistory`) still iterates `auditStore.getRange({ start: 1, end })` over the entire matching range, calls `removeAuditEntry` for every entry of this table — which bottoms out in that no-op — and increments `entriesDeleted` on each "success". The code's own comment says a RocksDB `deleteHistory` "removes nothing and must not claim it did"; the return value claims it did.
## Why it matters
- **Silent false success from a public method.** `Table.deleteHistory()` is a static method on every table, reachable from application code and custom resources. A caller on the default engine believes history was removed when none was, and gets a count that looks like proof.
- **Wasted work on large logs.** The full-range scan and per-entry no-op removals scale with the database's audit log, for no effect.
The operations API is already guarded: the bridge rejects table-scoped `delete_audit_logs_before` / `delete_transaction_logs_before` with a 400 on RocksDB (#2049). This is the direct-method path that guard does not cover.
## Scope
Pre-existing on `main` (the same `entriesDeleted++` is there), not introduced by #2458 — that PR only added the floor gating around it (`if (!isRocksDB)` for the floor raise, while the scan remains). Surfaced by the cross-model review of #2458; the adjudicator re-scoped Gemini's finding to pre-existing with disposition "separate issue".
## Suggested fix
On RocksDB, skip the audit-range scan entirely and either return `0` or throw the same "not supported for RocksDB" error the bridge uses — so the count is never a lie and no work is done for no effect. Optionally delete the no-op `remove()` per its TODO once nothing calls it.
## Related
- #2458 — audit retention floor infrastructure (where this was found)
- #2049 — the bridge-level guard for the operations API
- #2448 — subscribe-side consumer of the floor
Contributor guide
Research direction
Start in resources/Table.ts at the audit-range scan inside deleteHistory and compare its RocksDB behavior with resources/RocksTransactionLogStore.ts:554-557 and the operations API guard for #2049. Verify the direct method path on RocksDB, then ensure it does not scan or report deleted entries when no history is removed, using the return or error behavior selected by the issue.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- node.js, typescript
- Domain
- databases, performance
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 68/100