HarperFast / HarperFast/rocksdb-js
readOnly:true open races a live writer's compaction and reports a healthy database as corrupt; no OpenAsSecondary alternative exists
- Dominant language
- C++
- Stars
- 21
- Forks
- 2
- Avg merge
- 2d 9h
- Merged PRs (30d)
- 36
Description
## Summary
`RocksDatabase.open(path, { readOnly: true })` maps to [`rocksdb::DB::OpenForReadOnly`](https://github.com/HarperFast/rocksdb-js/blob/main/src/binding/database/db_descriptor.cpp#L1308). That call is **not safe against a database another process is actively writing**: it replays the MANIFEST to build the SST file list and then opens each file, holding no reference on any of them. A compaction in the writing process can unlink an input SST between those two steps, and the open then fails with a message that blames the MANIFEST:
```
Corruption: IO error: No such file or directory:
While open a file for random read: /000046.sst: No such file or directory
The file /MANIFEST-000005 may be corrupted.
```
**Nothing is corrupt.** That wording is RocksDB's generic text for *a file named in the MANIFEST was not there when I opened it*. The database is healthy; the reader simply raced a compaction.
Two problems, one root:
1. **The error misdiagnoses a healthy database as corrupt.** This is the part that can cause harm: an operator who reaches for a read-only open during an incident, sees "MANIFEST may be corrupted", and starts a repair or a restore is acting on a false report against intact data.
2. **There is no safe alternative in this library.** RocksDB's API for a follower reader that tolerates the primary deleting files is `DB::OpenAsSecondary` (plus `TryCatchUpWithPrimary()` to advance it). `grep -rn "OpenAsSecondary\|secondary" src/` returns nothing — the option is not exposed at any layer.
## Why this is not just a test concern
`harper` ships an operator-facing read-only mode. [`resources/databases.ts:364-368`](https://github.com/HarperFast/harper/blob/main/resources/databases.ts#L364-L368) sets `options.readOnly = true` whenever `isReadOnlyMode()` is true, which is driven by `HARPER_READONLY=1` or a `--readonly` / `--read-only` argv flag. Nothing stops that being pointed at a data directory a live Harper is writing — "bring up a read-only instance against the live data dir and look at it" is a natural incident move, and it is exactly the case that races.
## Reproduction
Deterministic-ish, no Harper needed:
1. Open a database read-write and write enough to produce several SSTs.
2. Start continuous write + `flush()` traffic so compactions run.
3. From a second handle or process, loop `RocksDatabase.open(path, { name, readOnly: true })` / `close()`.
The open fails intermittently with the message above. Flushing immediately before the read-only open makes it far more likely, because a flush creates new L0 files and that is what triggers the L0→L1 compaction the open then races.
Observed in CI at scale: `harper`'s `integrationTests/database/delete-index-atomicity-rocksdb.test.ts` opens a second read-only handle as a verification oracle, calls `POST /Flush/` immediately before each reopen, and hit this in **7 of 15** Integration runs on `harper` `main` (~47%) within hours of landing — across Node v22, Node v26.5.0, Bun and the uWS HTTP variant, so it is not runtime-specific. That test is being changed separately to stop losing this race; this issue is about the library.
## Affected
- `@harperfast/rocksdb-js` 2.8.0 (current) and, as far as I can tell, every earlier version — the `OpenForReadOnly` call has no file-deletion protection at any point in its history.
## Docs understate it
Neither the README nor the TSDoc warns that a read-only open is unsafe against a live writer. [README](https://github.com/HarperFast/rocksdb-js/blob/main/README.md#L109-L114) says only that read operations are permitted and writes throw `ERR_DATABASE_READONLY`; [`src/store.ts:283-289`](https://github.com/HarperFast/rocksdb-js/blob/main/src/store.ts#L283-L289) says the same. Neither mentions that the open is a point-in-time snapshot, nor that it can fail spuriously against a concurrent writer.
## Suggested remedy
1. **Expose `OpenAsSecondary`** — a `secondary: true` open mode (with its required secondary-instance path) plus a `catchUpWithPrimary()` binding for `TryCatchUpWithPrimary()`. This is the actual fix: secondary mode is designed for a live follower reader and tolerates the primary obsoleting files.
2. **Stop reporting a missing SST as MANIFEST corruption.** When `OpenForReadOnly` fails with an `IsIOError` naming a missing `.sst`, surface a distinct, accurate error — something like `ERR_CONCURRENT_COMPACTION` / "a concurrent compaction removed a file this read-only open needed; retry, or open as a secondary" — instead of passing RocksDB's corruption wording through. Whatever else is decided, an operator should never be told a healthy database may be corrupt.
3. **Document the hazard** on the `readOnly` option in both the README and the TSDoc: safe against a quiescent database, unsafe against a live writer, use secondary mode for that.
(1) and (2) are independent; (2) is small and worth doing even if (1) is deferred.
Contributor guide
Research direction
Start with the OpenForReadOnly path in src/binding/database/db_descriptor.cpp and the README and TSDoc references in src/store.ts; review the reported secondary-related search and reproduction. Use harper's integrationTests/database/delete-index-atomicity-rocksdb.test.ts as a concurrency reference. Done should distinguish a missing SST during live compaction from corruption and define a safe documented path for live-writer readers.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp, node.js
- Domain
- database
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100