Repository navigation
test: keep completed rows past ring rotation in job output and staged tests - #588
Merged
Merged
Conversation
… tests
Queue-storage clients built with the default settings rotate the ready
ring every second. While a test is still running, or inside the two-second
shutdown window, maintenance can rotate the ring and prune completed rows,
so a later admin::get_job or count of a terminal job finds nothing. This
made external_completion_stores_the_output_it_is_given fail intermittently
in CI with JobNotFound { id: 1 }, and the same race affects
failed_or_late_staging_never_commits_under_queue_storage, which counts a
completed trigger after it finishes.
job_output_test.rs gains a client_builder helper that sets a one-hour
queue and lease rotation, as completion_stores_the_output_on_both_terminal_paths
already did by hand, and every test that reads terminal rows after
completion uses it. The shared client_builder in staged_follow_up_test.rs
sets the same rotation for its queue-storage branch. No test in either
file relies on rotation or pruning.
hardbyte
added
the
full-ci
Claude
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 19 minutes. View limit details
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. CodeRabbit Verify
|
hardbyte
pushed a commit
that referenced
this pull request
Nov 3, 2026
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.
This was referenced Nov 3, 2026
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.
hardbyte
added a commit
that referenced
this pull request
Nov 3, 2026
…rotation race (#590) awa storage prepare-queue failed with permission denied for sequence on a lane whose sequences a 0.6 runtime with schema CREATE had created: 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. ensure_lane_inserts now inserts only head rows that are missing, under the same per-lane advisory lock. least_privilege_runtime_test covers it and fails on main without the fix. Found by the 0.6.10 upgrade rehearsal. The failure-path, dashboard and trace telemetry tests build their clients with a one-hour queue and lease rotation, so maintenance cannot prune completed rows before a test counts or reads them, as #588 did for the job output tests.
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.
Summary
job_output_test::external_completion_stores_the_output_it_is_givenfails intermittently onmainwithJobNotFound { id: 1 }atjob_output_test.rs:495. It failed 4 of 6 recentmainCI runs, and is the only red check on #586.Cause. The test's client uses the default one-second ready-ring rotation. While the test runs, or during the two-second
client.shutdown, maintenance can rotate the ring and prune the completed rows, so the nextadmin::get_jobfinds nothing.staged_follow_up_test::failed_or_late_staging_never_commits_under_queue_storagehas the same race (diagnosed on One job filter, and tags as the indexed handle (ADR-049) #572): it counts a completed trigger after it finishes.completion_stores_the_output_on_both_terminal_pathsalready avoided it by hand, with a one-hour rotation.Change. Test-only, two files.
job_output_test.rsgains aclient_builderhelper with a one-hour queue and lease rotation. Every test that reads jobs or terminal rows after completion uses it.client_builderinstaged_follow_up_test.rssets the same rotation for its queue-storage branch.NeverInserted,Oversized,Rejected) become stricter, because pruning could previously make them pass for the wrong reason.Left as they were in
job_output_test.rs:canonical_discards_outputs: canonical storage, no ring.an_explicit_output_descriptor_is_keptanddescriptors_sync_without_the_output_column: they read descriptors only.typed_external_completion_checks_the_kind: it reads the job only while it iswaiting_external.a_flush_split_by_output_size_keeps_every_output_with_its_jobandrescue_keeps_the_earlier_output_and_a_stale_completion_stores_nothing: they driveQueueStoragedirectly, with no maintenance running.Evidence
A temporary 3 s sleep was added while each client was still running: before
client.shutdownin the external-completion test, and before thecount_kindassertions in the staged test.external_completion_stores_the_output_it_is_givenJobNotFound { id: 1 }, the CI errorfailed_or_late_staging_never_commits_under_queue_storageonly the trigger itself(left 0, right 1)With the sleeps removed,
cargo test -p awa --test job_output_test --test staged_follow_up_testpassed twice: 15/15 and 17/17. fmt and clippy (-p awa --tests -D warnings) are clean.Generated by Claude Code