HarperFast / HarperFast/harper
Over-time transaction abort excludes sourceApply/isReplay, so those paths still force-commit partial write sets and can orphan secondary-index entries (silently) — #1407 gap
- Dominant language
- JavaScript
- Stars
- 89
- Forks
- 10
- Avg merge
- 2d 6h
- Merged PRs (30d)
- 200
Description
## Summary
#1407 ("Force-committing an over-time transaction leaves orphaned secondary-index entries") was
closed by making the transaction monitor **abort** an over-time transaction that has pending
writes, instead of force-committing a partial write set. That fix is correct, but it is guarded by
two exclusions — `sourceApply` and `isReplay` — and those paths still take the original
force-commit branch. The atomicity violation #1407 describes therefore still occurs on
replication-apply and crash-replay writes, and it is silent.
## The code
`core/resources/DatabaseTransaction.ts`, in the monitor's expiry handler:
```ts
} else if (txn.hasPendingWrites() && !txn.sourceApply && !txn.isReplay) {
// Abort and surface an error rather than force-committing a partial write set: silently
// committing on the application's behalf breaks atomicity and can leave orphaned
// secondary-index entries that only a full index rebuild repairs (issue #1407). ...
harperLogger.error(`Transaction was open too long and has been aborted ...`);
txn.abortDueToTimeout();
} else {
// Read-only long transaction (no atomicity/index risk ...), or a canonical-source
// apply/replay that must never drop a write: preserve the prior behavior of committing
// to close out the snapshot without poisoning the transaction.
const result = txn.commit();
...
}
```
The `else` arm is reached by two distinct cases with very different risk:
1. a **read-only** long transaction — genuinely no atomicity or index risk, as the comment says;
2. a **`sourceApply` / `isReplay` transaction with pending writes** — which is exactly the
force-commit-a-partial-write-set operation the first arm exists to prevent.
The comment justifies the exclusion on the grounds that aborting a canonical-source write would
drop it while the resume cursor advanced past it (harper-pro#348) — a permanent divergence. That
trade-off is real. The problem is that the alternative chosen is the one #1407 documented as
corrupting the index, and nothing marks or repairs the result.
## Why this matters
An orphaned secondary-index entry does not self-heal. Per #1407 it needs a full index rebuild, and
`updateIndices` cannot recover it during normal operation:
```ts
const existingValue = existingRecord && (resolver ? resolver(existingRecord) : existingRecord[key]);
if (value === existingValue && !isIndexing) continue; // an unchanged value does no index work
...
let valuesToRemove = getIndexedValues(existingValue, indexNulls);
if (valuesToRemove?.length > 0) { ... index.remove(...) } // removal driven ONLY by existingRecord
```
So a subsequent `put` of the same value is a no-op for the index, and a removal that was missed
stays missed. That is the mechanism behind #2211's report that "rewriting the record does not
repair it".
Two independent index divergences have now been reported in the wild on 5.2.x:
- #2211 — indexed reads returning a strict subset of the rows carrying a value, persistent, not
repairable by rewriting.
- A separate 4-node cluster where a `Long @indexed` attribute had entries left at a record's
**old** value with none at its current value. Three rows in ~1.3M, on two nodes independently,
arising on two different application write paths. Consequence there was severe: the stale entry
sorted to position 0 of an ascending index walk, and a consumer that stops at the first
future-dated row therefore processed nothing, silently, for ~18 days.
Neither is proven to originate from this branch — but this is a documented, in-tree path that
produces exactly that state, and it is the one path the #1407 fix deliberately left open.
## It is silent
The abort arm logs at `error` with the table name. The force-commit arm logs nothing on success,
and only `debug` on failure:
```ts
(result as any).catch((error) => {
harperLogger.debug?.(`Error committing timed out transaction: ${error.message}`);
});
```
So a partial write set committed on a replication-apply path leaves no operator-visible trace at
`info` or above, and no marker on the affected table. An operator cannot tell that an index may
now be inconsistent, nor which table to rebuild.
## Asks
1. Should `sourceApply` / `isReplay` transactions with pending writes really take the
force-commit path, given #1407 concluded that operation corrupts secondary indexes? If the
harper-pro#348 constraint makes aborting unacceptable, is there a third option — extending the
deadline for canonical-source applies, committing without the index short-circuit, or splitting
the apply — rather than choosing between dropping a write and corrupting an index?
2. If the force-commit must stay, can it at minimum be **observable**? A `warn` naming the table,
and ideally a durable marker that an index may be divergent, would turn a silent corruption
into something an operator can act on.
3. Is there a supported index rebuild/repair operation? #1407 says a full rebuild is the remedy
and #2211 asks the same question; we could not find one.
4. Is the read-only case worth separating from the write-bearing canonical-source case in that
`else`? They are grouped today, and only one of them is actually risk-free.
## Version
Observed on `harper-pro` 5.2.2/5.2.3 with `@harperfast/rocksdb-js` 2.7.x. Related: #1407 (closed),
#2211 (open), harper-pro#348 (closed).
Contributor guide
Research direction
Start in core/resources/DatabaseTransaction.ts at the monitor expiry handler and compare the read-only timeout path with sourceApply and isReplay transactions that have pending writes. Trace the index behavior described in the issue and review related issues #1407, #2211, and harper-pro#348; the work is complete only after the pending-write behavior and operator-visible handling are explicitly decided.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- node.js, typescript
- Domain
- databases
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Needs clarification
- Newbie friendliness
- 35/100