Skip to content

sync: reconciler-driven lock coordination as the "wait for a lock" pattern; intentionally no FIFO ticket lock #126

Description

@dev-arya23

Context

sync/ today offers only LockTable.TryAcquire (non-blocking: duplicate-key insert = held, returns immediately). Two follow-ups came up while reviewing the package:

  1. Is there a way to wait on Acquire (block until the lock is free)?
  2. Should we offer a read/write lock in addition to the plain mutex?

This issue records the decision for (1) — specifically the "hand the lock to the first waiter" / fair-acquire angle. (RWLock is tracked separately.)

Options considered

  • Barging blocking Acquire — a thin wrapper: TryAcquire → on errors.IsAlreadyExists wait for the lock-release notification (or ctx.Done()) → retry. ~30–40 lines on existing infra, no new primitive. It is barging: on release the change-stream delete fans out to every waiter via NotifyCallback, they all race to re-InsertOne, and whoever's write lands first wins. No ordering, so a brand-new contender can jump the queue.

  • FIFO / ticket (bakery) lock — a per-key doc with next_ticket / now_serving; acquire = atomic $inc next_ticket and read back the assigned number; release = $inc now_serving; each waiter watches the doc until now_serving == mine. This gives strict first-waiter-wins ordering across processes (order comes from the DB serializing the increments, not from client clocks). It requires an atomic read-modify-write that returns the post-image (FindOneAndUpdate), which core/db.StoreCollection does not expose today — so it is a core/db library change, not a sync/-local one.

Decision

Do NOT build the FIFO ticket lock (and do NOT add FindOneAndUpdate to core/db) at this time. Rationale:

  • Fairness ≠ correctness. Mutual exclusion is the correctness property, and TryAcquire already provides it. Strict FIFO is a QoS/premium property most workloads never need. Barging is the norm for DB/etcd/Redis-style locks, not a defect.
  • DB-backed locks are coarse and lease-bounded by nature (round-trip + change-stream propagation is tens of ms; liveness bounded by the owner age-out window, ~30s default). If we ever need high-frequency, many-waiter, strictly-ordered queueing, the DB is the wrong tool (that's etcd/ZooKeeper territory); adding a ticket lock would not change that.
  • The FIFO machinery is real and correct, but it should be gated behind a concrete, demonstrated starvation requirement — an actual case where a specific waiter is perpetually starved and it matters — not built speculatively.

Recommended pattern instead (documented + example in the accompanying PR): drive lock usage through the existing reconciler constructs rather than blocking on the lock.

  • Each replica registers its controller for lock-release via LockTable.RegisterLockRelease(name, ctrl) (and on its own domain table for normal, work-driven triggers).
  • On each Reconcile(key) the controller first checks whether there is real work pending for that key, and only then calls TryAcquire.
  • If the lock is held by a peer, it simply returns; the lock-release notification re-drives Reconcile(key) when the holder is done, at which point it re-evaluates whether work is still pending.

Gating acquisition on actual work is the key detail: it avoids the endless take-lock / find-nothing-to-do / release-lock churn that a naive blocking-acquire loop causes across N replicas, and it needs no new primitive.

Follow-up (only if needed)

If the reconciler-driven pattern proves insufficient for some consumer, revisit shipping the cheap barging blocking Acquire (internal TryAcquire → wait-on-release → retry loop, ctx-honoring, transient-error backoff). Track that here rather than reaching for FIFO.

Accepted architectural limitations of DB-backed locks (documented, not to be engineered around)

  • Barging, not fair.
  • Coarse-grained and relatively slow (tens of ms); suitable for briefly-held, low-contention, leader-ish sections.
  • Liveness is lease-bounded (~30s owner age-out), not instant.

Decision from the go-core-stack/core sync discussion. Documentation + reconciler example PR to follow, referencing this issue.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions