HarperFast / HarperFast/rocksdb-js

readOnly:true open races a live writer's compaction and reports a healthy database as corrupt; no OpenAsSecondary alternative exists

Open
#812 1 comment 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.