HarperFast / HarperFast/rocksdb-js
WriteBufferManager stall: runtime allowStall mutation + late-CF size-0 bypass the open-time history clamp
- Dominant language
- C++
- Stars
- 21
- Forks
- 2
- Avg merge
- 2d 9h
- Merged PRs (30d)
- 36
Description
## Summary
A Codex review on [#758](https://github.com/HarperFast/rocksdb-js/pull/758) (raised by @kriszyp) surfaced two concerns in the **write-buffer-manager / retained-history** feature (AGENTS.md invariant 10 — "Retained memtable history must fit the WriteBufferManager budget"). They are unrelated to that PR's background-error change (it only inherits these files via a `main` merge), so filing them here to track separately.
## 1. Runtime `writeBufferManagerAllowStall` mutation bypasses the open-time history clamp
`src/binding/database/db_settings.cpp` (~L209)
`buildColumnFamilyOptions` resolves the derived (`-1`) `max_write_buffer_size_to_maintain` to `0` **only when a stalling manager is configured at open time** (`resolveMaxWriteBufferSizeToMaintain`). A database can attach a WriteBufferManager while `allowStall` is `false`, so its column families keep RocksDB's derived (large) history target. Flipping `writeBufferManagerAllowStall` to `true` at runtime then enables stalling on that same manager **without revisiting those already-resolved CF targets**. With a budget below the retained history, every write to that database can stall permanently — the exact failure invariant 10 exists to prevent.
Related size-`0` mismatch for late-created families: an existing DB retains the manager, while `resolveMaxWriteBufferSizeToMaintain()` consults the *global* size and may re-derive a large history for a CF created after the global size was reset to `0`.
**Suggested direction:** enforce the invariant against the manager actually attached to each descriptor — either make `allowStall` immutable after attachment, or safely update/reject incompatible CF `max_write_buffer_size_to_maintain` targets before enabling stall.
**Coverage gap:** add tests for (a) enabling stall *after* open, and (b) creating a CF after the global size is reset to `0`.
## 2. Stall regression test can hang instead of failing
`test/write-buffer-manager-stall.test.ts` (~L41)
`db.put()` is declared `async` but calls `store.putSync()` before returning its promise. If RocksDB stalls inside that native call, the JS worker thread is blocked and Vitest's 60s timeout can't fire — the regression hangs until the outer CI job timeout rather than failing this test.
**Suggested direction:** run the stall scenario in a worker/child process the parent can terminate on a deadline, or drive it through a genuinely asynchronous native operation so the runner can still enforce its timeout.
---
Reported by @kriszyp's Codex review on #758; filed as a scoped follow-up against the write-buffer-manager work.
Contributor guide
Research direction
Read AGENTS.md invariant 10, then inspect src/binding/database/db_settings.cpp around buildColumnFamilyOptions and resolveMaxWriteBufferSizeToMaintain. Run test/write-buffer-manager-stall.test.ts and trace the db.put() helper around its store.putSync() call. Done means the two reported mutation and late-CF cases are covered without permanent stalls, and the regression test fails within the test runner's deadline.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp, nodejs, typescript
- Domain
- backend, databases, testing
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100