Bound a request-path commit's conflict retries to its queue-time budget

[kriszyp]

A request-path RocksDB commit that keeps losing write-intent conflicts now stops at its queue-time budget instead of running the full 40-attempt cap.

rocksdb-js 2.8 wakes a commit parked on another transaction's write intent after ROCKSDB_JS_PARK_TIMEOUT_MS (5s) and returns RETRY_NOW_VALUE even when the holder never releases. That turned the "commit that never settles" of #2450 into a stream of transient conflicts — but the attempt cap alone still keeps the initiating request pending for roughly 40 park timeouts, several times the 45s storage.maxTransactionQueueTime an operator configured. This adds the missing bound.

One elapsed clock per logical commit, stamped on the chain root at its first native submission, is read at both retry decisions (the coordinated RETRY_NOW_VALUE resolve path and the ERR_BUSY/ERR_TRY_AGAIN rejection path). Past Math.max(storage.maxTransactionQueueTime, timeoutBudget) the commit takes the existing abortChainAfterRetries() cleanup, logs the stuck-commit identity, and throws a 503 TransactionCommitConflictTimeoutError carrying code: 'TRANSACTION_COMMIT_CONFLICT_TIMEOUT'. The 40-attempt cap stays as the second bound.

Three properties are load-bearing:

• The clock is per logical commit, not per attempt. trackOutstandingCommit() is untouched — it measures native liveness and drives checkOverloaded()'s thread-wide load shedding, so back-dating it would let one uncapped sourceApply retry eventually 503 every unrelated request on the thread. The new clock is stamped only when unset, so retry rounds and chained per-store commits inherit it; the budget comparison itself is where sourceApply is exempted.
• retryable is true only when nothing landed durably. A chained link commits solely from its predecessor's success handler, and a head that rotated through a mid-scope commit already landed a segment; in both cases replaying the request would repeat durable audit entries and hooks, so the flag is false — pinned for the chained case and for the mid-scope case.
• The abandonment log gets its own cooldown slot. It initially shared checkOverloaded()'s 1s limiter, but that one fires on every bystander write during a wedge and would have starved the only server-side line naming the abandoned transaction; each site now has its own slot.

Source-applied writes are exempt from the deadline exactly as they are from the attempt cap: there is no resubscribe/sequence-resume path, so dropping one permanently diverges the node (harper-pro#348).

For the human reviewer

The step-6 planning gate never returned chosen-approach-sound. Two rounds returned Framing-Verdict: better-alternative-exists: the first rejected a ladder-local 45s clock in favour of reusing the outstanding-commit tracker; the second rejected that because back-dating the tracker would make one long source-apply retry shed every unrelated request on the thread. @kriszyp adjudicated and directed the third framing — deadline only on the chain root, tracker per attempt, abandonment-site diagnostic, retryability conditioned on whether the chain half-landed — which is what is implemented here. The design note is not in the diff; its "Approaches considered" set is summarised above.

Closes may overstate what this does. #2450 asks for three things. This delivers the deadline (ask 1) and a distinct error code separating "wedged" from "queue overloaded" (part of ask 3). It does not deliver the holder/key diagnostic (ask 2) — rocksdb-js 2.8 exposes neither the waited-on key nor the intent holder through its public Transaction API — nor cancellation of an already-submitted native commit, nor scoping checkOverloaded()'s rejection to what actually conflicts (the rest of ask 3). If you want those tracked, downgrade this to Refs and split them out.

Declined review findings, all raised and adjudicated pre-push:

• A wedged commit leaves the clock set on a reusable instance (minor). Real in principle: the release runs when the commit settles, and a commit that never settles never releases. I tried to fix it by clearing on save()'s immediate-commit reuse branch and reverted that — I could not reach the branch from a test, because Resource.ts starts a fresh transaction for a static op whose context transaction is closed. The residual is bounded: a thread with a permanently wedged commit is already 503ing every application write through checkOverloaded(), so the extra spurious 503 is not a new failure mode. Worth a follow-up if you disagree.
• The release wrapper costs one promise and two closures per commit (nit). Kept deliberately: it is one place to get right instead of five terminal branches, and it is on the returned promise rather than a second subscriber so a dropped commit rejection still surfaces as an unhandledRejection. Per commit, not per record, in a path that already builds several promises.

Open decisions, unchanged from the design and worth a second opinion:

• Math.max(maxTransactionQueueTime, timeoutBudget) means a caller can only ever raise its budget. A request with a 2s client deadline still burns the full configured queue time in retries.
• The abandonment is a 503 even when retryable is false. Clients that retry on status code alone will replay a half-landed multi-store request; the flag is in the response body (serverHelpers.js sends code/retryable whenever retryable !== undefined), not the status.
• The budget is measured from the first native submission, not from scope entry — so it is 45s of commit retry on top of whatever the request already spent staging and in pre-commit blob I/O.

Verification

• npm run build, npm run format:write, npm run lint:required — clean. The last commit reformats unitTests/resources/query-array-scoping.test.js, which is unrelated pre-existing drift from Pin element-scoping semantics of queries over array-valued properties #2437 under the current prettier 3.9.x — Format Check fails on it identically on main, so it had to be fixed here to get a green run. Formatting only.
• npm run test:unit:resources — 1929 passing, 0 failing.
• npm run test:unit:main — 5188 passing, 2 failing; both (gitCredentials, configValidator domain-socket path length) reproduce unchanged on origin/main in this worktree and are environmental.
• npm run test:integration:all, run in four chunks to fit the runner: 2006 passing, 0 failing. The only failures were the OllamaBackend against a real Ollama instance cases, which need a local Ollama daemon.
• Fails-on-base check: with resources/DatabaseTransaction.ts and utility/errors/hdbError.ts reverted to origin/main and dist rebuilt, the three behavioural cases in the new suite fail (they get the generic 500 exhaustion error after 41 attempts instead of the 503).
• End-to-end route: resource-layer tests driving real Harper table transactions with only the native commit result substituted. A native park deadline is covered in rocksdb-js and is not reproducible end-to-end here without deliberately leaking a native intent.

New coverage in unitTests/resources/commitConflictDeadline.test.js: production arming/release of the clock (so the one line that arms it cannot be deleted with the suite still green), abandonment on both conflict paths, one budget shared across a cross-database chain with retryable: false and the head's write durable, the mid-scope-rotation half of the retryable decision, sourceApply retrying to convergence past the deadline, and the clock being released so the next transaction starts fresh. Error shape in unitTests/utility/errors/hdbError.test.js.

Closes #2450

Complexity: moderate

🤖 Generated with Claude Code — reviewed by Claude Opus 5

_{Review-Coverage: authored=claude; ran=codex; blocked=gemini(auth); declined=cursor-grok,cursor-composer,domain; rounds=5 @ d8d49de}

_{Human-Review-Need: 3 @ d8d49de}

[gemini-code-assist]

Code Review

This pull request introduces a mechanism to track and enforce deadlines for transaction commit conflict retries, preventing requests from hanging indefinitely on locked write intents. It adds the TransactionCommitConflictTimeoutError (503) and implements budget checks on logical commits. Review feedback recommends aligning null-or-undefined checks with the repository style guide by using loose equality (== null), using strict assertions (assert.strictEqual and assert.deepStrictEqual) in tests to avoid type-coercion bugs, and replacing condition-polling helpers with deterministic watchdog timers in tests.