Skip to content

perf(msgqueue): optimistic insert + heal-on-23503 — drop the per-publish parent-row lock#5

Closed
hydeta wants to merge 1 commit into
baselayerfrom
fix/pgmq-optimistic-insert-heal-on-fk
Closed

perf(msgqueue): optimistic insert + heal-on-23503 — drop the per-publish parent-row lock#5
hydeta wants to merge 1 commit into
baselayerfrom
fix/pgmq-optimistic-insert-heal-on-fk

Conversation

@hydeta

@hydeta hydeta commented Jul 8, 2026

Copy link
Copy Markdown

Description

Follow-up to #2/#3. The self-heal closed the MessageQueueItem_queueId_fkey (23503) race, but it runs ON CONFLICT ("name") DO UPDATE SET "lastActive" = NOW() on the parent MessageQueue row for every publish to an auto-deleted queue. DO UPDATE takes an exclusive row lock on the conflicting tuple, so all concurrent publishers to the same queue (tenant exchanges, the dispatcher queue after #3 widened the gate) serialize on one row, and every message rewrites it (dead-tuple churn, WAL, vacuum pressure).

Production impact (Cloud SQL Query Insights, hatchet-production, 1h window): ~68,300s of lock waits against ~80,650s total execution time — 85% of all DB time is lock waiting, and 99% of it is this one query (55,600s of its 59,600s runtime, 93%, is lock wait; hw tuple locks dominate).

Fix — same guarantee, off the hot path

AddMessageEnsuringQueue now tries the plain MessageQueueItem insert first. The FK check takes only a KEY SHARE lock on the parent (shared — publishers never block each other), and it fails with 23503 exactly when the parent has been reaped. On that error — and only then — it runs the existing atomic ensure-queue CTE.

Why this still closes the #2 race, with no cache-TTL coupling:

  • The heal is the same single-statement upsert+insert, so the reaper can never interleave between parent creation and item insert.
  • The heal sets lastActive = NOW() and CleanupMessageQueue's threshold is 1h idle, so the retried path cannot lose the race again and the queue pays at most one heal per idle-hour, not one exclusive lock per message.
  • A wrong 15s existence cache now means a handled error branch, not a dropped message (pre-fix(msgqueue): self-heal auto-deleted queues on enqueue to prevent FK violation (alt to #1) #2 it meant an opaque gRPC 500 and a lost webhook run).

Non-auto-deleted queues still propagate 23503 unchanged — they are never reaped, so a missing parent there is a real bug that must not be masked.

What's Changed

Type of change

  • Performance improvement (non-breaking)

Checklist

  • go build ./pkg/repository/... and go vet ./pkg/repository/ clean
  • go test ./pkg/repository/ -run 'AddMessageEnsuringQueue|BindRefreshesLastActive' — all 7 pass (4 existing + 3 new) against testcontainers Postgres

Post-deploy verification

insights/perquery/lock_time for osiris-app-production:hatchet-production should drop from ~68,000s/hr to near zero and the ensure_queue querystring should leave the top-N by lock wait entirely.

🤖 Generated with Claude Code

…arent upsert

AddMessageEnsuringQueue now tries the plain MessageQueueItem insert first
(KEY SHARE on the parent only, so publishers don't serialize) and runs the
atomic ensure-queue CTE only when the insert fails with a foreign-key
violation, i.e. when the parent was actually reaped. The heal refreshes
lastActive, so a healed queue is not reap-eligible for another hour and
subsequent publishes stay on the fast path.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@github-actions github-actions Bot added the engine label Jul 8, 2026
@hydeta

hydeta commented Jul 8, 2026

Copy link
Copy Markdown
Author

Closing: the optimistic-insert design lets an actively-published (publisher-only) queue be reaped hourly, and any items enqueued in the ≤5min before the reap are orphaned (queueId = NULL via ON DELETE SET NULL) and become invisible to ReadMessages. The deployed per-publish lastActive refresh prevents that reap entirely, so this PR would regress delivery of recent fanout items to late-attaching consumers. A follow-up should keep the queue alive with a throttled lastActive touch instead of a per-publish exclusive lock.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant