Skip to content

Enforce one in-flight deployment per concurrency tag (multi-pod)

Placeholder ppxd requested to merge fix/p3-concurrency-atomic-task-slot into main

Summary

First of two PRs hardening multi-pod concurrency correctness (Squid runs as a horizontally-scaled K8s Deployment with a shared Redis-backed Hangfire queue, so any pod can run any task).

Problem. Per-environment deployment serialization relied on WaitForConcurrencySlotAsync — a non-atomic check-then-act poll that on timeout logged a warning and proceeded anyway, over a non-unique (ConcurrencyTag, State) index. Two pods could each observe a free slot and both run a deployment to the same environment. (Per-task exactly-once is already safe via the Pending→Executing CAS; this is the cross-task, same-tag hole.)

Fix — DB-enforced atomic claim over the ACTIVE set {Executing, Paused, Cancelling}:

  • New unique partial index ux_server_task_active_per_tag = at most one active row per ConcurrencyTag. Paused/Cancelling count as occupying because a transient-pause or wall-clock timeout transitions Executing→Paused while leaving the in-flight agent script running (preserved for resume) — a same-env deployment must not start over it. Migration pre-flight keeps the earliest active row per tag and administratively fails any pre-existing duplicates so the index builds on a live DB.
  • TransitionStateAsync maps the 23505 violation on the →Executing transition (the only transition that adds a row to the active set) to a typed ConcurrencySlotOccupiedException, scoped to the index by name via a shared const (OneActivePerTagIndexName) so migration ⇄ model ⇄ provider stay in lockstep.
  • The runner replaces the 300s in-process poll with one read → three outcomes: terminal (re-dispatched after cancel) → short-circuit; slot held → re-enqueue the still-Pending task (worker freed, never run-anyway), persisting the new Hangfire job id to task.JobId so a later cancel targets the live job; otherwise proceed. A TOCTOU claim race surfaces as ConcurrencySlotOccupiedException and is likewise re-enqueued — never overlapped, never failed, never lost.

Not a Postgres advisory lock: the EF DbContext runs over pooled connections, so a session-scoped advisory lock is unsafe; the unique index is connection-pool-safe and self-heals (terminal writes release it). Index built non-CONCURRENTLY (incompatible with DbUp's transaction) — partial over a retention-bounded table, so the brief build-time SHARE lock is acceptable. Untagged tasks skip the slot logic and are unchanged. PR2 will add the per-machine deploy↔️upgrade mutex (reusing the existing Redis lock).

Pre-merge adversarial review (18 agents) drove this revision: the original scoped the slot to Executing only (Paused-overlap gap) and discarded the re-enqueued job id (cancel-zombie); both are fixed here, with the Paused/Cancelling-holder case now covered by an integration test.

Test plan

  • Unit (6025/6025): runner terminal-guard / busy→requeue + JobId-persist / claim-race→requeue-not-fail; index-name SSOT pin
  • Integration (real Postgres, 7/7): unique index rejects a second active-same-tag (real 23505 → ConcurrencySlotOccupiedException); a Paused/Cancelling holder still blocks a same-tag claim; allows different tags + untagged; frees on terminal; two concurrent claims → exactly one winner + exactly one active row
  • Solution build: 0 errors

Merge request reports

Loading