HarperFast / HarperFast/harper
Record locks: dropping a table orphans its LockCoordinator without a retirement record, so a same-name re-create restarts the fencing-token counter
- Dominant language
- JavaScript
- Stars
- 89
- Forks
- 10
- Avg merge
- 2d 6h
- Merged PRs (30d)
- 200
Description
Dropping a table removes it from the in-memory schema without closing its `LockCoordinator`, so the coordinator's retirement record is never written. A same-name re-create inside one delegation lease then starts a fresh coordinator whose fencing-token counter restarts below tokens peers are still holding.
### The invariant
`retiredCoordinators` (`resources/recordLockCoordinator.ts:500`) exists for exactly this. Its doc comment states the rule:
> a coordinator built before then must not grant those keys again, and must not restart the counter into tokens the closed coordinator already issued
`close()` is the only writer of that record (`resources/recordLockCoordinator.ts:1107-1112`), and the constructor is its only reader (`:675-678`), raising `#counter` and `#grantableAfterMono` to the retired floor.
### The path that bypasses it
`resources/Table.ts:1852` drops the table from the schema:
```ts
delete databases[databaseName][tableName];
```
`lockCoordinator` is a closure variable on the per-table factory (`resources/Table.ts:598`) and the only `close()` calls on it are the standalone-claim path (`:5861`) and the transport-swap predecessor (`:5886`). Neither runs here, so the coordinator is orphaned rather than retired.
An orphan is not a leak: `tick()` expires both maps on their own deadlines, calls `#revokeAll` on each delegation, and removes itself from `tickingCoordinators` once it holds nothing — bounded by `DELEGATION_LEASE_MS + skew`. What is lost is the retirement record.
### Failure mode
Fencing tokens are `[generation, homeIncarnation, counter]` and `compareTokens` tiebreaks on the counter:
```ts
return a[0] - b[0] || a[1] - b[1] || a[2] - b[2];
```
A fresh coordinator starts at `#counter = 0` (`:624`). So, with the feature enabled:
1. Table T is home to keys delegated to peers; the coordinator's counter has advanced to *n*.
2. T is dropped. The coordinator is orphaned; no retirement record is written. Peers still hold delegations under tokens ending at *n*, valid for up to `DELEGATION_LEASE_MS + skew`.
3. T is re-created with the same name inside that window. `generation` and `homeIncarnation` are unchanged — the home map is operator-published per database, and a table drop advances neither.
4. The new coordinator mints `[generation, homeIncarnation, 1]`, which orders **below** the tokens already in peers' hands.
The drop path's own comment contemplates step 3 explicitly ("the tombstone guarantees the drop completes on the next startup (or on a same-name create)").
Silent: nothing logs, and the losing comparison reads as an ordinary stale-token rejection.
### Reachability
**Not reachable in any current build.** `lockCoordinator` is only constructed when `getClusterLockTransport(databaseName)` returns a transport (`resources/Table.ts:5857-5866`), and `registerClusterLockTransport` has no non-test caller in core — it is exported for harper-pro. It becomes reachable when harper-pro registers a transport (HarperFast/harper-pro#822 / #825), which is also the first point at which a fix is testable end to end.
### Fix sketch
Close the coordinator on the drop path so the retirement record is written, and treat the drop the way the transport-swap path already treats a predecessor. Worth doing alongside the harper-pro transport integration rather than blind, since the interesting assertion (a peer's token surviving a drop/re-create) needs a registered transport to write.
Found while adjudicating review feedback on #2498, where it was recorded as "not adjudicated here". That PR's ruling gave the reason as "pre-existing, with no line in this delta" — that part is wrong: `resources/recordLockCoordinator.ts` does not exist on `main` and `main`'s `Table.ts` contains no `lockCoordinator`. The correct reason to defer it is reachability, above.
Refs #483, #2498
Contributor guide
Research direction
Start in resources/Table.ts at the drop path around line 1852, then compare it with the coordinator-closing paths around lines 5861 and 5886. Read the retirement handling in resources/recordLockCoordinator.ts around lines 500, 675-678, and 1107-1112. With a registered cluster lock transport, verify that dropping and recreating a same-name table records retirement and prevents counters from restarting below still-held peer tokens.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- node.js, typescript
- Domain
- databases, distributed-systems
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 48/100