Repository navigation
Conversation
reserve_enqueue_seq read the lane's sequence name only after calling ensure_lane_sequences, which on every reservation computed two sequence names, ran two to_regclass lookups and two no-op UPDATEs of the lane heads. It now reads the name first and provisions only a lane row that has none. A multi-row range was reserved with nextval followed by setval, two sequence WAL records; it is now one setval under the lane lock that every writer of the sequence already holds, and a one-row reservation is a bare nextval. The definition moves into awa.refresh_enqueue_functions, called by the installer and by the v052 tail for every discovered substrate. Signature, lock key and return value are unchanged, so N-1 binaries and insert_job_compat call it as before. v050 and v051 are reserved for parallel branches.
The reserve path probed EXISTS on queue_enqueue_heads for a cached lane before reserving, but advance_enqueue_head already reads the head row and repairs a stale cache entry when it is missing, so a cached lane now goes straight to the reservation. The cancel paths that call ensure_lane without reserving keep the probe. The enqueue cursor read fetched the ring authority row on every call. Once a store has read ledger authority it reads the ledger cursor directly: the flip to ledger is one-way. prepare_schema and reset clear the observation.
A ready-only enqueue made ten round trips: BEGIN, job ids, the lane
probe, the reservation, the ring authority and cursor reads, the ready
and segment inserts, NOTIFY and COMMIT. v052 adds
{schema}.enqueue_ready_rows, which for each lane in the caller's lock
order provisions a missing head with the same lane lock and inserts as
ensure_lane_inserts, reserves through reserve_enqueue_seq, then reads
ring_cursor('queue'), appends the ready rows before their segments
(NULL job ids take the job id sequence) and notifies.
enqueue_params_copy and enqueue_params_batch call it in autocommit when
every row is ready, none takes a uniqueness claim and the batch is below
the COPY threshold: one round trip and one transaction. Inside a caller
transaction (uniqueness claims, deferred rows, enqueue_runtime_rows) it
replaces the per-lane ensure and reserve, the cursor reads and both
inserts. Batches of 2,048 rows or more keep COPY. The store probes once
for an executable function and keeps the multi-statement path on a
schema without it.
The one-write reservation read the sequence through a dynamic statement built per call. pg_sequence_last_value and setval take the sequence as a regclass, so the read and the write are static; sequence_next_value still covers a sequence that has not been called yet. Server time per 20-row reservation on PostgreSQL 17: 107 us on main, 55 us with the dynamic read, 30 us now; WAL per reservation stays at one sequence record.
…equences Counts ensure_lane_sequences calls inside a reservation with transaction-level function statistics: none for a named lane, one for a lane row whose sequence name is NULL. Fails against the pre-v052 reservation, which provisioned on every call.
…d version The newer-schema guard test deleted the newest version row and expected CURRENT_VERSION - 1. Versions are not contiguous (v008 is skipped and v050/v051 are reserved), so it now expects the highest listed version below CURRENT_VERSION.
…ck' into brian/enqueue-fold
The enqueue cursor read skipped the authority row once the store had seen ledger, an observation that could come from a caller's transaction that flipped the authority and rolled back. The cursor is now one statement that branches on the authority row inline: the same single round trip, no cache, and only the table privileges the old path used. The enqueue_ready_rows probe was also permanent. A missing function is now probed again after a minute, so a runtime started before v052 picks it up without a restart, and a call that fails with undefined_function or insufficient_privilege forgets the observation so the next enqueue re-probes and falls back. The probe also requires EXECUTE on ring_cursor, which the function calls and which is revoked from PUBLIC.
…ck' into brian/enqueue-fold
…nqueue-fold # Conflicts: # CHANGELOG.md # awa-model/src/migrations.rs
…den the reserve helper v052 sits above v050 in the runner, which only applies versions above the highest recorded, so the branch now carries #527's v050 and cannot land first. The upgrade guide no longer implies binary-first is safe for health-gated workers: /readyz waits for the schema to reach the binary's version. It also describes the current probe (re-probed after a minute, EXECUTE on ring_cursor required, a lost function failing one call), and the changelog distinguishes enqueue_params_batch from the COPY entry point's 2,048-row threshold. reserve_enqueue_seq rejects a NULL count, which returned a start without advancing the sequence. The lane-cache rollback test also runs with the function disabled, since only the multi-statement path fills that cache. The least-privilege test covers a runtime denied ring_cursor EXECUTE and EXECUTE revoked after use.
Rewrites the v052 changelog entry from the user's side while resolving the changelog conflict.
# Conflicts: # CHANGELOG.md
hardbyte
added
the
full-ci
Claude
|
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 32 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. |
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. |
… analyses Autovacuum does not analyse partitioned tables, so the thirteen ring parents have no inherited statistics, and the two head tables (queue_claim_heads, queue_enqueue_heads) never reach the autoanalyze threshold, so the planner prices lane joins against them at its defaults. The maintenance leader now runs ANALYZE on them when it is elected and every five minutes (ClientBuilder::statistics_refresh_interval, Python statistics_refresh_interval_ms; zero disables). One relation per transaction, with a bounded lock_timeout and statement_timeout, partitioned parents with SKIP_LOCKED, in a task beside the leader loop so a slow ANALYZE never delays rotate, prune or promote ticks. Migration v053 adds awa.analyze_queue_storage_relation(), a SECURITY DEFINER function so a runtime role that does not own the tables can refresh statistics without a MAINTAIN grant. A missing function or an owner that may not analyse the tables is logged once and recorded in awa.maintenance.analyze.attempts without affecting job processing.
…maintenance Many concurrent producers batch-inserting into one queue lane serialise on the per-lane reservation lock, which a transaction holds until it commits. The new enqueue_lane_contention_test reports per-lane throughput and batch latency percentiles at high producer concurrency, with and without unique keys, across enqueue_shards, with a unique-conflict replay mix, and optionally drains every shard afterwards. Its nightly gate runs sixteen producers on one lane (and on four shards), checks lane integrity, and asserts contention-scaled floors; the rate and p99 regression thresholds live in benchmarks/baseline.json. enqueue_lane_maintenance_coupling_test measures a leader-shaped loop (promotion and heartbeat rescue on one task) with and without producers saturating a lane, and with a lane lock held for a fixed time: promotion batch latency, promotion lag on an unrelated queue, and how late rescue starts.
insert_many and insert_many_copy called awa.insert_job_compat once per row. The function takes the (queue, priority, shard) lane lock in reserve_enqueue_seq on the batch's first row, and the lock lasts to commit, so it was held while every remaining row was processed. A 185-row batch held it for about 40 ms on a local database, and producers on one lane queued behind whole batches: the lane's ceiling was roughly 4,000 jobs/s at any producer count, and a producer's call took (producers x 40 ms). Migration v053 adds awa.insert_jobs_compat_batch. It allocates job ids, claims unique keys in key order, registers new lanes and builds the rows before it reserves each lane's sequence range once, in (queue, priority, shard) order, then writes the ready rows and segments in two statements. It proposes head rows only for lanes that do not exist yet, because the head tables' row trigger takes the lane lock before the conflict check. insert_many and insert_many_copy call it. insert_many_copy now skips unique conflicts in one statement instead of one savepoint per row, and insert_many_skip_unique_conflicts exposes the same behaviour for callers that replay work: after a unique conflict they inserted the batch again one job at a time, taking the lane lock once per job. The lane-order test now parks a batch on one lane and checks which lane locks it holds, since job ids no longer follow lock order.
…by leaders, report lock skips - Analyse only the known ring parents, not every partitioned table in the schema, so a substrate sharing a schema with application tables never analyses them. - A leader that resolved canonical at start but holds the queue-storage runtime as standby refreshes the queue-storage schema once routing flips to it, checked on every pass. - Take the parent's SHARE UPDATE EXCLUSIVE lock before ANALYZE (SKIP_LOCKED), so a conflicting lock on the parent fails the call with lock_not_available (recorded as skipped_lock) instead of skipping every partition and reporting analyzed. Correct the documentation of what SKIP_LOCKED covers. - Link the pull request from the migration header. - Tests: refresh on election before the first interval, both rejection guards of the helper from the runtime role, and a locked parent.
…t the batch function insert_many and insert_many_copy used awa.insert_jobs_compat_batch unconditionally, so a binary running ahead of migration v053 failed every batch insert. They now look for the function in the catalog and use the per-row path until it exists. The per-row statement asks the catalog as it inserts, the process keeps the answer, and a call that finds a cached function gone fails once and resets it. insert_many_skip_unique_conflicts needs the function and reports a schema without it. Tests run a binary against a schema without the function, through the migration appearing and the function disappearing again, and mix producers on the per-row and batch functions on one lane.
…scovery preflight The fail-closed test rewound the schema by the newest migration and expected the rerun to hit the substrate-ownership preflight. A migration that touches no custom substrate, such as the batch insert function, carries no preflight, so the test now picks the newest migration that does.
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. v053 refreshes the helpers and repairs existing lane sequences one at a time, reassigning and granting only what the migrating 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.
…may analyse - Revoke EXECUTE on awa.analyze_queue_storage_relation from PUBLIC. The SHARE UPDATE EXCLUSIVE lock it takes lasts until the caller's transaction ends, so any role could hold it open and block ring prune. The migration grants EXECUTE to the roles that already hold INSERT, UPDATE and DELETE on the awa head tables, and the function requires the calling session user to hold those privileges on the target schema. - Give the function its own lock_timeout. - Accept only the two head tables and the thirteen ring parents inside the function, not any partitioned table of the schema. - Probe for the function once per pass, so a binary ahead of the schema makes one catalog query instead of fifteen failing calls and server ERROR lines. Pass-level outcomes use a new "pass" metric scope. - Correct the header, which claimed the function takes no lock. - Tests: a role without EXECUTE and a role without head-table privileges are denied, the migration grants EXECUTE to existing grantees, non-ring tables are rejected, and a missing helper costs one probe and no calls.
…ded lock wait v053 reassigned lane sequences inside the migration 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 whole migration and every repaired lane waited for the upgrade to commit. v053 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. `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.
|
21 tasks
Owner
Author
|
Parking this Generated by Claude Code |
hardbyte
marked this pull request as draft
November 7, 2026 21:04
This branch has not been deployed
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.
Fixes aged running jobs disappearing from
awa.jobs(#569), carrying the unchanged claim/lease identity design from #570. Receipt claims and materialized leases join their exact job attempt rather than comparing effective and lane priorities; tombstoned and older-attempt rows remain excluded.This is the canonical landing branch for #570, whose head is an ancestor with the same jobs-view SQL. There is one shared migration, now v057, after #637 (v053), #631 (v054), #635 (v055), and #538 (v056). Existing main migrations v051 and v052 are preserved. Merge those prerequisites first; #600 separately fixes the corresponding Rust read paths.
Validation on the updated head:
Lane-order regression coverage
This branch includes the canonical #538 fixture repair: a gate on the actual second-lane reservation key, with both waiter checks tied to the intended backend PID. The obsolete provisioning gate was skipped by v056 after lanes were prepared. The unchanged seven lane-order cases pass on PostgreSQL 17 and 18, and required fmt/Clippy/build pass. The inherited migration chain remains unchanged, including jobs view v057. New-head full CI is pending.
This head also includes the reviewed transaction-wide prune timeout handling from #642 through #635. Lock contention after child locks now rolls back the whole maintenance transaction and returns
Blocked; rollup stops at the first blocked slot. No other #642 ancestry was imported. Both Rust workspaces passed required format, all-target/all-feature Clippy, and builds; all seven lane-order cases and both post-child-lock rollback controls passed on PostgreSQL 17 and 18. The existing v057 migration and lane fixture are preserved. Fresh exact-head full CI is pending.