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

Open
#2,591 0 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.