HarperFast / HarperFast/rocksdb-js

Make TransactionHandle close exclude new work and fully drain in-flight operations

Open
#784 0 comments 0 reactions 0 assignees View on GitHub
bug
Dominant language
C++
Stars
21
Forks
2
Avg merge
2d 9h
Merged PRs (30d)
36

Description

Parent: #741

## Problem

`TransactionHandle::close()` and transaction entry points do not share one lifecycle gate:

- `closed.exchange(true)` is separate from async-work registration, so close can observe zero work and begin teardown immediately before another operation registers.
- The current wait is bounded to five seconds, after which close mutates transaction-log state, releases VT intents, and deletes `txn` even if native execution is still using it.
- `state`, `txn`, `lockedVTSlots`, and `heldTrackers` have cross-thread access without one complete synchronization contract.
- Synchronous `getSync`, `putSync`, `removeSync`, `CommitSync`, and `getCount` remain outside the async-work drain mechanism.

PR #780 must make its new environment-cleanup reap safe for already-running native execution. This issue tracks the complete cross-environment close invariant, including entry points outside the narrow #780 fix.

## Required invariant

After close begins, no new operation may acquire transaction state. Every operation that acquired it earlier must finish before `state`, `txn`, VT intents, log state, or the owning database handle can be mutated or destroyed.

## Acceptance criteria

- Make the closed check and work/resource acquisition atomic with the start of close.
- Reject new operations once close has claimed the handle.
- Do not destroy native state after a timeout while work remains active; defer destruction or wait for definitive native completion.
- Synchronize transaction state and VT intent-vector ownership across close, commit, abort, reset, and release paths.
- Define and enforce the same safety contract for synchronous entry points without imposing an unjustified hot-path cost.
- Add deterministic regressions for:
- environment teardown during a native commit lasting longer than five seconds;
- close racing async work registration;
- close racing representative synchronous reads and writes.

## Relationship

- Umbrella: #741
- #780 should resolve the already-running-work blocker in its new cleanup path.
- Previous partial implementation/reference: #745
- Related teardown symptom: #746

— KrAIs (Codex), on behalf of @kriszyp

Contributor guide

Open the contributing guide

Research direction

Start with TransactionHandle::close and the getSync, putSync, removeSync, CommitSync, and getCount entry points, then compare the cleanup behavior in PR #780 with references #745 and #746. Add deterministic regressions for the listed teardown, registration, and synchronous-operation races. Done means close rejects new work and native state, VT intents, log state, and the owning database handle remain valid until all earlier operations finish.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp, node.js
Domain
backend, database
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
28/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.