HarperFast / HarperFast/rocksdb-js
DB-instance stress test awaits an empty array instead of worker close acknowledgements
- Dominant language
- C++
- Stars
- 21
- Forks
- 2
- Avg merge
- 2d 9h
- Merged PRs (30d)
- 36
Description
## Problem
The DB-instance stress test does not wait for workers to acknowledge closing their handles. It clears `promises` after the open phase, posts `{ close: true }` to every worker, then awaits `Promise.all(promises)` on the now-empty array. The existing message handler's `closed` branch only resolves the already-settled open-phase promise.
Confirmed by source trace on main `b4d104562e7d5353b6d7a4412e0a8b48ee805062` while profiling #462:
- [`stress-test/db-instances.stress.test.ts`](https://github.com/HarperFast/rocksdb-js/blob/b4d104562e7d5353b6d7a4412e0a8b48ee805062/stress-test/db-instances.stress.test.ts#L39): one promise per worker, resolved by `done` or `closed`.
- [Close phase](https://github.com/HarperFast/rocksdb-js/blob/b4d104562e7d5353b6d7a4412e0a8b48ee805062/stress-test/db-instances.stress.test.ts#L57): clears the array and never adds close-phase promises.
- [`stress-db-instances-worker.mts`](https://github.com/HarperFast/rocksdb-js/blob/b4d104562e7d5353b6d7a4412e0a8b48ee805062/stress-test/workers/stress-db-instances-worker.mts#L19): the worker actually closes 12,500 handles before sending `closed`.
The subsequent two GC calls and 150 ms of sleeps are the only delay before the ref-count assertion. This makes completion depend on machine timing and weakens the test as evidence for teardown synchronization. Worker exit before `done` also has no rejection path; several stress helpers have the same omission and rely on the outer test timeout.
## Observed run and scope
`node --expose-gc ./node_modules/vitest/vitest.mjs --run --config vitest-stress.config.ts` passed 9/9 on macOS arm64 / Node 24.16.0. This report is a confirmed test-harness synchronization defect, not a reproduced native crash or data-integrity failure. The erroneous empty-array await is present regardless of that passing result.
## Proposed correction
Install a distinct close-acknowledgement promise for each worker before posting the close message; reject on worker error or unexpected exit, and terminate/join workers in `finally`. Keep the final ref-count assertion, but make it follow confirmed close completion. Verify with a controlled delayed-close worker that the parent remains pending until the acknowledgement, and with an early-exit worker that it fails promptly.
Priority rationale: P2. Contained test reliability/coverage defect in the concurrency qualification path; no confirmed production impact. No release target or owning epic found; no customer or security context.
Contributor guide
Research direction
Start with stress-test/db-instances.stress.test.ts and trace its worker message handling and close phase, then read stress-test/workers/stress-db-instances-worker.mts to understand the closed acknowledgement. Make completion wait for each close acknowledgement and cover delayed-close and early-exit cases, while preserving the final ref-count assertion and worker cleanup.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- node.js, typescript
- Domain
- testing-qa
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 62/100