Skip to content

Record locks: make cluster-scoped lock() work at threads.count > 1 (owner-worker relay, per-isolate row queue, every-thread registration) #852

Description

@kriszyp

Summary

Cluster record locks (harper-pro#438, PR #822) work correctly with one http worker and are not usable at threads.count > 1. Three separate items are behind that; the first is the one an operator hits immediately, the second is a correctness gap, the third is a known hole from core's design note §11.

The feature is default-off, so nothing shipped is affected today — but this is what stands between "opt-in, single-worker" and "usable at the default worker count", and it gates any later decision to enable record locks by default.

1. A local lock() is not relayed to the coordinating worker

Core fails closed off the owner thread — Cluster record lock coordination is not owned by this worker thread; retry so the request reaches the coordinating thread (core/resources/recordLockCoordinator.ts) — and harper-pro relays inbound peer operations to the owner (replication/recordLockRpc.ts routes record_lock_delegate / record_lock_recall / record_lock_barrier through the main thread) but does not relay an application's own lock().

With threads.count: 4, roughly three quarters of HTTP connections land on a worker that answers 503, and a keep-alive client stays pinned to that worker across retries. replication/recordLockTransport.ts warns about exactly this at startup:

replication.recordLocks is enabled with N http worker threads: a cluster-scoped lock() succeeds only on the worker coordinating its database and answers 503 elsewhere (a keep-alive client must reconnect to retry); run one http worker (threads.count: 1) for uniform lock() service

Fix shape: relay the local acquire (and the matching release) through the main thread to the owner worker, mirroring the inbound path that already exists in recordLockRpc.ts. The relay, its 5 s bound, its fail-closed timeout answer and its main-thread routing are all already written for the other direction.

Design question the implementer must answer: a lock handle is per-transaction state on the calling worker, while the delegation lives on the owner. Relaying the acquire decision is straightforward; deciding what crosses the thread boundary (a grant token the caller's coordinator installs locally, versus the whole handle) is the real design content and should be settled before code.

2. rowQueues serializes per worker isolate, so a fenceExternal can restore a retracted generation

replication/recordLockHomes.ts's rowQueues is module-level, so it serializes same-database transitions per worker isolate, not per process. readRow also sits outside writeRow's transaction while the write replaces the whole row. Two workers can both read active: g1; a fenceExternal (which writes active: existing?.active) landing after a stage then restores the active the stage retracted — while the operator holds a 200 for that stage and proceeds to activate after the drain. That is two live generations, the exact invariant the home-map design exists to enforce.

Not reached by any test: every fixture runs threads.count: 1, and more than one worker is only warned about, not refused.

Fix shape: a process-wide guard (cross-worker lock or a main-thread-owned serializer) plus a compare-and-set against the state the plan was decided on, rather than a whole-row replace.

3. Full every-serving-thread transport registration

Core's docs/record-lock-ownership.md §11 "still owed" list: the transport must be registered on every worker that can serve a lock(), including a dedicated application worker (harper#2524), and core cannot check that. A worker that never registers never latches clusterRequiredDatabases, so a default-scoped lock() there takes the Phase 0 node lock alone while a peer runs the cluster protocol — two nodes admitting one key.

Acceptance

  • A cluster-scoped lock() succeeds on every http worker, with a cluster integration test running threads.count > 1 and asserting uniform service (today every record-lock fixture pins threads.count: 1).
  • A stage racing a fenceExternal across two workers cannot produce two live generations; covered by a test that exercises both workers.
  • The startup warning in recordLockTransport.ts is removed or narrowed to whatever genuinely remains.

Not in scope

The lockRelease cross-thread relay gap is related but distinct and already tracked in the same §11 list; fold it in only if the relay from item 1 covers it.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    Fields

    Priority

    P1

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions