fix(auditdb): align locker semantics, split key namespaces, stop nesting retries

docs/configuration.md

@@ -319,13 +319,30 @@ token:
           #   "memory"   – in-process mutex (default, single-replica only)
           #   "postgres" – PostgreSQL lease-table (multi-replica)
           backend: memory
+          # memory section is read only when backend == "memory".
+          memory:
+            # acquireDeadline bounds how long one acquisition waits for an EID held
+            # by another anchor. Every backend bounds its own waiting so that a
+            # caller which passes no deadline still gets an answer, and so that
+            # spending the whole budget can be reported as such; auditor.lock does
+            # not retry an acquisition that already exhausted it. Defaults to the
+            # same 1m as the postgres backend, so switching backends does not
+            # silently change how long an audit can block.
+            acquireDeadline: 1m
           # postgres section is read only when backend == "postgres".
           postgres:
             # ttl is the lease duration for each EID lock row.
             ttl: 30s
-            # acquireBackoff is the wait between retry attempts when a lock is contended.
+            # acquireBackoff is the initial wait between retry attempts when a lock
+            # is contended. Successive waits grow exponentially and are jittered,
+            # so this is the floor rather than a fixed poll interval.
             acquireBackoff: 100ms
-            # acquireDeadline is the total time allowed to acquire all EID locks.
+            # acquireMaxBackoff caps that growth. Raised to acquireBackoff if set
+            # below it.
+            acquireMaxBackoff: 2s
+            # acquireDeadline is the total time allowed to acquire all EID locks,
+            # and the whole budget for waiting out contention: auditor.lock does
+            # not retry an acquisition that already exhausted it.
             acquireDeadline: 1m
             # heartbeat is the interval at which held leases are renewed (~TTL/3).
             heartbeat: 10s
@@ -810,6 +827,14 @@ Default values:
 - **backoffMultiplier**: Factor by which the backoff delay increases after each retry (exponential growth)
 - **jitterFactor**: Randomization factor (0.0 to 1.0) added to backoff delays to prevent multiple auditors from retrying simultaneously (prevents thundering herd problem)
 
