HarperFast / HarperFast/rocksdb-js

DB-instance stress test awaits an empty array instead of worker close acknowledgements

Open
#859 0 comments 0 reactions 0 assignees View on GitHub
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

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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.