HarperFast / HarperFast/harper

Record locks: recall drain revokes an unlocked-but-staged write (decided) — document it and assert the 409

Open
#2,580 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
89
Forks
10
Avg merge
2d 6h
Merged PRs (30d)
200

Description

A decision the delegation protocol (harper#2541, `docs/record-lock-ownership.md` §6) needs before it can be enabled, surfaced while integrating harper-pro#822.

## The situation

`lock(id, { hold: true })` inside a request stages its writes rather than committing them — Phase 0's documented deferred-hold model (`DESIGN.md`, "Staging model: scoped stages like `update()`; hold stays deferred"). The request transaction commits them when the request ends. `unlock()` is synchronous and returns well before that.

Under delegations, `unlock()` correctly writes nothing: the delegation is retained, which is the amortization. But it does drop the delegation's `admitted` count to zero. A recall arriving in that window sees an idle delegation, drains immediately, and surrenders — and surrender **revokes** every handle the delegation admitted (`registerAdmission` → `handle.revokeLease()`), so the still-staged write is fenced with a 409 at commit.

```
request: lock(hold) → save() [staged] → unlock() … request txn still open
peer: recall → drain (admitted == 0) → surrender
request: commit → 409, whole request fails
```

That is **safe** — it is exactly what stops the predecessor's write landing after the successor was admitted, and it is the fix that closed one of the round-1 blockers. It is also a surprising failure for a natural pattern, and the caller has no way to see it coming.

## The decision

§6 says recall must "drain existing holders **and in-flight commits**". Two readings, and the code currently implements the first:

1. **Revoke (today).** The drain waits only on live admissions; anything already staged is revoked and 409s at commit. Exclusion is exact, the request loses its work, and the caller must handle 409-and-retry.
2. **Wait.** The drain also waits for the admitting transaction to commit or abort, not just for `unlock()`. Friendlier, and closer to §6's wording — but it ties the home's handoff latency to an arbitrary application request, which is what the recall bound (`DELEGATION_LEASE_MS`) exists to avoid, and it needs a way to observe "this transaction has settled" from the coordinator.

A hybrid is possible: wait up to a short bound, then revoke.

## Why it matters beyond ergonomics

§7.1's clean-handoff freshness argument rests on the release entry being "ordered behind the delegate's own data writes on its own stream". Under (1) that premise holds — the revoked write never commits, so the release genuinely follows every committed write. Under (2) it also holds, by waiting. Under a naive "drain on `admitted === 0` and surrender without revoking" it would **not**, which is worth recording because that is the obvious-looking implementation. Whoever builds harper#2542 should not have to rediscover this.

## Follow-up steps

1. Decide between (1), (2) and the hybrid. This is an API-behavior call, not an implementer's.
2. If (1) stands: document it on `lock()`'s contract (harper#2547 owns that text) — a held write staged in a request can be refused at commit if another node takes the key first — and add an integration case asserting the 409 rather than leaving it as an unwritten consequence.
3. If (2) or the hybrid: the coordinator needs a settle signal per admission. `registerAdmission` already carries a per-handle callback, so the shape exists; the work is exposing transaction settlement to it and bounding the wait.
4. Either way, the `{ hold: true }` + request-transaction combination deserves an explicit note in `DESIGN.md`'s Phase 1 section, since Phase 0's deferred-hold note predates cluster recall and does not cover it.

Refs #483, #2541, #2542, #2547

🤖 Filed by Claude Opus 5 on behalf of Kris.

Contributor guide

Open the contributing guide

Research direction

Read docs/record-lock-ownership.md §6 and DESIGN.md's deferred-hold and Phase 1 sections, then trace the referenced admission and transaction behavior. Resolve the revoke-versus-wait decision, record the resulting lock() contract, and add an integration case covering the selected outcome, including the 409 if revocation remains the behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, nodejs
Domain
databases, distributed-systems, documentation, testing-qa
Issue type
Documentation
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Needs clarification
Newbie friendliness
28/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.