fix(msgqueue): prevent MessageQueueItem_queueId_fkey (23503) when an auto-deleted queue is reaped mid-publish

[blakewatters]

Summary

On the Postgres message-queue backend (SERVER_MSGQUEUE_KIND=postgres), triggering a workflow can intermittently fail with an opaque gRPC Internal / HTTP 500. The root cause is a race between the queue-retention GC and a producer publishing a large message: the GC deletes an auto-deleted queue out from under an in-flight insert, and the insert fails its foreign key.

This PR makes producing to a queue count as keeping it alive, and makes the fallback insert self-healing, so the FK violation can't occur regardless of GC timing.

The bug

could not add event to task queue: ERROR: insert or update on table
"MessageQueueItem" violates foreign key constraint
"MessageQueueItem_queueId_fkey" (SQLSTATE 23503)

It's rare (timing-dependent) and surfaces to callers as a non-obvious Internal/500 on TriggerWorkflowRun, silently dropping the run.

Root cause

Two facts combine into a race:

1. Only a subscriber keeps an auto-deleted queue alive. CleanupMessageQueue deletes any queue where "autoDeleted" = true AND "lastActive" < NOW() - INTERVAL '1 hour' (runs every 60s). lastActive is refreshed only by an active subscriber's 60s heartbeat (UpdateMessageQueueActive). Binding/publishing did not refresh it — so a queue with active producers but no subscriber (e.g. a per-tenant fanout exchange nobody is streaming, or a dispatcher queue whose dispatcher is briefly down) goes stale and gets reaped.

2. The producer skips the re-bind for 15s. upsertQueue caches "this queue exists" in a 15s in-memory TTL cache and returns early on a hit, so within that window it does not re-run BindQueue (which would recreate the row).

So: the GC deletes the queue; within the 15s cache window the producer skips the re-bind; and the next insert references the now-missing parent. The >8000-byte pg_notify fallback in Notify routes large payloads to a durable INSERT INTO "MessageQueueItem", which then violates MessageQueueItem_queueId_fkey. (The FK is ON DELETE SET NULL, so existing items are orphaned rather than blocking the delete — only the next insert races and fails.)

Repro conditions

• SERVER_MSGQUEUE_KIND=postgres
• An autoDeleted queue with no active subscriber for >1h (so it gets reaped).
• A publish to it whose wrapped payload exceeds pg_notify's 8000-byte limit (so it takes the durable-insert fallback).

The fix

Two complementary changes — the first removes the precondition, the second guarantees correctness even if the race still happens:

1. Binding a queue counts as activity. UpsertMessageQueue (called by BindQueue) now also SET "lastActive" = NOW() on conflict. A queue that is actively produced to re-binds at least every 15s (when the existence cache expires), so it can no longer go stale and be reaped out from under its own traffic.

2. The durable fallback self-heals — without touching the parent row on the hot path. AddMessageEnsuringQueue first attempts the plain MessageQueueItem insert. Its FK check takes only a FOR KEY SHARE lock on the parent row: KEY SHARE locks do not conflict with each other, so concurrent publishers never serialize on the parent — but they do conflict with DELETE, so an in-flight insert and the reaper serialize correctly against each other, which makes the FK violation a reliable, catchable signal that the parent is gone. Only on an actual 23503 does it run the ensure-queue CTE, which recreates the parent and inserts the item in the same statement:

WITH ensure_queue AS (
    INSERT INTO "MessageQueue" ("name","lastActive","durable","autoDeleted","exclusive")
    VALUES (@queueId, NOW(), @durable, @autoDeleted, @exclusive)
    ON CONFLICT ("name") DO UPDATE SET "lastActive" = NOW()
    RETURNING "name"
)
INSERT INTO "MessageQueueItem" ("payload","queueId","readAfter","expiresAt")
SELECT @payload, "name", NOW(), NOW() + INTERVAL '5 minutes' FROM ensure_queue;

Because the heal (re)creates the parent in the same statement as its insert, there is no window for the GC to race between "ensure" and "insert", and the retried insert can't fail. With change 1 in place, an actively-published queue can never accrue the 1h of staleness the reaper requires (the bind refresh runs at least every 15s), so the heal fires at most once per queue per reap — it logs at WARN because a sustained heal rate indicates something is deleting MessageQueue rows out-of-band.

Why not run the ensure-queue CTE unconditionally? An earlier revision of this PR did, and we ran it in production: ON CONFLICT ("name") DO UPDATE takes a row-exclusive tuple lock on the parent row per publish, so every concurrent publisher to a given queue convoys on a single row (the dispatcher queue is expirable ⇒ auto-deleted, so this includes every task-assignment message). Measured via Cloud SQL Query Insights over a 1h production window: the statement spent 93% of its execution time waiting on locks (55,600s of 59,600s), accounting for 85% of total DB time on the instance, plus one dead tuple per message on a handful of hot rows. The optimistic form keeps the identical correctness guarantee with zero hot-path writes to the parent row.

Notify's >8000-byte fallback and the durable addMessage path route through AddMessageEnsuringQueue whenever the queue is autoDeleted — i.e. exactly the set the reaper can delete. This includes exclusive auto-deleted queues (the dispatcher queue is expirable ⇒ auto-deleted and exclusive; controller consumer queues are auto-deleted + exclusive), so the queue's real durable/autoDeleted/exclusive attributes are threaded through and a recreated row keeps correct semantics. ON CONFLICT DO UPDATE only touches lastActive, so an existing queue's exclusiveConsumerId is preserved; a recreated one has a NULL consumer until the next BindQueue, which is safe (ReadMessages keys on queueId). Durable, non-auto-deleted queues (e.g. the shared task queue) are never reaped and keep the plain insert.

Tests

pkg/repository/mq_selfheal_test.go (testcontainers Postgres):

• AddMessageEnsuringQueue creates a missing parent (no 23503) and mirrors the supplied bind attributes.
• After CleanupMessageQueue reaps a stale queue, AddMessageEnsuringQueue recreates it — covering both a non-exclusive queue and an exclusive auto-deleted queue (exclusive preserved).
• UpsertMessageQueue refreshes lastActive, so a re-bound queue survives the inactivity sweep.
• The hot path must not rewrite the parent row: a publish to an existing queue leaves a backdated lastActive untouched (guards against reintroducing the per-publish upsert).
• A publish against a reaped queue self-heals (bind attributes and lastActive restored, message stored) instead of surfacing 23503.
• A missing parent for a non-auto-deleted queue still propagates the FK error — real bugs stay loud instead of being masked by the self-heal.

sqlc regenerated with the pinned v1.29.0; go build ./... and go vet clean.

Type of change

• Bug fix (non-breaking change which fixes an issue)

🤖 AI Disclosure

• I acknowledge that an LLM was used in the creation of this Pull Request, in accordance with Hatchet's AI_POLICY.md.

• Details: Claude Code (Claude Opus 4.8; revision by Claude Fable 5). Correlated production logs with the engine source to isolate the reaper-vs-producer race, traced the queue taxonomy against the cleanup predicate to find the exclusive-queue gap, wrote the fix + regression tests, and regenerated sqlc. The optimistic-insert revision was driven by production lock-contention telemetry from running the earlier form of this patch. Reviewed and edited by a human before submission.

[vercel]

@blakewatters is attempting to deploy a commit to the Hatchet Team on Vercel.

A member of the Team first needs to authorize it.

[blakewatters]

Revision pushed (46d1190): the self-heal is now optimistic — the ensure-queue CTE no longer runs on every publish, only on an actual FK violation. The PR description has been updated to match. Same correctness guarantee, but the reason for the change deserves the detail below, because we learned it the hard way.

What happened

We deployed the previous form of this patch (ensure-queue CTE on every publish to an auto-deleted queue) to a production cluster. It fixed the 23503 as designed — and then caused a lock-contention incident. Cloud SQL Query Insights over a 1h window:

Metric	Value
Total instance DB time	~80,650 s
Total lock wait	~68,300 s (85% of everything)
The ensure-queue CTE: execution time	59,600 s
…of which lock wait	55,600 s (93%, 38,400 s heavyweight tuple locks)
Everything else on the instance combined	< 550 s lock wait

The mechanism: ON CONFLICT ("name") DO UPDATE SET "lastActive" = NOW() takes a row-exclusive tuple lock on the parent MessageQueue row, held to commit, on every publish. The dispatcher queue is expirable ⇒ auto-deleted, so every task-assignment message from every engine/scheduler/controller replica serialized on a handful of parent rows — plus one dead tuple per message on those rows (WAL + vacuum pressure on a table with ~dozens of rows).

sequenceDiagram
    participant P1 as publisher 1
    participant P2 as publisher 2..N
    participant MQ as MessageQueue row (one per queue)
    participant MQI as MessageQueueItem

    Note over P1,MQI: previous revision: ensure-queue CTE per publish
    P1->>MQ: ON CONFLICT DO UPDATE (row-exclusive lock)
    P2--xMQ: blocked (tuple lock wait)
    P1->>MQI: INSERT item
    P1->>MQ: commit → release
    P2->>MQ: acquires… (convoy repeats per message)

Loading

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

    Note over P1,MQI: this revision: optimistic insert, heal only on 23503
    P1->>MQI: INSERT item (FK check → KEY SHARE on parent)
    P2->>MQI: INSERT item (concurrent — KEY SHARE ∥ KEY SHARE)
    Note over MQ: parent row untouched on the hot path

Loading

Why the optimistic form is still fully correct

The plain insert's FK check takes FOR KEY SHARE on the parent row. KEY SHARE conflicts with DELETE, so an in-flight insert and a concurrent reap serialize correctly against each other — the 23503 is therefore a reliable, catchable signal that the parent is already gone, and the heal (the same atomic CTE, now on the error path only) closes it.

And with the first half of this PR in place (binding refreshes lastActive, and producers re-bind at least every 15s when the existence cache expires), an actively-published queue can never accrue the 1 hour of staleness CleanupMessageQueue requires — 15s ≪ 1h. A queue only becomes reap-eligible after a genuine hour of silence, in which case (a) every item it holds already expired (items carry expiresAt = NOW() + 5 minutes), so the ON DELETE SET NULL orphaning loses nothing readable, and (b) the next publish is guaranteed a bind-cache miss, recreating the queue before the insert. The heal is a backstop for windows that essentially cannot occur — which is why it now logs at WARN: a sustained heal rate means something is deleting MessageQueue rows out-of-band, and that should be visible, not silently absorbed.

Alternatives we considered and rejected for the hot path: ON CONFLICT DO NOTHING doesn't lock the existing parent (the race survives, narrower) and returns no row on conflict, so the chained INSERT … SELECT FROM ensure_queue inserts nothing in the common case; a conditional DO UPDATE … WHERE lastActive < … still takes the row lock before evaluating the auxiliary WHERE, so it convoys identically.

New regression tests

• A publish to an existing queue must not rewrite the parent row (asserted via a backdated lastActive surviving a publish) — guards against reintroducing the per-publish upsert.
• A publish against a reaped queue self-heals: bind attributes preserved, lastActive restored, message stored.
• A missing parent for a non-auto-deleted queue still propagates the FK error — never reaped means a missing parent is a real bug, and the heal must not mask it.

All msgqueue tests (the four originals plus these three) pass against Postgres 15.6 via testcontainers; go build / go vet / gofmt clean.

🤖 Generated with Claude Code