HarperFast / HarperFast/harper
DatabaseTransaction has no supported way to extend the open-transaction budget for one known-long write (LMDB gap + fragile workaround)
- Dominant language
- JavaScript
- Stars
- 89
- Forks
- 10
- Avg merge
- 2d 6h
- Merged PRs (30d)
- 200
Description
## Summary
`DatabaseTransaction`/`LMDBTransaction`'s open-time budget (`.timeout`) has no supported way
for a caller to extend it for one known-long write — a caller has to reach in and mutate
`.timeout` directly, and that mutation is fragile and doesn't propagate to every link that
actually needs it. This was found during cross-model review of
`fix/deploy-ingest-txn-timeout` (harper-large-deploy-txn-abort — the Large Deploy Payload Test
regression), which needed exactly this capability for the deploy payload-ingest write, and
worked around the gap in `components/deploymentRecorder.ts`'s `withExtendedTransactionTimeout`.
Two independent review lenses (Codex, an internal Harper-domain pass) and a third experimental
lens (Grok) all converged on the same root cause independently.
## The gap
1. **LMDB: the extension lands on the wrong object.** `resources/transaction.ts`'s
`transaction()` returns the `DatabaseTransaction` it creates
(`resources/transaction.ts:39`). The actual write resolves its transaction through
`txnForContext` (`resources/Table.ts:5489`), which reuses that head object only when
`isRocksDB` (`resources/Table.ts:5492`); for an LMDB-backed store it instead allocates a
**chained `new LMDBTransaction()`** on `.next` (`resources/Table.ts:5505`). The chained
object has its own `.timeout`, its own `getReadTxn()` (`resources/LMDBTransaction.ts:38`),
and its own `trackedTxns` set (`resources/LMDBTransaction.ts:16`) — the long-transaction
monitor never looks at the head's `.timeout`. Concretely: on `HARPER_STORAGE_ENGINE=lmdb`,
`withExtendedTransactionTimeout`'s 10-minute budget sits on an object nobody aborts, while
the LMDBTransaction that actually stages the row counts down from the generic default and
is aborted after ~30-60s — the exact "Transaction was open too long" failure the caller was
trying to avoid, with the workaround in place and its tests green. The same shape can reach
RocksDB too, whenever the write lands on a `.next` link because a *different* database was
already written earlier in the same transaction chain.
2. **The workaround (assign-after-call) is a coincidence, not an invariant.** `getReadTxn()`
unconditionally does `this.timeout = txnExpiration` (`resources/DatabaseTransaction.ts:362`,
`resources/LMDBTransaction.ts:41`). Setting the budget *after* calling the write (rather
than before) only survives today because the write's pre-commit read happens to be
synchronous for the common case — `_loadRecord`'s synchronous snapshot
(`resources/Table.ts:1003-1004`) and `_writeUpdate`'s `this.#entry ??` short-circuit for an
existing row (`resources/Table.ts:2188`). Concrete break: if the row is *absent* when the
write runs (concurrently deleted, or a future refactor that reorders row creation),
`loadLocalRecord` takes an async branch (`resources/Table.ts:5296`) and `getReadTxn()` fires
**after** the assignment — the budget silently reverts to the generic default and a large
write aborts, invisibly to any test that only exercises the arithmetic.
## Proposed root-cause fix
Give `DatabaseTransaction` (and `LMDBTransaction`) a `timeoutBudget` field, defaulting to the
current global `txnExpiration`:
- Both engines' `getReadTxn()` reset `.timeout` to `this.timeoutBudget` instead of the global
constant, so a caller's extended budget survives every subsequent read, not just the first
one, and survives regardless of read timing (sync or async).
- `txnForContext` propagates `timeoutBudget` down the `.next` chain the same way it already
propagates `sourceApply`/`isReplay` (`resources/Table.ts:5508`), so an LMDB-chained
transaction inherits the caller's budget instead of defaulting to the generic one.
- A caller then sets the budget **once**, before the write, on whichever transaction object it
has a handle to, and it holds for every link and every subsequent read — no ordering
gotchas, no per-engine special-casing at the call site.
## Where this currently matters
`components/deploymentRecorder.ts`'s `withExtendedTransactionTimeout` (added by
fix/deploy-ingest-txn-timeout) works correctly today for the actual regression it fixes
(RocksDB, the default and only engine the CI test exercises, single-database write, existing
row after `create()`). It is a known, documented gap for `HARPER_STORAGE_ENGINE=lmdb`
deployments and for any future caller in a similar situation — DESIGN.md's "Extending the
budget for one known-long write" section documents the workaround and this gap so the next
caller doesn't have to rediscover it. This issue tracks doing it properly at the
`DatabaseTransaction`/`LMDBTransaction`/`txnForContext` level so future callers don't need a
per-call workaround at all.
## Related, lower-priority observation from the same review
After a timeout abort, `Resource.ts`'s dispatch deliberately *joins* the poisoned transaction
so the write fails correctly (by design, for issue #1407's atomicity guarantee) — but this
means the deploy row's terminal `finish('failed', ...)` write *also* throws on the same
poisoned transaction, so `hdb_deployment.status` never leaves `pending` for a deploy that
definitively failed by timeout. This is pre-existing for any timeout-abort inside an operation
handler, not introduced by fix/deploy-ingest-txn-timeout, but that PR makes a too-large-to-fit
deploy's timeout abort the expected way it ends, so it's a more likely path than before.
Probably belongs in its own issue once someone picks this one up — noting it here so it isn't
lost.
Contributor guide
Research direction
Read resources/DatabaseTransaction.ts and resources/LMDBTransaction.ts first, then trace txnForContext in resources/Table.ts and transaction() in resources/transaction.ts. Verify how timeout values and chained transactions behave for synchronous and asynchronous reads, especially under LMDB. Done means one caller-set budget survives subsequent reads and every transaction link, with coverage beyond the existing RocksDB deploy regression.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- node.js, typescript
- Domain
- backend, databases
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100