Repository navigation
Read queue overviews from live work on queue storage - #580
Merged
Merged
Conversation
admin::queue_overviews counted every current job through terminal_jobs, which expands each completion batch and joins it back to ready_entries, so its cost grew with retained history: 22 s in one production deployment, polled by an open admin dashboard. queue_state_counts is no answer: the maintenance leader stops maintaining it once queue storage is active. The workers' queue gauges already read only live-work tables. That query moves into awa-model as queue_signals_sql and serves both. The admin variant also counts receipt-backed running claims, counts available rows exactly along each lane's primary key, and takes completions in the last hour from completion-batch sizes and done_entries. With 100k retained completions and 100k queued jobs, the overview drops from 7.7 s to 0.27 s; JIT is off for it, since compiling the statement cost 1.2 s.
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. |
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 41 minutes. View limit details
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Queue signal query modes awa-model/src/queue_storage.rs |
Adds QueueSignalReads::Gauge and QueueSignalReads::Admin, plus queue_signals_sql to derive queue-level signals without reading terminal_jobs. |
Overview and health metric integration awa-model/src/admin.rs, awa-worker/src/maintenance.rs, awa/tests/queue_storage_runtime_test.rs, CHANGELOG.md |
Queue overviews use admin signals, and worker health metrics use gauge signals. The integration test checks overview counts with stale canonical counts, recent completions, and a tombstoned ready row. The changelog records the change and reported benchmark timings. |
Estimated code review effort: 3 (Moderate) | ~20 minutes
Change: Refactor
Suggested reviewers: claude
Merge Risk: 
e3421
-
No actionable issue remains from this review; the change is ready for normal merge checks.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta… Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title "Read queue overviews from live work on queue storage" matches the main change. The PR changes admin::queue_overviews on queue storage to read live-work tables (via queue_signals_sql) instea… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Title check
-
Explanation
The title "Read queue overviews from live work on queue storage" matches the main change. The PR changes admin::queue_overviews on queue storage to read live-work tables (via queue_signals_sql) instead of expanding retained terminal-job history, and it moves the gauge query into awa-model so both reads share it. The title is specific, concise, and free of file lists or vague terms.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 2 functions across 2 files. (3 skipped: 1 unsupported, 2 too large.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches
-
Generate docstrings
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
A rabbit checks the queue at dawn
Five finished hops are neatly drawn
Two ready carrots wait in line
No running feet cross the design
The queue signals shine and rhyme
![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: e34211ab77
ℹ️ 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".
hardbyte
added
the
full-ci
Claude
A deleted or retried compact completion keeps its batch unchanged and gains a tombstone, which terminal_jobs honours. Subtract tombstones on members of batches finalised within the hour; such a tombstone is itself newer than the hour, so only recent tombstones are matched.
This was referenced Nov 2, 2026
hardbyte
added a commit
that referenced
this pull request
Nov 2, 2026
…backport of #584) A review of the #580/#581 backport found the last-hour completed count scanning completion batches once per recent tombstone (5.4 s on a production-sized fixture), and queue lag reading each lane's first row, which hides an older job a queue move or priority aging appends behind newer ones. - Available counts walk each lane's segments from its claim cursor and probe the ready key per segment; a range without the key's leading ring slot scanned every retained ready row. They also yield the oldest available run_at, which is the queue lag again, as in 0.6.9. - The tombstone check expands each recent batch in a tombstoned lane once and matches its members by hash. - The timeseries reads its window's terminal rows with ready-key probes instead of through terminal_jobs, which joined them to every ready row. - DLQ retry, purge and move invalidate the dashboard caches, as job mutations already do (from #579). admin_read_cost_test serves every polled endpoint against ten times more retained history and fails if a read does more work; on the previous head it fails for /api/queues and the timeseries.
hardbyte
added a commit
that referenced
this pull request
Nov 3, 2026
* fix(queue-storage): let restricted runtimes use pre-provisioned lane sequences (0.6 schema patch) Backport of #501 to the 0.6 line. The lane helpers and head-sync triggers ran CREATE SEQUENCE IF NOT EXISTS even when the sequence existed, which requires schema CREATE and defeats the documented migrator/runtime role split. They now skip the DDL when the sequence exists and raise SQLSTATE 42501 with a provisioning hint when it does not and the role lacks schema CREATE. 0.6 cannot add migration versions, so the change ships as the idempotent schema patch prepared_lane_sequences: it redefines awa.install_queue_storage_substrate and re-runs it for every installed queue-storage schema with its existing slot counts and receipt mode (lease_claims or lease_claim_batches in claim_ready_runtime). Fresh installs pick up the helper through v023. v023 is split in two files so the patch embeds only the helper definition: the default-schema installer, legacy fixups and schema_version row stay in v023_install_default_queue_storage_substrate.sql, which V23_UP still appends. The patch lint now skips the installer body, whose guarded ALTER/DROP TABLE statements run only when the function is called. Add QueueStorage::prepare_queue and `awa storage prepare-queue` to provision all priorities, shards and stripes as the migrator. * docs: restricted runtime roles, prepare-queue and sequence UPDATE grant * docs: shorten the prepared-lane changelog entry * fix(admin): read queue-storage dashboard counts from live work, not retained history Backport of #580 and #581 to the 0.6 line. On queue storage, admin::queue_overviews, state_counts and state_timeseries built every current job through queue_storage_current_jobs_cte, whose terminal_jobs branch expands each completion batch and joins it back to ready_entries, so their cost grew with retained history. One production deployment saw queue_overviews take 22 s, polled by an open admin dashboard. queue_state_counts is not maintained once queue storage is active, so it can't stand in. - queue_overviews reads the live-work tables the worker gauges read, counts available rows exactly per lane from its claim cursor, and takes completed_last_hour from completion-batch sizes (less tombstoned members) and completed done_entries rows. - state_counts counts completions from batch sizes less tombstones, and available rows per lane. - state_timeseries bounds every source by the window before any join; terminal rows come only from done rows and batches finalised within it. - JIT is off for all three; compiling cost more than running. Running keeps 0.6's leases-only definition. * chore: prepare 0.6.10 release * docs: grant sequence UPDATE in the pgroles example too Lane cursors need UPDATE on their sequences for setval; the SQL grant block already said so, the pgroles manifest did not. * chore: bump async-trait to 0.1.92 for clippy's double_must_use Rust 1.99's clippy flags the must_use that async-trait 0.1.89 puts on the boxed future of Worker::perform. main already locks 0.1.92, whose expansion passes; the 0.6 lockfiles follow it. * ci: run the Playwright E2E job on ubuntu-24.04 ubuntu-latest now resolves to 26.04, where the pinned Playwright refuses to install Chromium. main pins the job the same way. * fix(serve): stop abandoned admin requests from holding database connections Backport of #583. When a client gives up on an admin request, the server drops it but Postgres keeps running its query, and sqlx holds the connection until the query ends. If the query then ended in an error, sqlx's release check closed the connection with a hard close, which hangs on a TLS connection, so the pool lost that slot for good (sqlx 0.8.6 here, as 0.9 on main). awa serve now builds its pool with awa_ui::admin_pool_options and admin_connect_options: a 60 s statement timeout as a startup option (--statement-timeout / AWA_STATEMENT_TIMEOUT), a release hook that reads an abandoned query's result and keeps the connection, and client_connection_check_interval = 5s on Postgres 14+. * Round sub-millisecond admin statement timeouts up to 1ms * Round admin statement timeouts up from nanoseconds * fix(queue-storage): replace only the lane helpers in the prepared-lane patch The 0.6.10 prepared_lane_sequences patch re-ran awa.install_queue_storage_substrate for every installed queue-storage schema. The installer takes exclusive locks on hot tables such as leases and deferred_jobs, rebuilds the head-sync triggers and scans retained ready entries for backfills, all inside the migration transaction. Under live load on a v0.6.9 database it stalled every worker and producer for 23 s and deadlocked one enqueue. The three lane helpers (ensure_lane_sequences and the two head-sync trigger functions) now live in awa.refresh_lane_sequence_helpers(schema), as on main. The installer calls it, so new schemas get the fixed helpers. On existing databases the patch calls only the refresh, under the per-schema install advisory lock. It takes no table locks and reads no queue rows: the same live rehearsal applied it in 22 ms with no completion gap and no errors. The applied probe now checks all three helpers and the refresh function. A new test asserts the patch holds no relation lock on queue-storage objects, and that the installer does. * fix(admin): keep queue-storage dashboard reads off retained history (backport of #584) A review of the #580/#581 backport found the last-hour completed count scanning completion batches once per recent tombstone (5.4 s on a production-sized fixture), and queue lag reading each lane's first row, which hides an older job a queue move or priority aging appends behind newer ones. - Available counts walk each lane's segments from its claim cursor and probe the ready key per segment; a range without the key's leading ring slot scanned every retained ready row. They also yield the oldest available run_at, which is the queue lag again, as in 0.6.9. - The tombstone check expands each recent batch in a tombstoned lane once and matches its members by hash. - The timeseries reads its window's terminal rows with ready-key probes instead of through terminal_jobs, which joined them to every ready row. - DLQ retry, purge and move invalidate the dashboard caches, as job mutations already do (from #579). admin_read_cost_test serves every polled endpoint against ten times more retained history and fails if a read does more work; on the previous head it fails for /api/queues and the timeseries. * fix(admin): find unique-key cancel candidates without scanning retained ready rows (backport of #586) On queue storage, cancel_by_unique_key and cancel_by_unique_key_tx built every available ready row before filtering by key, reading every retained ready row, and missed jobs running on receipt claims not yet materialised into leases, so those couldn't be cancelled by key (#585). The key is now filtered where each source is read: available rows are walked lane by lane from the claim cursor through ready_segments (only the given queue's lanes when a queue is passed), scheduled and retryable rows use the deferred unique-key index, and running jobs come from leases and open receipt claims. open_receipt_running_claims_sql is ported from main, with the closed-claim-batch skip. The queue-storage tests from #586 cover each live state, oldest-first among jobs sharing a key, the tx variant's rollback and commit-time notice, and rows read staying flat as history and other queues' backlogs grow. Against the previous lookup they fail: the running job isn't found, and a scoped missing-key lookup reads 720 rows, then 5,206. * Cover a waiting_external job in the cancel-by-key test Port of the #586 follow-up: enter the callback wait so the test cancels a job in waiting_external, and note that the open-receipt-claim leg costs claim-ring retention. * fix(migrate): count the lane patch applied only when every substrate has it The prepared_lane_sequences probe checked the three helpers in awa only. A custom queue-storage schema restored or prepared with the 0.6.9 helpers passed once awa did, and a database whose only active substrate was a custom schema could never pass. The probe now checks the refresh function, the installer and the helpers of every installed substrate, from the catalogs so a schema the migrating role cannot use is reported. migrations::schema_patch_status names what each missing patch still has to repair, and migrations::run_with_report returns the schema versions, the migrations and patches applied, what each patch repaired or still lacks, and storage routing before and after. * fix(cli): make awa migrate safe to run for the 0.6.10 patch - Refuse --from, --to and --version when applying; they only select a range to render, and the apply path always migrates fully. - Wrap --sql output in one transaction that takes the migration advisory lock, with ON_ERROR_STOP guidance in the header; --no-transaction prints it bare for runners that own the transaction. - Report the result: schema version, each patch and what it updated, and storage routing before and after. Exit 1 when a patch is still missing on some schema, naming it. Tests cover the refused flags, a patch interrupted mid-run (nothing applied, retry completes it) and exported SQL that fails under psql with ON_ERROR_STOP and then applies cleanly. * fix(queue-storage): provision lanes a 0.6.9 runtime created A runtime role with schema CREATE owns the lane sequences it creates. Provisioning such a lane as the owner (awa storage prepare-queue, the first step of revoking runtime CREATE) failed with permission denied for sequence: the head tables' BEFORE INSERT triggers run even for a row ON CONFLICT DO NOTHING then skips, and they read and set the lane's sequence. Lane inserts now skip rows that exist, under the same per-lane advisory lock. * test: rehearse the 0.6.9 -> 0.6.10 -> 0.6.9 upgrade as a release gate Released 0.6.9 artifacts (the awa-cli and awa-pg wheels) install the schema as an owner role and run a separate runtime role in a custom queue-storage schema with the grants a working 0.6.9 deployment needs. Over a banked backlog with retries, scheduled jobs and slow jobs in flight, the candidate awa migrate patches under traffic, with no lock above RowExclusive on any awa or queue-storage relation (one owner transaction audit plus sampling during the real run), idle lanes at their exact positions, live lanes only advancing, and routing unchanged. Candidate and released workers then run together through killed and restarted workers; the fleet rolls back to 0.6.9 (including its awa migrate, which must leave the patch in place); privileges are tightened as the owner and the released workers restart. Every job must complete, retried jobs on a later attempt, with per-queue counts matching what was enqueued. CI runs it on PostgreSQL 17 and 18 alongside the other full-CI jobs; release.yml already requires this workflow to pass on the tagged SHA. Evidence is uploaded as JSON. * docs: 0.6.9 -> 0.6.10 upgrade guide One path: install the 0.6.10 CLI, run awa migrate once as the owner under traffic, roll processes. States what does not need doing, what the report means, how external runners apply the patch, how to roll back (leave the patch installed), and the optional procedure for revoking runtime CREATE, with a per-sequence grant because GRANT ON ALL SEQUENCES fails on sequences a 0.6.9 runtime created. * test: resolve a relative candidate CLI path against the workspace cargo runs the rehearsal from awa/, so AWA_CANDIDATE_CLI=target/debug/awa pointed at awa/target. Resolve it, and a relative CARGO_TARGET_DIR, from the workspace root as the released-artifact paths already are. * fix(admin): start a queue-scoped unique-key lookup from the key The 0.6.10 lookup read every open receipt claim before filtering by key. A long-running job keeps the claim ring from pruning, so on a database with 120,000 retained row-local claims each lookup took about 9.7 s, worse than 0.6.9's 2.8 s on the same data. Given a queue, the lookup now reads that queue's ready rows holding the key through the lane index and checks only those jobs: available past the claim cursor and not tombstoned, in leases, or on an open receipt claim found by job_id. With no such row only the deferred_jobs key index can hold the key, so that query alone answers, which also skips planning the full lookup across partitioned tables (about 10 ms). Measured on 180,200 retained ready rows and 120,300 claims in other queues, through cancel_by_unique_key_tx rolled back: 1.7 ms for a missing key, 11 ms and 20 ms to cancel a scheduled and an available job (p50 of 100); 0.6.9's lookup took 2.8 s. The cost test now also retains row-local claims; against the previous lookup it reads 434 rows, then 32,912. * fix(admin): find a moved running job by key, in one snapshot Review follow-ups on the queue-scoped unique-key lookup: - Match leases by job_id, as open receipt claims already are. A job moved to another queue while available is leased on its new lane, so matching by its original ready row's lane missed it once it ran or waited there; 0.6.9 found it. - Read the ready-row check and the deferred_jobs lookup in one statement, so a job promoted from deferred_jobs between two reads is not missed by both. - Say what the compact-batch leg of the job-restricted open-claims query reads: every retained batch header, unnesting only batches that hold one of the jobs. New test: two jobs moved to another queue, one running on a receipt claim and one waiting on a callback, are both found by a lookup naming their original queue. * Hide the storage transition card once the transition is finalized The runtime page showed the card on every cluster running queue storage, long after the cutover finished. It now appears only while an engine is prepared or a mixed transition is in flight. * test: keep completed rows past ring rotation in telemetry tests test_failure_path_metrics_reach_prometheus timed out waiting for its three completed jobs: with the default one-second ring rotation, maintenance can prune completed rows before the test counts them. The failure-path and dashboard tests, which both count completed jobs, now build their clients with a one-hour queue and lease rotation, as #588 did for the job output tests on main. * test(rehearsal): accept a fail_once job whose first attempt died with its worker The rehearsal failed on PostgreSQL 17 with "fail_once job 331 never retried". A worker killed after claiming a fail_once job but before its handler returns leaves no Retry event; heartbeat rescue then runs attempt 2, which completes as designed. The accounting now requires a Retry event only when attempt 1 started on a worker that survived, and still requires every fail_once job to complete on attempt 2 or later. * fix(admin): find striped lanes and tolerate a finished job in unique-key cancel The admin store is rebuilt from the active schema name with the default config, so a queue-scoped unique-key lookup bound only the logical queue name and missed available, running and waiting jobs in a striped queue's queue#N lanes. The physical queues now come from the schema's queue_lanes. A job that finishes between the lookup and the cancel made the queue-storage branch return JobNotFound; it now returns None, as the canonical engine does and the docs say. Both predate 0.6.10. Port of the same fixes on main (#586); the new striped-lanes test fails on the previous lookup. * docs(0.6.10): external-runner patch check, absolute upgrade-guide link, release date A runner that tracks only schema versions sees nothing new in 0.6.10, and the exported patch skips queue-storage schemas the running role cannot use without failing. The upgrade guide now says to add the R__ script, run it as the owner and confirm with awa migrate --sql --pending. The CHANGELOG links the guide absolutely so the link works in the GitHub release body.
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.
Problem
admin::queue_overviewssits behind the admin UI's queues and dashboard pages,awa queue statsand the Python queue stats. On queue storage it counted every current job throughqueue_storage_current_jobs_cte. That CTE includesterminal_jobs, which expands each completion batch and joins it back toready_entries, so the cost grows with retained history rather than live work.In one production deployment this statement took 22 s. It was 14.7% of database runtime, and an admin dashboard left open re-ran it every few minutes.
queue_state_countsis not a fix: the maintenance leader stops maintaining it once queue storage is active (recompute_dirty_admin_metadataandrefresh_admin_metadatareturn early). On a migrated cluster it holds whatever the canonical engine last wrote.Change
leases,deferred_jobs,done_entriesanddlq_entries. That SQL moves intoawa-modelasqueue_storage::queue_signals_sql, and the gauge andqueue_overviewsboth use it.Gaugereads are the gauge's existing query, unchanged. They never touch the receipt-claim tables, so a gauge can't block on claim-ring rotation's locks.Adminreads count three things the gauge doesn't:leases, throughopen_receipt_running_claims_sql, as before;completed_last_hour, from completion-batch sizes plus completeddone_entriesrows, with nothing unnested.The output is unchanged. Queues come from the same sources as before, and
failedis still done-failed plus DLQ rows.Measurements
The test data is 100,000 retained completions (two thirds of them tagged, so about two thirds sit in
done_entriesand the rest in compact batches) plus 100,000 queued jobs, created withtag_filter_bench's loader.queue_overviews, warm medianThe largest remaining cost is counting open receipt claims, about 190 ms here. It grows with retained claim batches, as it already did in
state_counts.Validation
cargo fmtis clean, andclippy -D warningsis clean onawa-model,awa-workerandawa.queue_storage_runtime_testpasses 194/194. The new testtest_queue_storage_queue_overview_counts_without_terminal_historycovers:queue_state_countsrow, which is ignored;availableexact where the sequence-head signal would still count it.queue_storage_metrics_query_uses_bounded_observability_pathtest passes, so the gauge still can't block on claim-ring locks.Follow-ups
state_countsandstate_timeseries.job_kind_overviews,distinct_kinds,distinct_queuesanddrain_queue's id lookup still readcurrent_jobson queue storage.Generated by Claude Code
Summary by CodeRabbit