Skip to content

Distributed lock fencing tokens are not actually distributed — no real multi-replica-safe LockStore exists #378

Description

@christabel888

Summary

FencingTokenGenerator (backend/src/distributed_lock/fencing.rs) issues fencing tokens from a per-process AtomicU64, and the only LockStore implementation in the codebase is InMemoryLockStore (backend/src/distributed_lock/store.rs). Neither of these can actually enforce the invariants the module's own documentation promises once the backend runs as more than one replica — which it must, for any real production deployment.

Location

  • backend/src/distributed_lock/fencing.rs
  • backend/src/distributed_lock/store.rs

Current gap / Motivation

store.rs's doc comment states the LockStore trait's implementors "must guarantee": atomicity of acquisition+token issuance, monotonicity of tokens per resource, exclusivity, and fencing of stale writes. DistributedLockConfig's own doc comments describe TTL/heartbeat tuning as if this were a real multi-instance coordination primitive ("If holder stalls > 60s, secondary instance acquires within 60s").

None of that is actually true today:

  • FencingTokenGenerator is an in-memory AtomicU64. It is never referenced outside its own unit tests (grep -rn "FencingTokenGenerator" backend/src/ turns up only fencing.rs itself). It cannot coordinate token issuance across two processes — each replica would hand out its own overlapping 0, 1, 2, ... sequence for the same resource.
  • The only impl LockStore is InMemoryLockStore — a single-process test double. There is no Redis- or Postgres-backed implementation despite store.rs's module doc explicitly describing itself as "abstracts over Redis or Postgres advisory locks as the backing store."

So the entire "distributed lock" subsystem is, today, safe only for a single process. Deploy two backend replicas (which the k8s manifests in this repo clearly anticipate) and the fencing-token pattern — whose entire purpose is to make stale writes after a GC pause / network partition detectable and rejectable — provides no actual protection, silently.

The hard part

This is not "write a Redis client." A correct implementation has to get right, at minimum:

  1. Atomic acquire-and-fence — lock acquisition and monotonic token issuance for a resource must be a single atomic operation from the store's perspective, even under concurrent acquisition attempts from N replicas hitting the same backing store simultaneously (Redis: this means a single Lua script or WATCH/MULTI transaction, not a GET then INCR then SET; Postgres: this means SELECT ... FOR UPDATE or advisory locks with correct isolation level, not read-then-write).
  2. Monotonicity across store restarts and failovers — if the backing Redis/Postgres instance fails over (e.g. Redis Sentinel promotes a replica that hasn't yet replicated the latest token), a naive implementation can hand out a token that has already been used, defeating fencing entirely. The implementation needs an explicit position on how it survives this (e.g. requiring WAIT for replication acknowledgment before considering a token "issued," or an explicit epoch/generation number layered on top of the per-resource counter).
  3. Clock-independent correctness — TTL expiry must be judged by the store's clock, not the holder's, and the design must be correct even when a lock holder experiences an arbitrarily long GC pause or network partition and later "wakes up" believing it still holds the lock (this is exactly the scenario fencing tokens exist to handle — the write path, not just the lock path, must check the token).
  4. No false exclusivity loss under legitimate heartbeat renewal races — a holder renewing its TTL right as it approaches expiry must not be able to race a second acquirer into a state where both believe they hold the lock, and must not have its legitimate renewal rejected due to ordinary network jitter.

Implementation

  • Add a real LockStore implementation backed by Redis (atomic Lua script covering acquire + token issuance + TTL set in one round trip) and/or Postgres (a table with a monotonic sequence per resource, correct row locking).
  • Wire FencingTokenGenerator::from_last_token (or replace it entirely) so token issuance is delegated to the backing store's atomic primitive rather than a local counter.
  • Document and test the exact failure-mode matrix: backing store failover, replica lag, holder GC pause past TTL, concurrent acquisition storms, and renewal-vs-expiry races.
  • Extend lock_sigstop_test.rs's existing SIGSTOP-based pause-simulation technique to a multi-process scenario proving two real OS processes can never both believe they hold the same resource's lock with valid, unfenced tokens.

Acceptance criteria

  • A LockStore implementation exists that is safe across ≥2 real, independent OS processes coordinating through a real backing store (not InMemoryLockStore).
  • A test demonstrates that a paused-then-resumed holder's write, using its original fencing token, is rejected after a second replica has acquired the lock and issued a new token.
  • A test demonstrates monotonicity survives a simulated backing-store failover (no token reuse).
  • The failure-mode table in ttl_manager.rs-style documentation is extended to cover this store, including what happens on backing-store unavailability mid-cycle.
  • Existing cargo test suite stays green.

Out of scope

Choosing a single "correct" backing store is not required — a well-reasoned choice of Redis or Postgres, correctly implemented and rigorously tested, is the bar. What's not acceptable is another abstraction layer with no real distributed implementation behind it.

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

    GrantFox OSSIssue tracked in GrantFox OSSMaybe RewardedIssue may be eligible for a GrantFox rewardThird CampaignCampaign: Third Campaign

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions