perf(msgqueue): optimistic publish + heal-on-23503 — remove the per-publish parent-row lock

[blakewatters]

Description

Replaces the per-publish ON CONFLICT DO UPDATE self-heal from #2/#3 with an optimistic publish + heal-on-23503: the hot path is a plain MessageQueueItem insert (no parent-row lock), and the ensure-queue CTE runs only when the insert actually fails with the FK violation — i.e. only when the parent queue was genuinely reaped. Supersedes #5 (same shape, adds the heal warn-log canary and the safety-arithmetic comments) and #6 (whose throttled-touch cache is redundant — the analysis below shows why). This is also the shape we should update upstream hatchet#4330 to.

Production impact of the current fix (why this PR exists)

Cloud SQL Query Insights, hatchet-production, 1h window on 2026-07-08:

Metric	Value
Total instance DB time	~80,650 s
Total lock wait	~68,300 s (85% of everything)
The WITH ensure_queue … CTE: exec time	59,600 s
The CTE's lock wait	55,600 s (93% of its exec time; 38,400 s heavyweight tuple locks)
Runner-up: BindQueue upsert lock wait (queued behind the same row locks)	12,200 s
Everything else on the instance combined	< 550 s

The mechanism: ON CONFLICT ("name") DO UPDATE SET "lastActive" = NOW() takes a row-exclusive tuple lock on the parent MessageQueue row, held until the transaction commits, on every publish to an auto-deleted queue. The dispatcher queue is durable + expirable, and bindAttrs promotes expirable ⇒ autoDeleted (internal/msgqueue/msgqueue.go), so every task-assignment message from 2 controllers + 4 engines + 4 schedulers serialized on a handful of rows — plus one dead tuple per message on those rows (WAL + vacuum pressure).

sequenceDiagram
    participant P1 as publisher 1
    participant P2 as publisher 2
    participant P3 as publisher N…
    participant MQ as MessageQueue row<br/>(one per queue)
    participant MQI as MessageQueueItem

    Note over P1,MQI: BEFORE (deployed #2/#3): every publish upserts the parent row
    P1->>MQ: ON CONFLICT DO UPDATE (row-exclusive lock)
    P2--xMQ: blocked (tuple lock wait)
    P3--xMQ: blocked (tuple lock wait)
    P1->>MQI: INSERT item
    P1->>MQ: commit → release lock
    P2->>MQ: acquires lock… (convoy repeats per message)

Loading

sequenceDiagram
    participant P1 as publisher 1
    participant P2 as publisher 2
    participant P3 as publisher N…
    participant MQ as MessageQueue row
    participant MQI as MessageQueueItem

    Note over P1,MQI: AFTER (this PR): plain insert, FK check takes KEY SHARE only
    P1->>MQI: INSERT item (FK → KEY SHARE on parent)
    P2->>MQI: INSERT item (concurrent — KEY SHARE ∥ KEY SHARE)
    P3->>MQI: INSERT item (concurrent)
    Note over MQ: parent row untouched on the hot path —<br/>no tuple locks between publishers, no dead tuples

Loading

FOR KEY SHARE locks (what an FK check takes on the referenced row) do not conflict with each other, so publishers never wait on publishers. They do conflict with DELETE, so an in-flight insert and the reaper serialize correctly against each other — that conflict is exactly what makes the FK violation a reliable, catchable signal that the parent is gone.

The original bug, for context (why a self-heal exists at all)

sequenceDiagram
    participant Prod as producer (engine)
    participant Cache as bind cache (15s TTL)
    participant GC as CleanupMessageQueue (60s)
    participant DB as Postgres

    Note over Prod,DB: pre-#2: unpatched v0.84.0
    Prod->>Cache: queue bound? → yes (cached)
    GC->>DB: DELETE MessageQueue WHERE autoDeleted AND lastActive < NOW()-1h
    Note over DB: parent row gone
    Prod->>DB: INSERT MessageQueueItem (>8KB pg_notify fallback)
    DB--xProd: 23503 MessageQueueItem_queueId_fkey
    Note over Prod: TriggerWorkflowRun → gRPC Internal / HTTP 500<br/>run silently dropped

Loading

Two upstream defects combined: only a subscriber's 60s heartbeat refreshed lastActive (producing didn't count as activity), and the producer's 15s existence cache skipped the re-bind that would have recreated the queue. #2 fixed the liveness half (bind now refreshes lastActive) and closed the FK half with the CTE — correct, but the CTE ran on every publish instead of only on the race.

Why this PR is complete — the liveness argument

The reaper (CleanupMessageQueue, 60s cadence) deletes queues where autoDeleted AND lastActive < NOW() - INTERVAL '1 hour' (pkg/repository/sqlcv1/mq.sql). For that to bite, a queue must go a full hour with zero lastActive refreshes. Three independent layers make that impossible for any queue anyone cares about:

flowchart TD
    subgraph liveness [What refreshes lastActive]
        A["Producer bind refresh<br/>addMessage → upsertQueue<br/>RegisterTenant → upsertQueue (tenant exchange)<br/>15s TTL cache → UpsertMessageQueue SET lastActive=NOW()"]
        B["Subscriber heartbeat<br/>60s ticker → UpdateQueueLastActive<br/>(msgqueue.go:186-197)"]
        C["Heal-on-23503 (this PR)<br/>recreates queue with lastActive=NOW()<br/>+ WARN log"]
    end
    A -->|"≤15s staleness while publishing"| Q[MessageQueue row]
    B -->|"≤60s staleness while consuming"| Q
    C -->|"backstop; fires ≈ never"| Q
    Q --> R{"reap-eligible?<br/>lastActive < NOW()-1h"}
    R -->|"active queue: NO — 15s ≪ 1h"| S[never reaped mid-traffic]
    R -->|"truly idle >1h: YES"| T["reaped — but holds only expired items<br/>(items carry expiresAt = NOW()+5min;<br/>ReadMessages filters expiresAt > NOW())"]

Loading

Walk every queue class through it:

Queue	durable/autoDeleted (effective)	Producer bind refresh	Consumer heartbeat	Can a reap lose anything?
Static queues (task_processing_queue_v2, OLAP, DLQs)	durable, not autoDeleted	✓ (15s)	✓	Never reap-eligible at all
Dispatcher queues (<id>_dispatcher_v1)	durable, expirable ⇒ autoDeleted	✓ (15s)	✓ (60s while dispatcher lives)	Only if bind refresh AND heartbeat are both gone for 1h — i.e. dispatcher dead and nobody publishing to it; pending items are 5-min-TTL messages addressed to a dead consumer
Controller consumer queues (<partition>_<controller>_v1)	non-durable, autoDeleted	✓ (15s, via publish path)	✓	Same as above
Tenant exchanges (<tenant>_v1)	non-durable, autoDeleted, fanout	✓ (15s, via RegisterTenant on every tenant-scoped publish, any payload size)	only while a client streams run events	Idle >1h ⇒ reaped, but every item inside already expired (5-min TTL); the next publish is guaranteed a bind-cache miss (15s ≪ 1h) so the queue is recreated before the insert

The timeline that makes the last row airtight:

flowchart LR
    T0["publish at T<br/>(bind refreshed ≤15s ago →<br/>lastActive ≥ T-15s)"] --> T1["…queue goes quiet…"]
    T1 --> T2["T+1h: reap-eligible<br/>(requires NO bind for a full hour)"]
    T2 --> T3["reaper deletes queue<br/>FK ON DELETE SET NULL orphans<br/>only items that expired ≥55min ago"]
    T3 --> T4["next publish: bind cache expired<br/>long ago (15s ≪ 1h) → upsertQueue<br/>recreates queue BEFORE the insert"]
    T4 --> T5["insert succeeds — no 23503,<br/>no loss, no heal needed"]

Loading

The heal path is therefore a backstop for windows that essentially can't occur in this codebase (a process pausing >1h between its cache check and its insert; something deleting MessageQueue rows out-of-band) — which is exactly why it logs at WARN: a sustained heal rate is a signal that an invariant broke, not normal operation.

Why we don't need to do more — alternatives considered and rejected

• ON CONFLICT DO NOTHING in the CTE — doesn't lock the existing parent row, so the reaper can still delete it between the CTE's conflict check and the item insert's FK check (the exact race, narrower). Worse, on conflict it returns no row, so INSERT … SELECT "name" FROM ensure_queue silently inserts nothing in the common queue-already-exists case. Broken twice.
• Conditional DO UPDATE … WHERE lastActive < NOW() - '5 minutes' — Postgres takes the row lock before evaluating the auxiliary WHERE, so this serializes publishers exactly like the unconditional version. No help.
• perf(msgqueue): throttled lastActive touch — publishers stop serializing on the parent row, active queues never reaped #6's throttled touch cache (5-min repo-layer lastActive touch) — correct, but strictly redundant: the 15s bind refresh already keeps every actively-published queue alive on all paths and at all payload sizes, while perf(msgqueue): throttled lastActive touch — publishers stop serializing on the parent row, active queues never reaped #6's touch only fires on publishes that reach AddMessageEnsuringQueue (durable autoDeleted + >8KB fallbacks) — as a liveness mechanism it's both later (5min vs 15s) and narrower than what already exists. It also adds a second cache and more parent-row writes than this PR. Its one real payoff — insurance if a future upstream sync regresses the bind refresh — is already covered by the heal + WARN log here: the regression would surface as heal logs, not as silent loss (and the orphanable items in that scenario are 5-min-TTL messages addressed to a dead consumer).
• Making the reaper skip queues with live items (AND NOT EXISTS (… "expiresAt" > NOW())) — sound hardening, but redundant given the timeline above (a reap-eligible queue provably holds only expired items). Can be added upstream as a follow-up if reviewers want belt-and-suspenders in SQL rather than in comments.
• Client-side changes in osiris — none help: the failing publish is engine-side (addTenantExchangeMessage fans a copy of every tenant-scoped message to the tenant exchange regardless of enqueue transport; both the admin trigger and the event ingestor wrap the same call). The only client lever is keeping payloads under pg_notify's 8KB so the durable fallback never engages — worth pursuing separately for load reasons, but not a correctness requirement.

What changed

• pkg/repository/mq.go: AddMessageEnsuringQueue tries the plain insert first and runs the ensure-queue CTE only on an actual FK violation (with a WARN log); isForeignKeyViolation helper; comment updates. The Notify >8KB fallback and the durable addMessage path route through it unchanged — they just inherit the optimistic behavior.
• pkg/repository/mq_selfheal_test.go: three new tests — the hot path must not rewrite the parent row (asserted via a backdated lastActive surviving a publish); a publish against a reaped queue must self-heal with bind attributes and lastActive restored; a missing non-auto-deleted parent must still surface the FK error (real bugs stay loud).

No SQL changes: AddMessageEnsuringQueue (the CTE) stays as the heal statement; UpsertMessageQueue's lastActive = NOW() from #2 stays as the liveness mechanism.

Testing

• go test ./pkg/repository/ -run 'TestAddMessageEnsuringQueue|TestBindRefreshesLastActive' — all 7 pass against a real Postgres 15.6 (testcontainers), including the 4 pre-existing self-heal tests from fix(msgqueue): self-heal auto-deleted queues on enqueue to prevent FK violation (alt to #1) #2/fix(msgqueue): self-heal exclusive auto-deleted queues too (closes FK gap in #2) #3.
• go build / go vet / gofmt clean.

Rollout

Cut save-v0.84.0-baselayer.2-<sha> and watch two things: Query Insights lock-wait share (should collapse from 85% to noise — the 12.2k s/hr of BindQueue waits were convoy victims of the same row locks and should evaporate too), and the new heal WARN log (expected ≈ zero; any sustained rate means out-of-band MessageQueue deletes).

Refs: #1 (closed — bind-refresh-only), #2/#3 (deployed — introduced the per-publish lock), #5 (closed — same shape as this, pre-canary), #6 (superseded — throttled touch), upstream hatchet#4330 (to be updated to this shape).

🤖 Generated with Claude Code

[hydeta]

Non-blocking, just an alerting note: this WARN also fires on legitimate idle-hour reaps — a queue that's genuinely quiet for an hour will WARN exactly once on its next publish, with nothing wrong. The comment above already frames it correctly (a sustained heal rate is the invariant-broken signal), so whoever wires alerting on this line should threshold on rate, not on any single occurrence.