+**Relationship to `auditor.locker`:** this retry covers failures that another attempt
+might survive, such as a transient database error. It deliberately does **not** retry
+an acquisition that failed with `ErrLockAcquireTimeout`, because that means the locker
+already spent its own `acquireDeadline` waiting out the contention — retrying would
+spend it again. Waiting under contention is configured once, in
+[`auditor.locker`](#optional-tokentmsauditorlocker), so `maxRetries` here does not
+multiply `acquireDeadline` there.
+
 **Tuning Recommendations:**
 
 1. **For High-Contention Environments:**
@@ -924,6 +949,7 @@ token:
           postgres:
             ttl: 30s
             acquireBackoff: 100ms
+            acquireMaxBackoff: 2s
             acquireDeadline: 1m
             heartbeat: 10s
             owner:
@@ -933,8 +959,9 @@ Default values:
 
 - backend: `memory` (in-process mutex, single-replica only)
 - postgres.ttl: 30s
-- postgres.acquireBackoff: 100ms
-- postgres.acquireDeadline: 1m
+- postgres.acquireBackoff: 100ms (initial wait; grows exponentially, jittered)
+- postgres.acquireMaxBackoff: 2s (cap on that growth; raised to `acquireBackoff` if set below it)
+- postgres.acquireDeadline: 1m (the whole budget for waiting out contention)
 - postgres.heartbeat: 10s
 - postgres.owner: empty, defaults to the FSC node ID (`config.Provider.ID()`). Required
   when `backend: postgres` — if both this value and `fsc.id` are empty or blank, the

docs/services/auditor.md

@@ -83,6 +83,37 @@ When multiple auditor replicas share the same AuditDB (PostgreSQL), concurrent p
 
 The locker is injected into `auditdb.StoreService` at startup. Before appending audit records, the store calls `AssertLocksHeld` to verify leases are still valid.
 
+Both backends implement the same contract, defined on the `Locker` interface, because the backend is chosen from configuration — a behavioural difference between them would be a correctness difference between two deployments running the same code. The shared expectations are exercised against every backend by `locker/conformance_test.go`.
+
+**`AssertLocksHeld` detects lost locks, not absent ones.** It fails only when a lease this replica held has since expired or been taken over. An anchor that holds nothing succeeds — whether because the request yielded no enrollment IDs, or because the caller never locked anything for it. The latter is a supported flow: an auditor may call `Validate` (which takes no locks) and then `Append`, as the dvp and nft sample views do.
+
+**`AcquireLocks` is all-or-nothing, and never gives up ground.** A failed call holds none of the EIDs it had reached for, and leaves untouched whatever the anchor already held from an earlier successful call. Acquiring an *empty* set is a successful acquisition of nothing — it must not, and does not, release what the anchor is already holding.
+
+**Re-acquiring a live anchor may shrink its EID set, never grow it.** A refresh keeps the EIDs still named and releases the ones dropped from the set; naming an EID the anchor does not already hold fails with `ErrLockSetWidened`. That restriction is what keeps the lockers deadlock-free. Deadlock freedom rests on every caller taking shared EIDs in one canonical order, and that order can only be imposed over the EIDs of a single call — an anchor that keeps earlier locks while waiting for new ones is holding locks outside it, so two anchors widening into each other's EIDs wait on each other indefinitely. `Audit` acquires once per anchor and releases when done, so no caller needs to widen.
+
+**Each backend bounds its own waiting.** A caller that passes no deadline still gets an answer: the Postgres backend stops after `acquireDeadline`, and the in-memory backend after its own `acquireDeadline`. Without a budget of its own a backend could only ever stop when the caller's context did, and could never report `ErrLockAcquireTimeout` — the signal `auditor.Service` reads to tell "already waited in full" from "worth another attempt".
+
+**Release always runs.** `ReleaseLocks` is idempotent, safe on an anchor that never acquired anything, and safe to `defer`. Its statement is deliberately detached from the caller's context, since the usual case is a deferred release on a context that is already cancelled and a skipped release would leave the EIDs locked against every replica until the lease TTL expired. It carries its own short internal deadline so a stuck release cannot outlive the call that issued it.
+
+### Error classification
+
+Callers act on the outcome of a failed acquisition, so both backends classify it the same way:
+
+| Outcome | Sentinels | Meaning |
+|---------|-----------|---------|
+| Another anchor holds an EID and the waiting budget ran out | `ErrLockContention` + `ErrLockAcquireTimeout` | A real conflict, already waited out in full; retrying only adds delay |
+| Another anchor holds an EID, but a transient failure or a cancelled caller ended the wait | `ErrLockContention` | A real conflict, not yet waited out; a later attempt may succeed |
+| The caller cancelled or ran out of time while nothing held the EIDs | neither (plain context error) | Not a conflict, and not counted as one |
+| The backend's own waiting budget elapsed while nothing held the EIDs | neither (plain context error) | Not a conflict; worth retrying while the caller's context is still live |
+| The anchor asked for an EID it does not already hold | `ErrLockSetWidened` | A caller error, not a conflict: every attempt reproduces it |
+| The database failed | neither; the underlying error is preserved | An infrastructure fault, reported as itself rather than as contention |
+
+The distinction is drawn from whether an attempt actually lost a race for an EID — not from which context expired first. `acquireDeadline` defaults to a minute, so a request-scoped caller context is almost always the shorter of the two, and keying off it would mean the Postgres backend hardly ever reported contention in production.
+
+Only the two `ErrLockContention` rows count towards `auditor_audit_lock_conflicts_total`. The rows that are not conflicts are not counted as ones, so a graceful-shutdown cancellation or a database outage does not inflate the metric operators alert on for contention.
+
+Whether another attempt is worth making is read from the caller's context rather than from the error, because the two rows carrying a plain context error are indistinguishable by the error alone: the caller having given up is final, whereas the backend's own budget elapsing is exactly the transient case `auditor.lock` exists to retry. See `isRetriableLockError`.
+
 ### Configuration
 
 Configure under `token.tms.<name>.auditor.locker` (see [Configuration](../configuration.md#optional-tokentmsauditorlocker)):
@@ -96,8 +127,9 @@ token:
           backend: postgres   # use "memory" (default) for single replica
           postgres:
             ttl: 30s
-            acquireBackoff: 100ms
-            acquireDeadline: 1m
+            acquireBackoff: 100ms     # initial wait; grows exponentially, jittered
+            acquireMaxBackoff: 2s     # cap on that growth
+            acquireDeadline: 1m       # whole budget for waiting out contention
             heartbeat: 10s
             owner:            # required; defaults to the FSC node ID
 ```
@@ -113,6 +145,12 @@ The Postgres backend creates an `eid_leases` table (prefixed per TMS persistence
 
 **Lease ownership:** each row is keyed by EID and carries the holding replica (`owner`) plus the request it was taken for (`anchor`). An acquisition may only take over an existing row when the lease has expired, or when the row is the same replica's lease for the *same* anchor — a re-acquisition, which just refreshes the deadline. Two different anchors therefore never hold the same EID at once, including two concurrent audits on a single replica; the second one is contended and retried until `acquireDeadline`.
 
+### Waiting under contention
+
+`acquireDeadline` is the **entire** budget for waiting out a contended EID, and one layer does the waiting. Within it, the locker retries the atomic acquisition starting at `acquireBackoff`, doubling up to `acquireMaxBackoff`, with jitter on every sleep. The jitter matters as much as the growth: a fixed interval is identical on every replica, so contenders retry in lockstep and keep colliding, and at the defaults it also costs one round trip per interval for the whole deadline — hundreds per acquisition.
+
+`auditor.Service` wraps `AcquireLocks` in its own retry (`token.tms.<name>.auditor.lock`) for transient failures, but it does **not** retry an error carrying `ErrLockAcquireTimeout`: the locker reporting one means it already spent `acquireDeadline`, so calling it again would spend that deadline afresh rather than improve the odds. Worst-case blocking for one audit is therefore about `acquireDeadline`, not `maxRetries × acquireDeadline`.
+
 ### Replica owner identity
 
 Every lease row carries an `owner` column, and each replica scopes all of its lease

token/services/auditor/auditor.go

@@ -178,7 +178,13 @@ func (a *Service) Audit(ctx context.Context, tx Transaction) (*token.InputStream
 	// Acquire locks with retry and exponential backoff to prevent livelock
 	logger.DebugfContext(ctx, "audit transaction [%s], acquire locks with retry", tx.ID())
 	if err := a.acquireLocksWithRetry(ctx, string(request.Anchor), eids); err != nil {
-		a.metrics.AuditLockConflicts.Add(1)
+		// Only a genuine conflict counts towards the conflict metric. Counting every
+		// failure meant a graceful-shutdown cancellation or a database outage — neither
+		// of which involves a second holder — inflated the one signal operators are
+		// told to alert on for contention.
+		if errors.Is(err, auditdb.ErrLockContention) {
+			a.metrics.AuditLockConflicts.Add(1)
+		}
 
 		return nil, nil, err
 	}
@@ -193,6 +199,13 @@ func (a *Service) Audit(ctx context.Context, tx Transaction) (*token.InputStream
 // acquireLocksWithRetry attempts to acquire locks with exponential backoff and randomized jitter
 // to prevent livelock conditions when multiple auditors compete for the same enrollment IDs.
 // This implements the mitigation strategy for deadlock/livelock prevention.
+//
+// The locker owns the waiting policy and bounds it itself, so this loop must not
+// re-run an attempt that already spent that budget: an error carrying
+// ErrLockAcquireTimeout is final here. Retrying it anyway multiplied the locker's
+// deadline by MaxRetries — worst case, ten minutes of blocking for a single audit
+// against the Postgres backend, on top of the round trips each of those attempts
+// spent polling. Context errors are final for the same reason: the caller is gone.
 func (a *Service) acquireLocksWithRetry(ctx context.Context, anchor string, eids []string) error {
 	// Create a retry runner with jitter support
 	retryRunner := utils.NewRetryRunnerWithJitter(
@@ -204,18 +217,47 @@ func (a *Service) acquireLocksWithRetry(ctx context.Context, anchor string, eids
 		a.lockConfig.JitterFactor,
 	)
 
-	// Use the retry runner to acquire locks
-	err := retryRunner.RunWithContext(ctx, func() error {
-		return a.auditDB.AcquireLocks(ctx, anchor, eids...)
-	})
+	// Use the retry runner to acquire locks, stopping early on errors that another
+	// attempt cannot improve on.
+	err := retryRunner.RunWithErrorsContext(ctx, func() (bool, error) {
+		err := a.auditDB.AcquireLocks(ctx, anchor, eids...)
+		if err == nil {
+			return true, nil
+		}
 
+		return !isRetriableLockError(ctx, err), err
+	})
 	if err != nil {
 		return errors.WithMessagef(err, "failed to acquire locks for anchor [%s]", anchor)
 	}
 
 	return nil
 }
 
+// isRetriableLockError reports whether re-running AcquireLocks stands a chance of
+// a different outcome. Most failures do — a contended lock may be free by now, a
+// database blip may have passed — but three do not: ErrLockAcquireTimeout means
+// the locker already spent its whole waiting budget, so an identical attempt would
+// just spend it again; ErrLockSetWidened is a caller error that every attempt will
+// reproduce; and a caller whose own context is done is no longer waiting for an
+// answer.
+//
+// Whether the caller is gone is read from ctx, not from the error. The lockers
+// bound their own waiting, and a budget of their own that elapses with nothing
+// contending surfaces as a bare context.DeadlineExceeded — indistinguishable, by
+// the error alone, from the caller's deadline elapsing. Classifying it from the
+// error stopped the auditor after a single attempt at exactly the transient
+// database failures this retry exists to survive, while ctx was still perfectly
+// live.
+func isRetriableLockError(ctx context.Context, err error) bool {
+	if ctx.Err() != nil {
+		return false
+	}
+
+	return !errors.Is(err, auditdb.ErrLockAcquireTimeout) &&
+		!errors.Is(err, auditdb.ErrLockSetWidened)
+}
+
 // Append adds the passed transaction to the auditor database, reusing the
 // audit record computed by Audit, when available.
 // It also releases the locks acquired by Audit.