fix(client): serialize pgmq schema install across clients and processes - #398
Merged
Conversation
ensure_pgmq_schema's @schema_ensured + synchronized guard is per-instance (and synchronized is a no-op on the dedicated-connection path), so two clients could run check+install concurrently: the loser's UniqueViolation surfaced as SchemaNotReady, and on the shared-AR Proc path the concurrent install DDL on one shared libpq connection desynced the protocol and wedged a thread on a socket read forever (getzazu/app#3413). - Serialize bootstrap process-wide via a class-level mutex. - Run check+install in one transaction holding pg_advisory_xact_lock on a fixed key — cross-process serialization, xact-scoped so it self-releases and stays safe through transaction-pooling poolers. - Rescue duplicate-object install failures by re-checking pgmq.meta: present = another process won, proceed; absent = re-raise. Closes #397
|
Running ultrareview automatically — This change reworks concurrency-sensitive schema bootstrap (process-wide mutex, pg_advisory_xact_lock, transaction/error framing) where a subtle bug could wedge connections or corrupt install state across processes — worth a deep review.. I'll post findings when complete. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
Ultrareview completed in 10m 33s
All reported issues were addressed across 3 files
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
3 tasks
mhenrixon
added a commit
that referenced
this pull request
Aug 12, 2026
…nstall The shared Proc-supplied PG::Connection is single-owner: every access must hold @pgmq_mutex. The new transaction-status probe ran outside it, and the #397/#398 rework had swapped ensure_pgmq_schema's synchronized for the class-level install mutex — silently dropping the connection-ownership guard the original code had. Both now hold synchronized (nested inside the class install mutex; safe order, nothing acquires them reversed). Also dedupes the create+tune DDL pair shared by queue and DLQ creation. Refs #399 review
mhenrixon
added a commit
that referenced
this pull request
Aug 12, 2026
…nsaction (#399) * fix(client): schema install must not commit or destroy a caller's transaction Follow-up to #398 review (P1 + P3): - The Proc-supplied shared connection can arrive mid-transaction (perform_later inside an application `transaction do`); the unconditional BEGIN was a warning no-op there, so the matching COMMIT/ROLLBACK operated on the CALLER's transaction. The framing is now transaction_status-aware: idle -> owned BEGIN..COMMIT, in-transaction -> SAVEPOINT/RELEASE with ROLLBACK TO SAVEPOINT on failure. - The process-wide serialization spec's fixed `sleep 0.05` could false-pass on a saturated scheduler; it now waits deterministically for thread B to block or terminate before asserting zero connection traffic. Refs #398 * fix(client): don't cache schema_ensured from a savepoint-path install A savepoint-path ensure rides the caller's transaction, so the install is only durable once THAT commits — caching @schema_ensured there means an outer rollback leaves the schema missing while every future check is skipped. Only the owned-COMMIT path caches now; the savepoint path re-checks on the next ensure (one SELECT). Also: the serialization spec's settle-wait now fails explicitly if thread B never settles, instead of silently degrading back into a fixed-duration window. Refs #399 review * fix(client): queue-creation cache follows the same durability rule Queue DDL on the shared Proc-supplied connection joins the caller's open transaction; caching @queues_created there outlives a caller rollback, so later ensures skip recreation and message operations fail against a missing queue. When the DDL rides a caller's transaction the queue is created (idempotent CREATE IF NOT EXISTS) but not cached; dedicated-path DDL runs on pgmq-ruby's own pool connections and caches as before. Refs #399 review * fix(client): hold the connection mutex for the txn probe and schema install The shared Proc-supplied PG::Connection is single-owner: every access must hold @pgmq_mutex. The new transaction-status probe ran outside it, and the #397/#398 rework had swapped ensure_pgmq_schema's synchronized for the class-level install mutex — silently dropping the connection-ownership guard the original code had. Both now hold synchronized (nested inside the class install mutex; safe order, nothing acquires them reversed). Also dedupes the create+tune DDL pair shared by queue and DLQ creation. Refs #399 review
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ensure_pgmq_schemais now serialized process-wide (class-level mutex) instead of per-instance —synchronizedwas a no-op on the dedicated-connection path, and on the shared-AR Proc path two instances each held their own mutex around one shared libpq connection, so concurrent bootstrap DDL could desync the protocol (message type 0x… arrived from server while idle) and wedge a thread forever.pg_advisory_xact_lock(PGMQ_INSTALL_LOCK_KEY)— cross-process serialization that self-releases at COMMIT/ROLLBACK and is safe through transaction-pooling poolers (a session lock's unlock could land on a different server connection).PG::UniqueViolation,PG::Duplicate*— a process without the advisory lock won the race) is rescued by re-checkingpgmq.meta: present → proceed as installed; absent → re-raise wrapped inSchemaNotReady.Downstream forensics: getzazu/app#3413 (merge-queue CI_TIMEOUT shard evictions).
Closes #397
Test plan
bundle exec rspec spec/pgbus/ spec/generators/— 3854 examples, 0 failures, coverage floors passbundle exec rubocopclean on changed filesSummary by cubic
Serialize PGMQ schema installation across client instances and processes to eliminate install races that caused
PG::UniqueViolation, protocol desyncs on sharedlibpqconnections, and threads wedged on socket reads. Previously each client used a per-instance guard; now installs are process- and cross-process safe.synchronizedwith a class-level mutex aroundensure_pgmq_schema.pg_advisory_xact_lock(PGMQ_INSTALL_LOCK_KEY); the xact-scoped lock self-releases and is safe with transaction-pooling poolers.PG::UniqueViolation/PG::Duplicate*as a race loss, re-checkpgmq.meta, and proceed if present; otherwise re-raise asSchemaNotReady.Written for commit 7b6d493. Summary will update on new commits.