Repository navigation
feat(worker): configurable queue-storage prune lock timeout and prune outcome metrics (0.6) - #630
Merged
Merged
Conversation
|
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
 |
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. |
![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: 73712830a5
ℹ️ 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".
Comment on lines
2613
to
+2615
| if prune_tracker.should_skip(PRUNE_BRANCH_QUEUE) { | ||
| debug!(branch = PRUNE_BRANCH_QUEUE, "Prune backed off this tick"); | ||
| self.metrics.record_prune_backed_off("queue"); |
There was a problem hiding this comment.
When a prune returns Blocked, PruneBackoffTracker::record_outcome also schedules skipped ticks, so this call emits backed_off after lock contention as well as after SkippedActive. The new metric documentation and changelog define backed_off exclusively as post-skipped_active, which makes the outcome series misleading in the exact contention scenario this feature is intended to diagnose. Either preserve the triggering outcome in the tracker or document backed_off as following both blocked and skipped_active.
Useful? React with 👍 / 👎.
…(0.6) Ring prune and terminal-delta rollup take their exclusive locks under a 10 ms lock_timeout that could only be changed through a hidden QueueStorage method. When short readers of the ring partitions hold a conflicting lock at that instant, prune reports blocked every tick and the rings stop recycling. Add ClientBuilder::prune_lock_timeout and the Python queue_storage_prune_lock_timeout_ms start() argument. The default stays 10 ms and zero is rejected, as with_prune_lock_timeout does. Make QueueStorage::with_prune_lock_timeout public and add set_prune_lock_timeout / prune_lock_timeout. awa.maintenance.prune.attempts already counts pruned, noop, blocked and skipped_active per ring; add backed_off (prune skipped by the post-skipped_active backoff) and error (prune failed with a database error) so a ring that has stopped recycling is visible for every cause.
hardbyte
force-pushed
the
brian/prune-lock-timeout-config-06
branch
from
November 6, 2026 00:16
87e5184 to
ed89d9f
Compare
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.
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.
…ut or statement_timeout
…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
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
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.
Backport of #629 to the 0.6 line.
Problem
Queue-storage ring prune (queue, lease and claim rings) and terminal-delta rollup take their exclusive locks under a
lock_timeoutof 10 ms. The value could only be changed through the hiddenQueueStorage::with_prune_lock_timeout, so operators could not tune it. When short, frequent readers of the ring partitions hold a conflicting lock at the moment prune tries, prune reportsblockedevery tick and the rings stop recycling.Change
ClientBuilder::prune_lock_timeout(Duration)(Rust) andqueue_storage_prune_lock_timeout_msonclient.start()(Python). It may be set before or afterqueue_storage. The default stays 10 ms. A zero timeout is rejected (BuildError::InvalidQueueStoragein Rust,ValueErrorin Python).QueueStorage::with_prune_lock_timeoutis now public and documented, withset_prune_lock_timeoutandprune_lock_timeout().awa_maintenance_prune_attempts_total{awa_ring, awa_ring_outcome, awa_ring_reason}already countedpruned,noop,blockedandskipped_activefor thequeue,leaseandclaimrings. It now also countsbacked_off(prune skipped by the backoff after repeatedskipped_active) anderror(prune failed with a database error).[Unreleased].Differences from #629: the 0.6 line has no
already_prunedoutcome, no ring rotation cadence and no operations skill, so the tests set up the lease and claim rings throughrotate_leases/rotate_claims, and the end-to-end metric test drives the queue ring.Validation
cargo fmt,cargo clippy --all-targets --all-features -- -D warnings(workspace andawa-python).build()) and a metric unit test covering every outcome per ring.Blockedonly after at least 300 ms, and a long timeout waits out a reader that releases after 600 ms. An end-to-end test runs a client withprune_lock_timeoutset while a reader holds the queue segment and observesblockedand thenprunedin the in-process metrics.tests/test_start_config.pycovers the new argument's validation.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
310a333de02626f25ac79c5991b848c92535d700.