Repository navigation
fix(queue-storage): promote each enqueue lane in its own transaction and hand busy lanes to a waiter (0.6) - #640
Conversation
A lane's enqueue and claim sequences were created by whichever role first touched the lane, through SECURITY INVOKER helpers, so only that role and the roles its default privileges named could use them. Another runtime role then failed with "permission denied for sequence" on enqueue, claim or promotion. Each queue-storage schema now has two SECURITY DEFINER functions owned by the schema owner, provision_lane and provision_lane_sequences. The lane helpers create a missing sequence through them, and each new sequence gets the privileges of the head table it backs. ensure_lane_inserts calls provision_lane when it exists and is executable, inside a savepoint, and otherwise writes the lane rows as before. The definer_lane_provisioning schema patch refreshes the helpers and repairs existing lane sequences one at a time, reassigning and granting only what the applying role may and reporting the rest. awa.lane_sequence_problems and the new `awa storage lane-sequences` command list what remains with the statements to run; awa.repair_lane_sequences and `--repair` rerun the repair.
…vision_lane Queue storage takes any queue name, including an empty one, and any smallint priority, so the provisioning functions only reject a null argument or a shard outside 0-63. The released-binary rehearsal now expects a 0.6.9 producer without CREATE to create a new lane, owned by the schema owner.
…and hand busy lanes to a waiter (0.6) Backport of #636. Promotion re-enqueues due scheduled and retryable rows into their lane, which takes the lane's reservation lock that producers hold until they commit. One promotion batch was a single transaction across every queue it touched, and the maintenance leader runs its branches one after another, so a lane kept busy by producers delayed due jobs of unrelated queues in the same batch and started rescue, rotation and prune late. QueueStorage::promote_due_lanes selects a batch without locking it and promotes each (queue, priority) lane's share in its own transaction, under an optional lock_timeout; a lane that hits it rolls back and is reported as contended. promote_due_lane promotes the oldest due rows of one lane. promote_due keeps its signature and promotes lane by lane with no timeout. Within a lane, promoted rows take lane sequence numbers in due order. The leader's promoter runs each tick under a 50 ms lane lock timeout, skips contended lanes for the rest of the tick, and hands each to a dedicated waiter task (at most four at once) that promotes the lane under a 30 s lock timeout. PostgreSQL grants a lock to its waiters in arrival order, so the waiter is served after the producers queued ahead of it however busy the lane stays. awa.maintenance.promote.lane_contention counts handed_off, waiter_limit and waiter_timed_out by state and queue. The overrunning-promotion test slowed the queue-storage DELETE, which no longer runs when nothing is due; it now slows the canonical-plane promotion DELETE that every body runs.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the
 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2ce3a31cf6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| match store | ||
| .promote_due_lane( | ||
| pool, | ||
| *state, | ||
| lane, | ||
| PROMOTE_BATCH_SIZE, | ||
| Some(PROMOTE_LANE_WAITER_LOCK_TIMEOUT), | ||
| ) |
There was a problem hiding this comment.
When the worker pool has only one connection—or four waiters consume all available connections—this call begins a transaction and can hold its connection for the full 30-second advisory-lock wait. If the lane lock is held by a producer using another pool or runtime, subsequent claims, rescues, and maintenance branches block acquiring a connection, so the hand-off still stalls the worker and can repeat indefinitely on timeout. Reserve pool capacity by making the waiter limit depend on the configured pool size, or run these waits through a separate bounded pool.
Useful? React with 👍 / 👎.
| ORDER BY run_at ASC, priority ASC, job_id ASC | ||
| LIMIT $2 |
There was a problem hiding this comment.
If the oldest selected deferred row is locked by another transaction, this query still counts it toward LIMIT; the later per-lane DELETE ... FOR UPDATE SKIP LOCKED then skips it. For example, promote_due(..., 1) returns zero while a newer due row is unlocked, and repeated ticks keep selecting the same locked row; with the maintenance batch size, 4,096 locked leading rows similarly prevent all later lanes from being promoted. The previous atomic selection skipped locked rows before applying the limit, so the initial selection needs equivalent lock-aware behavior.
Useful? React with 👍 / 👎.
Once a handler returned, the executor removed the attempt from the heartbeat set and finalized it in a detached task that Client::shutdown did not wait for. When the last handler returned as the drain ended, shutdown cancelled the completion batcher before that finalize reached it, and the direct-finalize fallback raced the process exit. The attempt stayed open with no heartbeat, so stale-heartbeat rescue ran the finished job again after heartbeat_staleness. Track finalizers in a TaskTracker and wait for them, within the drain timeout, before stopping the completion batcher, heartbeat and maintenance services. This is the shutdown part of #521 on main.
…ound lane waiters by pool size The batch selection was a plain SELECT ... LIMIT and only the per-lane DELETE skipped locked rows. When the oldest due rows were locked by another transaction, such as a lane waiter left running by a former leader, every batch of a tick reselected them and promoted nothing until the lock went away. The selection now takes FOR UPDATE OF deferred SKIP LOCKED. It runs as its own statement, so the row locks end with it, and deferred_jobs is not partitioned, so no run-time pruning meets them. Each lane waiter holds a pool connection while it waits. The leader now runs one waiter per eight pool connections, at least one and at most four, and the configuration docs say so.
…ded lock wait The definer_lane_provisioning patch reassigned lane sequences inside the patch transaction. ALTER SEQUENCE ... OWNER takes the sequence's ACCESS EXCLUSIVE lock until commit, with no lock_timeout, so an open enqueue on one lane could stall the patch and every repaired lane waited for it to commit. The patch now only refreshes the helpers. After it commits, awa migrate repairs each lane sequence with its own statement through awa.repair_lane_sequence(schema, sequence), which has lock_timeout = 300ms; on 55P03 the sequence is left unchanged and reported as busy. The results are in MigrationReport::lane_sequences. `awa storage lane-sequences --repair` uses the same path, and the set-returning awa.repair_lane_sequences(schema) is gone. Reassigning needs the current owner's privileges, which the schema owner lacks for sequences the migration role or an app role created. A migration role connected with role set to the owner therefore could not reassign its own sequences. The repair now retries such a sequence as the login role, lane_sequence_problems reports repair_roles (the login roles that can repair each sequence), and leftovers are reported with the next step to take. The docs state which role to run the repair as.
hardbyte
added
the
full-ci
Backport of #636 to the 0.6 line.
Problem
On queue storage, promotion moves due
scheduledandretryablerows fromdeferred_jobsinto the ready ring. Re-enqueueing a row reserves lane sequence numbers under the lane's(queue, priority, shard)advisory lock, which producers also take and hold until they commit. Two things turned one busy lane into a leader-wide stall:Change
Store (
awa-model).QueueStorage::promote_due_lanes(pool, state, batch_size, lane_lock_timeout, skip)selects the batch's due rows (sameORDER BY run_at, priority, job_id LIMITas before, so the same index and batch limit apply) withFOR UPDATE OF deferred SKIP LOCKED, groups them by(queue, priority)lane, and promotes each lane's share in its own transaction. With alane_lock_timeout, each lane's transaction sets thatlock_timeout; a lane that hits it rolls back, keeps its rows deferred, and is returned inPromoteOutcome::contended, and the other lanes go ahead.skipleaves named lanes out of the selection. The selection skips locked rows, so rows another transaction holds, such as a waiter left running by a former leader, cannot fill every batch of a tick. It runs as its own statement, so its row locks end with it.promote_due_lanepromotes the oldest due rows of a single lane.promote_duekeeps its signature and now promotes lane by lane with no timeout.Leader (
awa-worker). A per-leadership-termQueueStoragePromoterruns each promotion tick with a 50 ms lane lock timeout. A contended lane is skipped for the rest of the tick and handed to a dedicated waiter task, which promotes that lane in batches with a 30 s lock timeout. PostgreSQL grants a lock to its waiters in arrival order, so the waiter is served after the producers already queued, however busy the lane stays. That is the fairness bound: a permanently saturated lane is promoted once per drain of its lock queue, and the leader never waits on it for longer than one 50 ms timeout per newly contended lane per tick. While a lane has a waiter, the leader's ticks skip it. Each waiter holds a pool connection while it waits, so the leader runs at most one per eight pool connections, never fewer than one or more than four. A 10-connection pool therefore gives up at most one extra connection to a busy lane. A further contended lane is retried on the next tick. Dropping the promoter, on step-down or shutdown, aborts its waiters, and an aborted waiter's transaction rolls back, leaving its rows deferred.Option 1 alone (a transaction per lane) still runs the busy lane's wait on the leader task, and option 2 alone (a timeout with an escalating budget) either never wins a lock queue that is never empty or eventually blocks the leader for the whole queue drain. Handing the lane to a waiter (option 3, scoped to the lane) gives the fairness bound without the leader paying for it. Promotion as a whole stays on the leader loop, so its tick and overrun accounting are unchanged.
Constraints kept:
lane_seqorder. A lane is never promoted by the leader and its waiter at the same time. Rows within a lane now take lane sequence numbers in(run_at, job_id)order; before, they followedDELETE ... RETURNINGorder.DELETEboth lock rows only indeferred_jobs, which is a plain table rather than a partitioned one, so no statement both locks rows and relies on run-time pruning.try_begin_without_cooldown. A contended lane is retried every tick, either by its waiter or by the next tick, and is never backed off.DELETEbinds its job ids as one array, and the ready inserts are still array/unnestshaped.Metric.
awa.maintenance.promote.lane_contention(awa_maintenance_promote_lane_contention_total{awa_job_state, awa_job_queue, awa_promote_outcome}) has the outcomeshanded_off,waiter_limit(every waiter busy) andwaiter_timed_out(a waiter's 30 s ran out). Waiter batches are recorded in the existingpromote_batches/promote_batch_size/promote_durationinstruments.Docs. CHANGELOG
[Unreleased], architecture (enqueue/promotion), configuration (deferred promotion), the Grafana metric reference, and the operations skill.Measurements (0.6)
Same harness and method as #636: private
postgres:17-alpine/postgres:18-alpinecontainers, two rounds with the before and after binaries run in alternating order. Before =release/0.6.6(promote_duebatches until short); after = this branch (the leader'sQueueStoragePromoter). Cells are round 1 / round 2, in ms unless marked. The second round of the before binary ran on a busier host, which is why its spread is wide.* Rescue did not run at all during the 12 s and 27 s windows: one promotion tick held the leader for the whole run.
With no producers, the unrelated-queue lag p99 was 334–847 ms before and 378–580 ms after. As on main, the lag left after the change comes from the leader's own rescue run on the same task, not from the hot lane. The one after-change outlier (1814 ms, PG18 held, round 2) coincides with a 1.24 s rescue body.
Validation
cargo fmt --all,SQLX_OFFLINE=true cargo clippy --all-targets --all-features -- -D warnings.queue_storage_runtime_test(PG17 and PG18 locally):test_queue_storage_promotion_leaves_a_contended_lane_deferred_and_in_order: with the hot lane's lock held, a batch promotes the unrelated queue, returns the hot lane as contended after one lock timeout (bounded withscaled_timeout), and leaves its rowsscheduled. A batch that skips the lane does not touch it. Promoted later, two rows at a time, the lane's rows takelane_seqin due order (job ids deliberately in the opposite order), and a claim returns all of them.test_queue_storage_runtime_busy_lane_holds_back_neither_promotion_nor_rescue: a full client, with two sessions taking turns holding the hot lane's lock for 5 s each so that the lane is never free. A due job on another queue completes, and an abandoned attempt is rescued, withinscaled_timeout(2 s). The hot queue's due jobs still complete while the lane stays busy, which is the fairness bound. The test fails without the lock timeout (the unrelated job times out), and it fails without the hand-off (the hot jobs are never promoted).test_queue_storage_runtime_locked_due_rows_do_not_starve_promotion: another session holds row locks on the 4,160 oldest due rows, more than one batch selects, as an orphaned waiter would. A due job on another queue still completes withinscaled_timeout(2 s), and the locked rows are promoted once released. WithoutSKIP LOCKEDon the selection, every batch reselects the locked rows and the other job never runs.test_queue_storage_runtime_promotion_not_suppressed_after_overrunning_bodies(fix(worker): never back promotion off, and make branch cooldowns wall-clock #620) slowed promotion with a statement trigger on the queue-storagedeferred_jobsDELETE. ThatDELETEnow runs only for lanes with due rows, so the trigger moved to the canonical-plane promotionDELETE(awa.scheduled_jobs), which every promotion body still runs once per state.Notes
(state, run_at, ...)index. While a lane with a large due backlog is waiting, each tick's selection scans past that backlog. In the 24k-row runs that cost about 5–12 ms per tick, and the old path paid far more by blocking on the lane.Differences from #636
&format!, since 0.6 has noaudited_sql. The laneDELETEis bound to a local because its binds depend on the row selection.active_standby_queue_storage()guard.scaled_timeout; 0.6 has noawa/tests/ci_timing.rs.QueueStoragePromoter.There is no schema change and no migration. Only the elected leader promotes, so in a mixed 0.6/0.7 fleet the behaviour follows whichever version holds leadership.
Independent review and validation
Includes the small #651 point-read fix. After a priority or queue move crossed a ready-ring wrap, retained tombstoned source rows could make
get_jobreturn old fields even after the batch succeeded. The available-candidate query now excludes full-identity tombstones (a 13-line SQL change), with priority and queue ring-wrap regressions. Both regressions fail before the fix and pass after it on PostgreSQL 17 and 18; existing aged-read and batch cases also pass. Required formatting, Clippy across all targets/features, and workspace build checks pass for the prepared source.Full CI passed all 29 jobs for
e8fbc4367c27edb81c7a03e9624c1812eca44b52.