diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 83079c307..1237f8677 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -259,7 +259,7 @@ jobs: # doctests run as `workspace`. Shard membership lives in # scripts/ci-test-shard.sh; the lint job runs its `check` mode so renames # can't silently drop coverage. - timeout-minutes: 20 + timeout-minutes: 30 strategy: fail-fast: false matrix: @@ -477,7 +477,7 @@ jobs: - name: Create venv and install deps run: | uv venv .venv --python "${{ matrix.python }}" - uv pip install maturin pytest pytest-asyncio asyncpg django greenlet psycopg psycopg-binary sqlalchemy + uv pip install maturin pytest pytest-asyncio asyncpg django greenlet "psycopg[binary]" sqlalchemy - name: Build extension (maturin develop) run: .venv/bin/maturin develop - name: Wait for Postgres diff --git a/CHANGELOG.md b/CHANGELOG.md index 0555517d8..0bf1ab217 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,9 @@ Notable changes between releases. Detailed migration notes for storage transitio ### Forward-ported from 0.6.16 - Queue-storage rescue drains a large backlog of abandoned claims in bounded time. A killed runtime leaves every attempt it claimed open, and until they close, queue prune skips the queue slots they reference and claim prune skips their claim slots, so neither ring can wrap. A rescue pass takes 500-attempt batches, each in its own transaction, until a batch comes back short, `rescue_pass_budget` (default 1 s, at most half the rescue interval) is spent, or it has rescued `rescue_pass_max_rows` (default 5,000), and while a backlog remains the next pass runs a second later. `ClientBuilder::rescue_pass_budget` and `rescue_pass_max_rows` (Python `rescue_pass_budget_ms` and `rescue_pass_max_rows`) set both; zero for either keeps one batch per interval. Deadline and callback rescue act on their expired deadlines; stale-heartbeat rescue takes more than one batch per pass only once runtime snapshots show the claims' owners gone. Rescue's cursor scans seek to their cursor through the index, including the compact deadline-batch scan that 0.6 does not have, and rescue transactions run without JIT. `awa.maintenance.rescue.pass_size` (by `awa.rescue.kind` and `awa.rescue.pass_end`) and `awa.maintenance.rescue.oldest_attempt_age` show a drain in progress. Heartbeats no longer queue behind a completion: an attempt whose closer holds its lock is skipped, lease and progress writes skip rows a finalizer has locked and retry them on the next pass, and a finished attempt stops flushing progress once its handler returns. A point read of a failed job that starts while queue prune moves its row out of the slot being pruned is covered by a test; on this branch the read already locks the ring before it takes its snapshot. See [draining a backlog of abandoned claims](docs/configuration.md#rescue-backlog). +- Claim prune no longer treats a partial GIN index on `lease_claim_batches.job_ids` as the claim batch index (migration v056). It recognises only a valid, non-partial GIN index on `job_ids` alone. When another index or relation already has the index's name, `ensure_claim_batch_job_index` leaves it in place, returns false and raises a warning naming it, instead of reporting the slot indexed; the slot keeps scanning until an operator drops or renames it. Refreshing the function as a migration role that owns it but may only `SET ROLE` to the table owner (PostgreSQL 16+ membership `WITH INHERIT FALSE`) no longer leaves `EXECUTE` with `PUBLIC`: the refresh resets `EXECUTE` after it hands the function to the table owner, acting as the new owner when it cannot as itself. Migrations run with the owner as the effective role, as documented, were not affected. v056 replaces functions only and takes no table locks. See [v056](docs/upgrade-0.6-to-0.7.md#v056-claim-batch-index-recognition). +- Creating a new queue lane through `QueueStorage` (its batch enqueues, and workers promoting scheduled or retried jobs) no longer deadlocks with a concurrent `insert`, `insert_many` or `insert_many_copy` that is also the first to use the same `(queue, priority)` (#692). `QueueStorage` took the lane's provisioning lock before writing its `queue_lanes` row, while SQL inserts write the row first and take the lock in the head-table trigger that creates the lane's sequences, so each transaction waited on the other and one failed with `40P01`. `QueueStorage` now writes the row first too, and so does the `provision_lane` function it calls (migration v058, [v058](docs/upgrade-0.6-to-0.7.md#v058-lane-row-before-lane-lock)). Lanes that already existed were never affected. `QueueStorage` processes on 0.6.15 or earlier, or on an earlier 0.7 build, keep the old order until they are upgraded. +- `insert_many_copy` and `insert_many_skip_unique_conflicts` no longer deadlock with per-row producers (generic `insert_many`, single-job inserts, binaries before v055) that share their unique keys or lanes (migration v057). With the v055 function, when a per-row transaction held a lane and then inserted a key a waiting batch had claimed, PostgreSQL aborted one of the two with `deadlock detected` (`40P01`). The batch function now waits at most 500 ms (or half of its session's `deadlock_timeout`, if lower) while it holds claims or lanes: on a longer wait it releases them and tries again, so the per-row transaction proceeds and the batch then treats the keys it committed as already claimed. The same applies to a transaction that enqueues lanes in separate statements in the opposite order to a batch. This covers per-row sessions whose `deadlock_timeout` is above 500 ms, as PostgreSQL's default of 1 s is, and batches whose unique claims insert within that budget (10,000 to 20,000 keys on a fast server, fewer on a slow or busy one); larger batches behave as before. The protection lasts for 2 s of retries; then the batch makes one last attempt with ordinary lock waits. A per-row transaction that holds a lane for longer than that before it inserts one of the batch's keys can therefore still deadlock with it. A cycle through a lock the batch's own transaction took in an earlier statement also ends in `deadlock detected`, after about 3 to 4 s instead of 1 to 2 s, and usually aborts the batch's transaction. The caller's `lock_timeout` applies to each lock wait, as before. Results and the lane lock window are unchanged. v057 is one function replacement with no table locks, and producers that already call the function get the fix as soon as it commits. See [v057](docs/upgrade-0.6-to-0.7.md#v057-batch-inserts-give-way-to-per-row-producers). ### Added diff --git a/awa-model/migrations/v023_install_queue_storage_substrate.sql b/awa-model/migrations/v023_install_queue_storage_substrate.sql index a0d8448db..c78fc221f 100644 --- a/awa-model/migrations/v023_install_queue_storage_substrate.sql +++ b/awa-model/migrations/v023_install_queue_storage_substrate.sql @@ -325,13 +325,22 @@ BEGIN SET search_path = pg_catalog, pg_temp AS $func$ BEGIN - -- Validates the lane and takes its lock before any row is written. - PERFORM %1$I.provision_lane_sequences(p_queue, p_priority, p_enqueue_shard); + IF p_queue IS NULL + OR p_priority IS NULL + OR p_enqueue_shard IS NULL OR p_enqueue_shard NOT BETWEEN 0 AND 63 THEN + RAISE EXCEPTION 'invalid queue lane: queue %%, priority %%, enqueue shard %%', + quote_nullable(p_queue), p_priority, p_enqueue_shard + USING ERRCODE = '22023'; + END IF; + -- The lane row comes before the lane lock, the order every + -- producer takes them in. INSERT INTO %1$I.queue_lanes (queue, priority) VALUES (p_queue, p_priority) ON CONFLICT (queue, priority) DO NOTHING; + PERFORM %1$I.provision_lane_sequences(p_queue, p_priority, p_enqueue_shard); + -- The head triggers fire before ON CONFLICT and set the lane's -- sequence, so rows that already exist are skipped outright. INSERT INTO %1$I.queue_enqueue_heads (queue, priority, enqueue_shard) @@ -798,7 +807,9 @@ REVOKE EXECUTE ON FUNCTION awa.install_queue_storage_job_output(TEXT) FROM PUBLI -- index on a populated partition holds SHARE, which blocks claim inserts, for -- the whole build (minutes per million batches), so the index is only ever -- created on an empty partition: by the installer, or by claim prune right --- after it truncates a slot. +-- after it truncates a slot. A different relation that already has the +-- index's name is never dropped: the function warns, naming it, and the slot +-- keeps scanning until an operator removes or renames it. -- -- fastupdate is off: job ids arrive in ascending order, so direct inserts -- append to the right edge of the entry tree and cost no more than the @@ -822,6 +833,7 @@ DECLARE v_function REGPROCEDURE; v_function_owner OID; v_grantee TEXT; + v_previous_role TEXT; v_set_mode TEXT := CASE WHEN current_setting('server_version_num')::int >= 160000 THEN 'SET' ELSE 'MEMBER' END; BEGIN @@ -873,19 +885,44 @@ BEGIN USING ERRCODE = '22023'; END IF; + -- Only a valid, non-partial GIN index on job_ids alone, with the + -- default array_ops opclass, serves the by-job lookups. IF EXISTS ( SELECT 1 FROM pg_index AS idx JOIN pg_class AS index_class ON index_class.oid = idx.indexrelid JOIN pg_am AS method ON method.oid = index_class.relam + JOIN pg_attribute AS job_ids + ON job_ids.attrelid = idx.indrelid + AND job_ids.attname = 'job_ids' + AND NOT job_ids.attisdropped + JOIN pg_opclass AS opclass ON opclass.oid = idx.indclass[0] WHERE idx.indrelid = to_regclass(format('%%I.%%I', %1$L, v_child)) AND method.amname = 'gin' AND idx.indisvalid - AND pg_get_indexdef(idx.indexrelid) LIKE '%%USING gin (job_ids)%%' + AND idx.indpred IS NULL + AND idx.indexprs IS NULL + AND idx.indnatts = 1 + AND idx.indkey[0] = job_ids.attnum + AND opclass.opcname = 'array_ops' + AND opclass.opcnamespace = 'pg_catalog'::regnamespace ) THEN RETURN TRUE; END IF; + -- Something else already has the index's name: a partial or + -- otherwise different index, or another relation. Leave it alone + -- and keep scanning; the operator decides what to do with it. + IF to_regclass(format('%%I.%%I', %1$L, v_index)) IS NOT NULL THEN + RAISE WARNING '%%.%% is not a GIN index on job_ids, so claim slot %% of schema %% keeps scanning for job ids', + quote_ident(%1$L), quote_ident(v_index), p_slot, %1$L + USING HINT = format( + 'Drop or rename %%s.%%s; claim prune builds the job_ids index the next time it empties claim slot %%s.', + quote_ident(%1$L), quote_ident(v_index), p_slot + ); + RETURN FALSE; + END IF; + -- NOWAIT: a partition someone is writing to is not empty for long -- enough to index, so give up rather than queue behind claims. -- Leaving the block through the exception rolls back its @@ -899,11 +936,11 @@ BEGIN RAISE EXCEPTION USING ERRCODE = 'object_not_in_prerequisite_state'; END IF; EXECUTE format( - 'CREATE INDEX IF NOT EXISTS %%I ON %%I.%%I USING GIN (job_ids) WITH (fastupdate = off)', + 'CREATE INDEX %%I ON %%I.%%I USING GIN (job_ids) WITH (fastupdate = off)', v_index, %1$L, v_child ); EXCEPTION - WHEN lock_not_available OR object_not_in_prerequisite_state THEN + WHEN lock_not_available OR object_not_in_prerequisite_state OR duplicate_table THEN RETURN FALSE; END; RETURN TRUE; @@ -928,6 +965,7 @@ BEGIN v_function, pg_get_userbyid(v_table_owner) ); + v_function_owner := v_table_owner; ELSE RAISE WARNING '% is owned by %, not the owner % of %.lease_claim_batches, so it cannot create the job_ids index', v_function, pg_get_userbyid(v_function_owner), pg_get_userbyid(v_table_owner), p_schema @@ -939,6 +977,14 @@ BEGIN END IF; END IF; + -- Reset EXECUTE as the function's owner, after any transfer, so a former + -- owner that holds TRUNCATE keeps it. A role that may only SET ROLE to the + -- new owner cannot grant or revoke as itself, so it acts as the owner for + -- the reset and then restores its previous role. + IF NOT pg_has_role(current_user, v_function_owner, 'USAGE') THEN + v_previous_role := current_setting('role'); + PERFORM set_config('role', pg_get_userbyid(v_function_owner), true); + END IF; FOR v_grantee IN SELECT DISTINCT CASE WHEN acl.grantee = 0 THEN 'PUBLIC' ELSE quote_ident(pg_get_userbyid(acl.grantee)) END @@ -958,6 +1004,9 @@ BEGIN LOOP EXECUTE format('GRANT EXECUTE ON FUNCTION %s TO %s', v_function, v_grantee); END LOOP; + IF v_previous_role IS NOT NULL THEN + PERFORM set_config('role', v_previous_role, true); + END IF; END; $refresh$; diff --git a/awa-model/migrations/v056_claim_batch_job_index_shape.sql b/awa-model/migrations/v056_claim_batch_job_index_shape.sql new file mode 100644 index 000000000..c04fd2d7c --- /dev/null +++ b/awa-model/migrations/v056_claim_batch_job_index_shape.sql @@ -0,0 +1,57 @@ +-- v056: recognise only a full job_ids GIN index as the claim batch index. +-- Tracking: https://github.com/hardbyte/awa/pull/701 +-- .ensure_claim_batch_job_index(slot) from v054 reported a claim +-- partition indexed when it had any valid GIN index whose definition named +-- job_ids, a partial one included, and its CREATE INDEX IF NOT EXISTS took a +-- different relation that already had the index's name for the index. The +-- function now accepts only a valid, non-partial GIN index on job_ids alone +-- with the default opclass. When something else has the index's name it +-- raises a warning naming it, returns false and leaves it in place. +-- +-- awa.refresh_claim_batch_job_index_helper now resets EXECUTE on the function +-- after it hands the function to the owner of lease_claim_batches, acting as +-- that owner when the caller may only SET ROLE to it, so EXECUTE ends with +-- exactly the roles that hold TRUNCATE on the table. +-- +-- This file only reinstalls the function in each queue-storage schema; it +-- builds and drops no index. `awa migrate` and +-- `awa migrate --sql --from 55 --to 56` run V23_UP first, which defines the +-- helper. +-- +-- N-1 binaries call the function with the same argument and result. An index +-- that v054's function built is a full job_ids index and is still recognised. +-- +-- Live load: function replacement only. It takes no lock on any awa or +-- queue-storage table. Run it as the owner of the queue-storage tables, or as +-- a migrator that can SET ROLE to it, so the function is owned by the table +-- owner and can create the index. + +DO $$ +DECLARE + v_schema TEXT; +BEGIN + FOR v_schema IN + SELECT discovered.schema_name FROM awa.queue_storage_schemas() AS discovered + LOOP + IF NOT EXISTS ( + SELECT 1 FROM pg_class AS batches + WHERE batches.oid = to_regclass(format('%I.lease_claim_batches', v_schema)) + AND batches.relkind = 'p' + ) THEN + CONTINUE; + END IF; + + -- Serialize with a concurrent prepare_schema() of the same schema. This + -- is an advisory lock, not a table lock: claims and completions do not + -- take it. + PERFORM pg_advisory_xact_lock( + hashtextextended(format('awa.queue_storage.install:%I', v_schema), 0) + ); + PERFORM awa.refresh_claim_batch_job_index_helper(v_schema); + END LOOP; +END; +$$; + +INSERT INTO awa.schema_version (version, description) +VALUES (56, 'Recognise only a full job_ids index as the claim batch index') +ON CONFLICT (version) DO NOTHING; diff --git a/awa-model/migrations/v057_insert_jobs_compat_batch_bounded_wait.sql b/awa-model/migrations/v057_insert_jobs_compat_batch_bounded_wait.sql new file mode 100644 index 000000000..c1157ca00 --- /dev/null +++ b/awa-model/migrations/v057_insert_jobs_compat_batch_bounded_wait.sql @@ -0,0 +1,719 @@ +-- v057: the batch producer insert gives way instead of deadlocking. +-- Tracking: https://github.com/hardbyte/awa/pull/700 +-- +-- awa.insert_jobs_compat_batch (v055) takes a batch's unique claims before +-- its lane locks. Per-row producers take each job's claim and its lane +-- together, so one can hold a lane the batch needs while it waits for a key +-- the batch has claimed, and v055's body then waited for the lane without +-- bound: PostgreSQL aborted one of the two with deadlock detected. +-- +-- The batch takes its claims and lanes in a subtransaction, and limits each +-- lock wait to what remains of a budget measured from the attempt's start: +-- 500 ms, or half of the batch session's deadlock_timeout when that is lower. +-- When a wait would run longer, the batch rolls back, which releases every +-- claim and lane it took, and retries, so a per-row producer whose +-- deadlock_timeout exceeds the budget (PostgreSQL's default is 1 s) usually +-- proceeds before its deadlock check runs. After four budgets of retries the +-- batch makes one last attempt with ordinary waits, as v055's body did, so +-- contention or a cycle through locks the caller took earlier ends in +-- success, a lock timeout or deadlock detected, never in endless retries. +-- +-- The function's signature, privileges and results are unchanged: the column +-- arrays are parallel and must have equal length, every element is required +-- except run_at, unique_key, unique_states and ordering_hash, and +-- p_skip_unique_conflicts skips rows whose unique key is already claimed +-- instead of failing the whole batch with a unique violation. +-- +-- A 0.6.16 database already carries this body from the 0.6 +-- insert_jobs_compat_batch schema patch; this replaces it in place. +-- +-- N-1 binaries that call the function pass the same arguments and get the +-- bounded waits as soon as this migration commits. Binaries that never call +-- it keep calling awa.insert_job_compat, which this migration does not touch. +-- +-- `awa migrate` and `awa migrate --sql --from 56 --to 57` run V23_UP first, +-- which bootstraps trusted queue-storage discovery and refuses an untrusted +-- registered substrate, as every migration since v049 does. +-- +-- Live load: one CREATE OR REPLACE FUNCTION, no table DDL, no table locks. + +CREATE OR REPLACE FUNCTION awa.insert_jobs_compat_batch( + p_kind TEXT[], + p_queue TEXT[], + p_args JSONB[], + p_state TEXT[], + p_priority SMALLINT[], + p_max_attempts SMALLINT[], + p_run_at TIMESTAMPTZ[], + p_metadata JSONB[], + p_tags JSONB[], + p_unique_key BYTEA[], + p_unique_states TEXT[], + p_ordering_hash TEXT[], + p_skip_unique_conflicts BOOLEAN DEFAULT FALSE +) +RETURNS SETOF awa.jobs +AS $$ +DECLARE + v_count INT := COALESCE(cardinality(p_kind), 0); + v_schema TEXT; + v_old_search_path TEXT; + v_now TIMESTAMPTZ := clock_timestamp(); + v_job_ids BIGINT[]; + v_skipped BIGINT[] := ARRAY[]::BIGINT[]; + v_lane RECORD; + v_lane_queues TEXT[] := ARRAY[]::TEXT[]; + v_lane_priorities SMALLINT[] := ARRAY[]::SMALLINT[]; + v_lane_shards SMALLINT[] := ARRAY[]::SMALLINT[]; + v_lane_starts BIGINT[] := ARRAY[]::BIGINT[]; + v_start BIGINT; + v_ready_slot INT; + v_ready_generation BIGINT; + v_row RECORD; + v_shards SMALLINT[]; + v_result awa.jobs[]; + v_claim RECORD; + v_attempt INT := 0; + v_attempt_started TIMESTAMPTZ; + v_wait_budget INTERVAL; + v_caller_lock_timeout TEXT; + v_caller_wait_ms NUMERIC; + v_wait_ms NUMERIC; + v_wait_is_callers BOOLEAN := FALSE; + v_phase TEXT; + v_claims_one_by_one BOOLEAN := FALSE; + v_bounded_until TIMESTAMPTZ; + v_final BOOLEAN := FALSE; +BEGIN + IF v_count = 0 THEN + RETURN; + END IF; + + IF cardinality(p_queue) IS DISTINCT FROM v_count + OR cardinality(p_args) IS DISTINCT FROM v_count + OR cardinality(p_state) IS DISTINCT FROM v_count + OR cardinality(p_priority) IS DISTINCT FROM v_count + OR cardinality(p_max_attempts) IS DISTINCT FROM v_count + OR cardinality(p_run_at) IS DISTINCT FROM v_count + OR cardinality(p_metadata) IS DISTINCT FROM v_count + OR cardinality(p_tags) IS DISTINCT FROM v_count + OR cardinality(p_unique_key) IS DISTINCT FROM v_count + OR cardinality(p_unique_states) IS DISTINCT FROM v_count + OR cardinality(p_ordering_hash) IS DISTINCT FROM v_count + THEN + RAISE EXCEPTION 'insert_jobs_compat_batch: column arrays must have equal length' + USING ERRCODE = '22023'; + END IF; + + IF EXISTS ( + SELECT 1 + FROM unnest(p_kind, p_queue, p_args, p_state, p_priority, p_max_attempts, p_metadata, p_tags) + AS input(kind, queue, args, state, priority, max_attempts, metadata, tags) + WHERE input.kind IS NULL OR input.queue IS NULL OR input.args IS NULL + OR input.state IS NULL OR input.priority IS NULL OR input.max_attempts IS NULL + OR input.metadata IS NULL OR input.tags IS NULL + ) THEN + RAISE EXCEPTION 'insert_jobs_compat_batch: kind, queue, args, state, priority, max_attempts, metadata and tags must not be NULL' + USING ERRCODE = '22004'; + END IF; + + IF EXISTS (SELECT 1 FROM unnest(p_kind) AS kind WHERE length(kind) > 200) THEN + RAISE EXCEPTION 'job kind length must be <= 200 characters' + USING ERRCODE = '23514'; + END IF; + + IF EXISTS (SELECT 1 FROM unnest(p_queue) AS queue WHERE length(queue) > 200) THEN + RAISE EXCEPTION 'queue name length must be <= 200 characters' + USING ERRCODE = '23514'; + END IF; + + IF EXISTS ( + SELECT 1 FROM unnest(p_priority) AS priority + WHERE priority < 1 OR priority > 4 + ) THEN + RAISE EXCEPTION 'priority must be between 1 and 4' + USING ERRCODE = '23514'; + END IF; + + IF EXISTS ( + SELECT 1 FROM unnest(p_max_attempts) AS max_attempts + WHERE max_attempts < 1 OR max_attempts > 1000 + ) THEN + RAISE EXCEPTION 'max_attempts must be between 1 and 1000' + USING ERRCODE = '23514'; + END IF; + + IF EXISTS ( + SELECT 1 FROM unnest(p_tags) AS tags + WHERE jsonb_array_length(tags) > 20 + ) THEN + RAISE EXCEPTION 'job tags must contain at most 20 values' + USING ERRCODE = '23514'; + END IF; + + v_schema := awa.active_queue_storage_schema(); + + -- Canonical storage has no lane lock to shorten: insert row by row. + IF v_schema IS NULL THEN + IF NOT p_skip_unique_conflicts THEN + RETURN QUERY + SELECT inserted.* + FROM unnest( + p_kind, p_queue, p_args, p_state, p_priority, p_max_attempts, + p_run_at, p_metadata, p_tags, p_unique_key, p_unique_states + ) WITH ORDINALITY AS input( + kind, queue, args, state, priority, max_attempts, run_at, + metadata, tags, unique_key, unique_states, ord + ) + CROSS JOIN LATERAL awa.insert_job_compat( + input.kind, + input.queue, + input.args, + input.state::awa.job_state, + input.priority, + input.max_attempts, + input.run_at, + input.metadata, + ARRAY( + SELECT tag.value + FROM jsonb_array_elements_text(input.tags) WITH ORDINALITY AS tag(value, tag_ord) + ORDER BY tag.tag_ord + ), + input.unique_key, + input.unique_states::bit(8), + NULL + ) AS inserted + ORDER BY input.ord; + RETURN; + END IF; + + -- Skipping needs a subtransaction per row. + FOR v_row IN + SELECT + input.ord, + input.kind, + input.queue, + input.args, + input.state, + input.priority, + input.max_attempts, + input.run_at, + input.metadata, + ARRAY( + SELECT tag.value + FROM jsonb_array_elements_text(input.tags) WITH ORDINALITY AS tag(value, tag_ord) + ORDER BY tag.tag_ord + ) AS tags, + input.unique_key, + input.unique_states + FROM unnest( + p_kind, p_queue, p_args, p_state, p_priority, p_max_attempts, + p_run_at, p_metadata, p_tags, p_unique_key, p_unique_states + ) WITH ORDINALITY AS input( + kind, queue, args, state, priority, max_attempts, run_at, + metadata, tags, unique_key, unique_states, ord + ) + ORDER BY input.ord + LOOP + BEGIN + RETURN NEXT awa.insert_job_compat( + v_row.kind, + v_row.queue, + v_row.args, + v_row.state::awa.job_state, + v_row.priority, + v_row.max_attempts, + v_row.run_at, + v_row.metadata, + v_row.tags, + v_row.unique_key, + v_row.unique_states::bit(8), + NULL + ); + EXCEPTION WHEN unique_violation THEN + NULL; + END; + END LOOP; + RETURN; + END IF; + + IF EXISTS ( + SELECT 1 FROM unnest(p_state) AS state + WHERE state NOT IN ('available', 'scheduled', 'retryable') + ) THEN + RAISE EXCEPTION 'queue storage does not support initial state %', + (SELECT state FROM unnest(p_state) AS state + WHERE state NOT IN ('available', 'scheduled', 'retryable') LIMIT 1) + USING ERRCODE = '22023'; + END IF; + + v_old_search_path := current_setting('search_path'); + PERFORM set_config('search_path', format('%I,awa,public', v_schema), true); + + EXECUTE format( + 'SELECT array_agg(nextval(%L::regclass)::bigint) FROM generate_series(1, $1)', + format('%I.job_id_seq', v_schema) + ) + INTO v_job_ids + USING v_count; + + -- Rows that enqueue as `available` go to a lane. Their shard is derived as + -- insert_job_compat derives it: ordering-key rows from the key hash, the + -- rest from the backend pid. + SELECT array_agg( + CASE WHEN input.state = 'available' THEN + MOD( + CASE WHEN input.ordering_hash IS NULL + THEN pg_backend_pid()::numeric + ELSE input.ordering_hash::numeric + END, + COALESCE(meta.enqueue_shards, 1)::numeric + )::smallint + END + ORDER BY input.ord + ) + INTO v_shards + FROM unnest(p_queue, p_state, p_ordering_hash) + WITH ORDINALITY AS input(queue, state, ordering_hash, ord) + LEFT JOIN awa.queue_meta AS meta ON meta.queue = input.queue; + + -- Every claim and lane the batch takes is taken inside one attempt, a + -- subtransaction. Until the bounded phase ends, each lock wait in an + -- attempt ends within the wait budget of the attempt's start; a wait that + -- would run longer rolls the attempt back, which releases everything it + -- took, and the batch tries again. A per-row producer that holds a lane + -- and waits for one of the batch's keys is then released before its own + -- deadlock check runs, provided its deadlock_timeout exceeds the budget. + -- The budget is capped because that check runs on the other session's + -- deadlock_timeout, which this session cannot see. + -- + -- Rolling back cannot release what the caller's transaction took before + -- this call, so retrying alone could spin forever on a cycle through such + -- a lock. After the bounded phase the batch makes one final attempt with + -- the caller's lock_timeout and ordinary waits, as the function did before + -- it had a budget: it succeeds, or PostgreSQL's deadlock detector or the + -- caller's lock_timeout ends it. + -- + -- The caller's lock_timeout applies to each wait, as it does to any + -- statement: a bounded wait is the shorter of the remaining budget and the + -- caller's lock_timeout, and only a wait that the caller's lock_timeout + -- ended raises lock_not_available to the caller. The budget is per wait + -- too: one claim statement can wait on several in-flight keys, and a lane + -- reservation can wait for the lane's provisioning lock and then its lane + -- lock, each up to the remaining budget. + v_caller_lock_timeout := current_setting('lock_timeout'); + v_caller_wait_ms := ceil(1000 * extract(epoch FROM v_caller_lock_timeout::interval)); + v_wait_budget := LEAST( + current_setting('deadlock_timeout')::interval / 2, + interval '500 milliseconds' + ); + v_bounded_until := clock_timestamp() + 4 * v_wait_budget; + + -- The result rows do not depend on claims or lanes, so they are built once, + -- before any lock, and the skipped rows are filtered out on return. + SELECT array_agg( + ( + v_job_ids[input.ord], + input.kind, + input.queue, + input.args, + input.state::awa.job_state, + input.priority, + 0::smallint, + input.max_attempts, + COALESCE(input.run_at, v_now), + NULL::timestamptz, + NULL::timestamptz, + NULL::timestamptz, + NULL::timestamptz, + v_now, + ARRAY[]::jsonb[], + input.metadata, + ARRAY( + SELECT tag.value + FROM jsonb_array_elements_text(input.tags) WITH ORDINALITY AS tag(value, tag_ord) + ORDER BY tag.tag_ord + ), + input.unique_key, + input.unique_states::bit(8), + NULL::uuid, + NULL::timestamptz, + NULL::text, + NULL::text, + NULL::text, + NULL::text, + 0::bigint, + NULL::jsonb + )::awa.jobs + ORDER BY input.ord + ) + INTO v_result + FROM unnest( + p_kind, p_queue, p_args, p_state, p_priority, p_max_attempts, p_run_at, + p_metadata, p_tags, p_unique_key, p_unique_states + ) WITH ORDINALITY AS input( + kind, queue, args, state, priority, max_attempts, run_at, + metadata, tags, unique_key, unique_states, ord + ); + + LOOP + v_attempt := v_attempt + 1; + BEGIN + v_skipped := ARRAY[]::BIGINT[]; + v_lane_queues := ARRAY[]::TEXT[]; + v_lane_priorities := ARRAY[]::SMALLINT[]; + v_lane_shards := ARRAY[]::SMALLINT[]; + v_lane_starts := ARRAY[]::BIGINT[]; + v_attempt_started := clock_timestamp(); + + -- Claims go in unique-key order, so two producers with overlapping + -- keys cannot wait on each other in opposite orders. The first + -- attempt inserts them in one statement, each lock wait capped at + -- 10 ms (or the budget, if shorter), and gives way to any claim of + -- the same key still in flight; later bounded attempts go key by + -- key, each waiting at most for the rest of the budget; the final + -- attempt inserts them in one statement with ordinary waits. + v_phase := 'claims'; + IF v_final OR NOT v_claims_one_by_one THEN + v_wait_ms := LEAST(10, ceil(1000 * extract(epoch FROM + v_attempt_started + v_wait_budget - clock_timestamp()))); + v_wait_is_callers := v_final OR v_caller_wait_ms BETWEEN 1 AND v_wait_ms; + PERFORM set_config('lock_timeout', CASE WHEN v_wait_is_callers THEN v_caller_lock_timeout + ELSE format('%sms', GREATEST(1, v_wait_ms)::bigint) END, true); + IF p_skip_unique_conflicts THEN + WITH claimed AS ( + INSERT INTO awa.job_unique_claims (unique_key, job_id) + SELECT input.unique_key, v_job_ids[input.ord] + FROM unnest(p_unique_key, p_unique_states, p_state) + WITH ORDINALITY AS input(unique_key, unique_states, state, ord) + WHERE input.unique_key IS NOT NULL + AND input.unique_states IS NOT NULL + AND awa.job_state_in_bitmask(input.unique_states::bit(8), input.state::awa.job_state) + ORDER BY input.unique_key, input.ord + ON CONFLICT (unique_key) DO NOTHING + RETURNING job_id + ) + SELECT COALESCE(array_agg(wanted.job_id), ARRAY[]::BIGINT[]) + INTO v_skipped + FROM ( + SELECT v_job_ids[input.ord] AS job_id + FROM unnest(p_unique_key, p_unique_states, p_state) + WITH ORDINALITY AS input(unique_key, unique_states, state, ord) + WHERE input.unique_key IS NOT NULL + AND input.unique_states IS NOT NULL + AND awa.job_state_in_bitmask(input.unique_states::bit(8), input.state::awa.job_state) + ) AS wanted + WHERE NOT EXISTS (SELECT 1 FROM claimed WHERE claimed.job_id = wanted.job_id); + ELSE + INSERT INTO awa.job_unique_claims (unique_key, job_id) + SELECT input.unique_key, v_job_ids[input.ord] + FROM unnest(p_unique_key, p_unique_states, p_state) + WITH ORDINALITY AS input(unique_key, unique_states, state, ord) + WHERE input.unique_key IS NOT NULL + AND input.unique_states IS NOT NULL + AND awa.job_state_in_bitmask(input.unique_states::bit(8), input.state::awa.job_state) + ORDER BY input.unique_key, input.ord; + END IF; + ELSE + FOR v_claim IN + SELECT input.unique_key, v_job_ids[input.ord] AS job_id + FROM unnest(p_unique_key, p_unique_states, p_state) + WITH ORDINALITY AS input(unique_key, unique_states, state, ord) + WHERE input.unique_key IS NOT NULL + AND input.unique_states IS NOT NULL + AND awa.job_state_in_bitmask(input.unique_states::bit(8), input.state::awa.job_state) + ORDER BY input.unique_key, input.ord + LOOP + v_wait_ms := ceil(1000 * extract(epoch FROM + v_attempt_started + v_wait_budget - clock_timestamp())); + v_wait_is_callers := v_caller_wait_ms BETWEEN 1 AND v_wait_ms; + PERFORM set_config('lock_timeout', CASE WHEN v_wait_is_callers THEN v_caller_lock_timeout + ELSE format('%sms', GREATEST(1, v_wait_ms)::bigint) END, true); + IF p_skip_unique_conflicts THEN + INSERT INTO awa.job_unique_claims (unique_key, job_id) + VALUES (v_claim.unique_key, v_claim.job_id) + ON CONFLICT (unique_key) DO NOTHING; + IF NOT FOUND THEN + v_skipped := v_skipped || v_claim.job_id; + END IF; + ELSE + INSERT INTO awa.job_unique_claims (unique_key, job_id) + VALUES (v_claim.unique_key, v_claim.job_id); + END IF; + END LOOP; + END IF; + + -- The writes outside the lane loop wait at most for relation + -- extension, or for a prune that gives way under its own + -- lock_timeout, so they keep the caller's lock_timeout. + v_wait_is_callers := TRUE; + PERFORM set_config('lock_timeout', v_caller_lock_timeout, true); + + -- Re-enqueues that are not yet due carry no lane. + INSERT INTO deferred_jobs ( + job_id, kind, queue, args, state, priority, attempt, run_lease, + max_attempts, run_at, attempted_at, finalized_at, created_at, + unique_key, unique_states, payload + ) + SELECT + v_job_ids[input.ord], + input.kind, + input.queue, + input.args, + input.state::awa.job_state, + input.priority, + 0, + 0, + input.max_attempts, + COALESCE(input.run_at, v_now), + NULL, + NULL, + v_now, + input.unique_key, + input.unique_states, + jsonb_build_object( + 'metadata', input.metadata, + 'tags', input.tags, + 'errors', '[]'::jsonb, + 'progress', NULL + ) + FROM unnest( + p_kind, p_queue, p_args, p_state, p_priority, p_max_attempts, p_run_at, + p_metadata, p_tags, p_unique_key, p_unique_states + ) WITH ORDINALITY AS input( + kind, queue, args, state, priority, max_attempts, run_at, + metadata, tags, unique_key, unique_states, ord + ) + WHERE input.state <> 'available' + AND v_job_ids[input.ord] <> ALL (v_skipped); + + -- Lane lock window: in (queue, priority, shard) order, register each + -- lane the batch is the first to use and reserve its sequence + -- range once, then write the ready rows. The locks are held to the + -- end of the transaction, which keeps the rows of a lane visible + -- in lane_seq order. + -- + -- A new lane is registered only when the loop reaches it, as every + -- other producer does: the head tables' row trigger provisions the + -- lane under its lock, and the uncommitted head row blocks other + -- producers of that lane, so registering a later lane before + -- reserving an earlier one would hold it out of order. Only missing + -- rows are proposed, because the trigger fires before the conflict + -- check. + v_phase := 'lanes'; + FOR v_lane IN + SELECT lanes.queue, lanes.priority, lanes.shard, lanes.row_count + FROM ( + SELECT input.queue, input.priority, v_shards[input.ord] AS shard, + count(*) AS row_count + FROM unnest(p_queue, p_priority, p_state) + WITH ORDINALITY AS input(queue, priority, state, ord) + WHERE input.state = 'available' + AND v_job_ids[input.ord] <> ALL (v_skipped) + GROUP BY input.queue, input.priority, v_shards[input.ord] + ) AS lanes + ORDER BY lanes.queue COLLATE "C", lanes.priority, lanes.shard + LOOP + v_wait_ms := ceil(1000 * extract(epoch FROM + v_attempt_started + v_wait_budget - clock_timestamp())); + v_wait_is_callers := v_final OR v_caller_wait_ms BETWEEN 1 AND v_wait_ms; + PERFORM set_config('lock_timeout', CASE WHEN v_wait_is_callers THEN v_caller_lock_timeout + ELSE format('%sms', GREATEST(1, v_wait_ms)::bigint) END, true); + + INSERT INTO queue_lanes (queue, priority) + SELECT v_lane.queue, v_lane.priority + WHERE NOT EXISTS ( + SELECT 1 FROM queue_lanes AS existing + WHERE existing.queue = v_lane.queue AND existing.priority = v_lane.priority + ) + ON CONFLICT ON CONSTRAINT queue_lanes_pkey DO NOTHING; + + v_wait_ms := ceil(1000 * extract(epoch FROM + v_attempt_started + v_wait_budget - clock_timestamp())); + v_wait_is_callers := v_final OR v_caller_wait_ms BETWEEN 1 AND v_wait_ms; + PERFORM set_config('lock_timeout', CASE WHEN v_wait_is_callers THEN v_caller_lock_timeout + ELSE format('%sms', GREATEST(1, v_wait_ms)::bigint) END, true); + + INSERT INTO queue_enqueue_heads (queue, priority, enqueue_shard) + SELECT v_lane.queue, v_lane.priority, v_lane.shard + WHERE NOT EXISTS ( + SELECT 1 FROM queue_enqueue_heads AS existing + WHERE existing.queue = v_lane.queue + AND existing.priority = v_lane.priority + AND existing.enqueue_shard = v_lane.shard + ) + ON CONFLICT (queue, priority, enqueue_shard) DO NOTHING; + + v_wait_ms := ceil(1000 * extract(epoch FROM + v_attempt_started + v_wait_budget - clock_timestamp())); + v_wait_is_callers := v_final OR v_caller_wait_ms BETWEEN 1 AND v_wait_ms; + PERFORM set_config('lock_timeout', CASE WHEN v_wait_is_callers THEN v_caller_lock_timeout + ELSE format('%sms', GREATEST(1, v_wait_ms)::bigint) END, true); + + INSERT INTO queue_claim_heads (queue, priority, enqueue_shard) + SELECT v_lane.queue, v_lane.priority, v_lane.shard + WHERE NOT EXISTS ( + SELECT 1 FROM queue_claim_heads AS existing + WHERE existing.queue = v_lane.queue + AND existing.priority = v_lane.priority + AND existing.enqueue_shard = v_lane.shard + ) + ON CONFLICT (queue, priority, enqueue_shard) DO NOTHING; + + v_wait_ms := ceil(1000 * extract(epoch FROM + v_attempt_started + v_wait_budget - clock_timestamp())); + v_wait_is_callers := v_final OR v_caller_wait_ms BETWEEN 1 AND v_wait_ms; + PERFORM set_config('lock_timeout', CASE WHEN v_wait_is_callers THEN v_caller_lock_timeout + ELSE format('%sms', GREATEST(1, v_wait_ms)::bigint) END, true); + + EXECUTE format('SELECT %I.reserve_enqueue_seq($1, $2, $3, $4)', v_schema) + INTO v_start + USING v_lane.queue, v_lane.priority, v_lane.shard, v_lane.row_count::BIGINT; + + v_lane_queues := v_lane_queues || v_lane.queue; + v_lane_priorities := v_lane_priorities || v_lane.priority; + v_lane_shards := v_lane_shards || v_lane.shard; + v_lane_starts := v_lane_starts || v_start; + END LOOP; + EXCEPTION WHEN lock_not_available THEN + IF v_wait_is_callers THEN + RAISE; + END IF; + IF v_phase = 'claims' THEN + v_claims_one_by_one := TRUE; + END IF; + v_final := clock_timestamp() >= v_bounded_until; + PERFORM pg_sleep( + LEAST(0.1, 0.001 * power(2, LEAST(v_attempt, 7))) * (0.5 + random() / 2) + ); + CONTINUE; + END; + EXIT; + END LOOP; + + PERFORM set_config('lock_timeout', v_caller_lock_timeout, true); + + -- The lane locks are held, so the ready rows can be written outside the + -- attempt. + IF cardinality(v_lane_queues) > 0 THEN + SELECT rc.slot, rc.generation + INTO v_ready_slot, v_ready_generation + FROM ring_cursor('queue') AS rc; + + INSERT INTO ready_entries ( + ready_slot, ready_generation, job_id, kind, queue, args, priority, + attempt, run_lease, max_attempts, lane_seq, enqueue_shard, run_at, + attempted_at, created_at, unique_key, unique_states, payload + ) + SELECT + v_ready_slot, + v_ready_generation, + v_job_ids[placed.ord], + placed.kind, + placed.queue, + placed.args, + placed.priority, + 0, + 0, + placed.max_attempts, + lane.start_seq + placed.lane_rank - 1, + lane.shard, + COALESCE(placed.run_at, v_now), + NULL, + v_now, + placed.unique_key, + placed.unique_states, + jsonb_build_object( + 'metadata', placed.metadata, + 'tags', placed.tags, + 'errors', '[]'::jsonb, + 'progress', NULL + ) + FROM ( + SELECT + input.*, + v_shards[input.ord] AS shard, + row_number() OVER ( + PARTITION BY input.queue, input.priority, v_shards[input.ord] + ORDER BY input.ord + ) AS lane_rank + FROM unnest( + p_kind, p_queue, p_args, p_state, p_priority, p_max_attempts, p_run_at, + p_metadata, p_tags, p_unique_key, p_unique_states + ) WITH ORDINALITY AS input( + kind, queue, args, state, priority, max_attempts, run_at, + metadata, tags, unique_key, unique_states, ord + ) + WHERE input.state = 'available' + AND v_job_ids[input.ord] <> ALL (v_skipped) + ) AS placed + JOIN unnest(v_lane_queues, v_lane_priorities, v_lane_shards, v_lane_starts) + AS lane(queue, priority, shard, start_seq) + ON lane.queue = placed.queue + AND lane.priority = placed.priority + AND lane.shard = placed.shard; + + -- One segment per run of consecutive lane_seq values sharing a run_at. + INSERT INTO ready_segments ( + ready_slot, ready_generation, queue, priority, enqueue_shard, + first_lane_seq, next_lane_seq, first_run_at + ) + SELECT + v_ready_slot, + v_ready_generation, + runs.queue, + runs.priority, + runs.shard, + min(runs.lane_seq), + max(runs.lane_seq) + 1, + runs.run_at + FROM ( + SELECT + placed.*, + placed.lane_seq - row_number() OVER ( + PARTITION BY placed.queue, placed.priority, placed.shard, placed.run_at + ORDER BY placed.lane_seq + ) AS run_id + FROM ( + SELECT + input.queue, + input.priority, + v_shards[input.ord] AS shard, + COALESCE(input.run_at, v_now) AS run_at, + lane.start_seq + row_number() OVER ( + PARTITION BY input.queue, input.priority, v_shards[input.ord] + ORDER BY input.ord + ) - 1 AS lane_seq + FROM unnest(p_queue, p_priority, p_state, p_run_at) + WITH ORDINALITY AS input(queue, priority, state, run_at, ord) + JOIN unnest(v_lane_queues, v_lane_priorities, v_lane_shards, v_lane_starts) + AS lane(queue, priority, shard, start_seq) + ON lane.queue = input.queue + AND lane.priority = input.priority + AND lane.shard = v_shards[input.ord] + WHERE input.state = 'available' + AND v_job_ids[input.ord] <> ALL (v_skipped) + ) AS placed + ) AS runs + GROUP BY runs.queue, runs.priority, runs.shard, runs.run_at, runs.run_id + ON CONFLICT (ready_slot, ready_generation, queue, priority, enqueue_shard, first_lane_seq) + DO NOTHING; + END IF; + + RETURN QUERY + SELECT * FROM unnest(v_result) AS result + WHERE result.id <> ALL (v_skipped); + + IF cardinality(v_lane_queues) > 0 THEN + PERFORM pg_notify('awa:' || notified.queue, '') + FROM (SELECT DISTINCT unnest(v_lane_queues) AS queue) AS notified; + END IF; + + PERFORM set_config('search_path', v_old_search_path, true); +END; +$$ LANGUAGE plpgsql VOLATILE +SET search_path = pg_catalog, awa, public; + +INSERT INTO awa.schema_version (version, description) +VALUES (57, 'Batch producer insert that gives way instead of deadlocking') +ON CONFLICT (version) DO NOTHING; diff --git a/awa-model/migrations/v058_provision_lane_row_first.sql b/awa-model/migrations/v058_provision_lane_row_first.sql new file mode 100644 index 000000000..ba1c81184 --- /dev/null +++ b/awa-model/migrations/v058_provision_lane_row_first.sql @@ -0,0 +1,38 @@ +-- v058: provision a lane's queue_lanes row before its lane lock. +-- Tracking: https://github.com/hardbyte/awa/pull/728 +-- .provision_lane took the lane's advisory lock and then inserted its +-- queue_lanes row. Producers and QueueStorage insert the row first and take +-- the lock after it, so a provision_lane call and a producer that both created +-- the same new lane could each hold what the other waited for, and PostgreSQL +-- cancelled one with a deadlock error. provision_lane now inserts the row +-- first. +-- +-- This file reinstalls provision_lane and provision_lane_sequences in each +-- queue-storage schema through awa.refresh_lane_sequence_helpers, which +-- V23_UP defines. N-1 binaries call provision_lane with the same arguments. +-- +-- Live load: function replacement only. It takes no lock on any awa or +-- queue-storage table. + +DO $$ +DECLARE + v_schema TEXT; +BEGIN + FOR v_schema IN + SELECT discovered.schema_name FROM awa.queue_storage_schemas() AS discovered + LOOP + IF to_regclass(format('%I.queue_enqueue_heads', v_schema)) IS NULL + OR to_regclass(format('%I.queue_claim_heads', v_schema)) IS NULL + OR to_regclass(format('%I.queue_ring_state', v_schema)) IS NULL + OR to_regclass(format('%I.lease_ring_state', v_schema)) IS NULL + OR to_regclass(format('%I.claim_ring_state', v_schema)) IS NULL THEN + CONTINUE; + END IF; + PERFORM awa.refresh_lane_sequence_helpers(v_schema); + END LOOP; +END; +$$; + +INSERT INTO awa.schema_version (version, description) +VALUES (58, 'Provision a lane''s row before its lock') +ON CONFLICT (version) DO NOTHING; diff --git a/awa-model/src/insert.rs b/awa-model/src/insert.rs index d628e4e66..de894c8a2 100644 --- a/awa-model/src/insert.rs +++ b/awa-model/src/insert.rs @@ -20,8 +20,14 @@ use sqlx::{PgExecutor, PgPool}; // `insert_many_copy` and `insert_many_skip_unique_conflicts` use // `awa.insert_jobs_compat_batch` (migration v055), which does the lane-independent work first (job ids, unique claims, // payloads) and then registers and reserves its lanes one at a time in -// canonical order, so a lane is held only across its registration, -// reservation, the ready-row writes and the commit. +// canonical order. Each lane's lock is taken when the batch reaches that +// lane rather than at the batch's first ready row, and like every enqueue +// lock it is held until the transaction ends. From migration v057, while it +// holds claims or lanes it waits at most 500 ms (or half of its session's +// `deadlock_timeout`, if lower) for another; a longer wait releases them all +// and retries, so a per-row producer that holds a lane and waits for one of +// the batch's keys usually proceeds instead of deadlocking with it. After 2 s +// of retries it makes one last attempt with ordinary lock waits. // // Generic `insert_many` always uses the per-row path, as does // `insert_many_copy` on a schema before v055: rows are sorted by the diff --git a/awa-model/src/migrations.rs b/awa-model/src/migrations.rs index 9a7365a34..dcbbd36d4 100644 --- a/awa-model/src/migrations.rs +++ b/awa-model/src/migrations.rs @@ -5,7 +5,7 @@ use sqlx::{Connection, PgPool}; use tracing::{info, warn}; /// Current schema version. -pub const CURRENT_VERSION: i32 = 55; +pub const CURRENT_VERSION: i32 = 58; /// Migrations that require an exclusive (no-live-runtime) upgrade window. /// @@ -307,6 +307,21 @@ const MIGRATIONS: &[(i32, &str, &[&str])] = &[ "Batch producer insert with a short enqueue lane lock window", &[V23_UP, V55_UP], ), + ( + 56, + "Recognise only a full job_ids index as the claim batch index", + &[V23_UP, V56_UP], + ), + ( + 57, + "Batch producer insert that gives way instead of deadlocking", + &[V23_UP, V57_UP], + ), + ( + 58, + "Provision a lane's row before its lock", + &[V23_UP, V58_UP], + ), ]; const V1_UP: &str = include_str!("../migrations/v001_canonical_schema.sql"); @@ -370,6 +385,9 @@ const V52_UP: &str = include_str!("../migrations/v052_retained_job_output.sql"); const V53_UP: &str = include_str!("../migrations/v053_definer_lane_provisioning.sql"); const V54_UP: &str = include_str!("../migrations/v054_claim_batch_job_index.sql"); const V55_UP: &str = include_str!("../migrations/v055_insert_jobs_compat_batch.sql"); +const V56_UP: &str = include_str!("../migrations/v056_claim_batch_job_index_shape.sql"); +const V57_UP: &str = include_str!("../migrations/v057_insert_jobs_compat_batch_bounded_wait.sql"); +const V58_UP: &str = include_str!("../migrations/v058_provision_lane_row_first.sql"); /// Old version numbers from pre-0.4 releases that used V3/V4/V5 numbering. /// Also tolerates the unreleased inline-V6 branch numbering used during review. diff --git a/awa-model/src/queue_storage.rs b/awa-model/src/queue_storage.rs index 4b8d285b3..9241b013a 100644 --- a/awa-model/src/queue_storage.rs +++ b/awa-model/src/queue_storage.rs @@ -5206,6 +5206,22 @@ impl QueueStorage { enqueue_shard: i16, ) -> Result<(), AwaError> { let schema = self.schema(); + // The lane row comes before the lane lock, the order every producer + // takes them in: SQL inserts write the row, then take the lock in the + // head-table trigger that provisions the lane's sequences. + sqlx::query(audited_sql(format!( + r#" + INSERT INTO {schema}.queue_lanes (queue, priority) + VALUES ($1, $2) + ON CONFLICT (queue, priority) DO NOTHING + "# + ))) + .bind(queue) + .bind(priority) + .execute(tx.as_mut()) + .await + .map_err(map_sqlx_error)?; + let lane_lock_key = format!("{schema}:{queue}:{priority}:{enqueue_shard}"); let provisioner_callable = if self.lane_provisioner_present.load(Ordering::Relaxed) { sqlx::query("SELECT pg_advisory_xact_lock(hashtextextended($1, 0))") @@ -5237,19 +5253,6 @@ impl QueueStorage { return Ok(()); } - sqlx::query(audited_sql(format!( - r#" - INSERT INTO {schema}.queue_lanes (queue, priority) - VALUES ($1, $2) - ON CONFLICT (queue, priority) DO NOTHING - "# - ))) - .bind(queue) - .bind(priority) - .execute(tx.as_mut()) - .await - .map_err(map_sqlx_error)?; - // The head tables' BEFORE INSERT triggers run even for a row that // ON CONFLICT then skips, and they read and set the lane's sequence. // Skipping rows that exist keeps provisioning a live lane off a diff --git a/awa/tests/enqueue_lane_contention_test.rs b/awa/tests/enqueue_lane_contention_test.rs index 84e6f2f69..2945a4dc0 100644 --- a/awa/tests/enqueue_lane_contention_test.rs +++ b/awa/tests/enqueue_lane_contention_test.rs @@ -34,7 +34,7 @@ //! separately (insert_many mode only) //! AWA_LANE_BENCH_WITHOUT_BATCH_FUNCTION //! 0|1: drop `awa.insert_jobs_compat_batch` -//! and its v055 version row first, so +//! and the version rows from v055 first, so //! insert_many_copy measures its per-row path //! (the next migration run puts the function //! back) @@ -124,7 +124,14 @@ struct Samples { /// transaction. statement_ms: Vec, commit_ms: Vec, + /// Jobs the producers asked to insert in the measured window. + requested: u64, + /// Jobs the inserts returned in the measured window: requested jobs less + /// those skipped for a unique conflict. jobs: u64, + /// Jobs the inserts returned over the whole run, warmup and the batches + /// still in flight at the end of the window included. + acknowledged_total: u64, batches: u64, duplicate_batches: u64, fallback_rows: u64, @@ -136,7 +143,9 @@ impl Samples { self.fallback_row_ms.extend(other.fallback_row_ms); self.statement_ms.extend(other.statement_ms); self.commit_ms.extend(other.commit_ms); + self.requested += other.requested; self.jobs += other.jobs; + self.acknowledged_total += other.acknowledged_total; self.batches += other.batches; self.duplicate_batches += other.duplicate_batches; self.fallback_rows += other.fallback_rows; @@ -288,7 +297,10 @@ const LEGACY_ROW_FUNCTION_SQL: &str = r#" ORDER BY input.ord "#; -async fn legacy_row_function_insert(pool: &PgPool, jobs: &[InsertParams]) -> Result<(), AwaError> { +async fn legacy_row_function_insert( + pool: &PgPool, + jobs: &[InsertParams], +) -> Result { let rows: Vec<_> = jobs .iter() .map(|job| { @@ -296,7 +308,7 @@ async fn legacy_row_function_insert(pool: &PgPool, jobs: &[InsertParams]) -> Res .expect("prepare row") }) .collect(); - sqlx::query(LEGACY_ROW_FUNCTION_SQL) + let inserted = sqlx::query(LEGACY_ROW_FUNCTION_SQL) .bind(rows.iter().map(|row| row.kind()).collect::>()) .bind(rows.iter().map(|row| row.queue()).collect::>()) .bind(rows.iter().map(|row| row.args()).collect::>()) @@ -326,7 +338,7 @@ async fn legacy_row_function_insert(pool: &PgPool, jobs: &[InsertParams]) -> Res ) .fetch_all(pool) .await?; - Ok(()) + Ok(inserted.len()) } #[allow(clippy::too_many_arguments)] @@ -369,57 +381,65 @@ async fn producer_loop( let mut commit_ms = None; let mut fallback_rows_ms: Vec = Vec::new(); - let outcome: Result<(), AwaError> = match scenario.mode { + // Rows the insert returned (or, for the queue-storage batch, reports): + // a skipping insert omits the jobs whose unique key is taken. + let outcome: Result = match scenario.mode { Mode::InsertMany if scenario.split_commit => { let mut tx = pool.begin().await.expect("begin"); let statement_started = Instant::now(); let result = insert_many(&mut *tx, &jobs).await; let statement_elapsed = statement_started.elapsed(); match result { - Ok(_) => { + Ok(rows) => { let commit_started = Instant::now(); tx.commit().await.expect("commit"); statement_ms = Some(statement_elapsed.as_secs_f64() * 1e3); commit_ms = Some(commit_started.elapsed().as_secs_f64() * 1e3); - Ok(()) + Ok(rows.len()) } Err(err) => Err(err), } } - Mode::InsertMany => insert_many(&pool, &jobs).await.map(|_| ()), - Mode::InsertManyCopy => insert_many_copy_from_pool(&pool, &jobs).await.map(|_| ()), + Mode::InsertMany => insert_many(&pool, &jobs).await.map(|rows| rows.len()), + Mode::InsertManyCopy => insert_many_copy_from_pool(&pool, &jobs) + .await + .map(|rows| rows.len()), Mode::InsertManySkip => insert_many_skip_unique_conflicts(&pool, &jobs) .await - .map(|_| ()), - Mode::QueueStorage => store.enqueue_params_batch(&pool, &jobs).await.map(|_| ()), + .map(|rows| rows.len()), + Mode::QueueStorage => store.enqueue_params_batch(&pool, &jobs).await, Mode::LegacyRowFunction => legacy_row_function_insert(&pool, &jobs).await, }; let mut fallback_count = 0u64; - match outcome { - Ok(()) => {} + let inserted: u64 = match outcome { + Ok(rows) => rows as u64, Err(err) if is_unique_violation(&err) => { // What a replaying producer does on a unique conflict: // insert the batch again one row at a time, skipping // duplicates. + let mut fallback_inserted = 0u64; for job in &jobs { let row_started = Instant::now(); match insert_many(&pool, std::slice::from_ref(job)).await { - Ok(_) => {} + Ok(rows) => fallback_inserted += rows.len() as u64, Err(err) if is_unique_violation(&err) => {} Err(err) => panic!("fallback insert failed: {err}"), } fallback_rows_ms.push(row_started.elapsed().as_secs_f64() * 1e3); fallback_count += 1; } + fallback_inserted } Err(err) => panic!("enqueue failed: {err}"), - } + }; let elapsed = started.elapsed(); + samples.acknowledged_total += inserted; if started >= measure_from { samples.batch_ms.push(elapsed.as_secs_f64() * 1e3); - samples.jobs += scenario.batch_size as u64; + samples.requested += scenario.batch_size as u64; + samples.jobs += inserted; samples.batches += 1; if let Some(ms) = statement_ms { samples.statement_ms.push(ms); @@ -544,8 +564,8 @@ fn print_result(result: &ScenarioResult) { println!( "[lane-bench] {name} mode={mode} producers={producers} batch={batch} unique={unique} \ shards={shards} dup_pct={dup} | {rate:.0} jobs/s | batch ms p50={p50:.1} p95={p95:.1} \ - p99={p99:.1} max={max:.1} | waiters_mean={waiters:.1} | landed={landed} \ - distinct_seqs={distinct}", + p99={p99:.1} max={max:.1} | waiters_mean={waiters:.1} | inserted={inserted} of \ + requested={requested} | landed={landed} distinct_seqs={distinct}", name = scenario.name, mode = scenario.mode.label(), producers = scenario.producers, @@ -559,6 +579,8 @@ fn print_result(result: &ScenarioResult) { p99 = result.p99_ms, max = result.max_ms, waiters = result.mean_waiters, + inserted = result.samples.jobs, + requested = result.samples.requested, landed = result.landed, distinct = result.distinct_lane_seqs, ); @@ -597,6 +619,7 @@ fn print_result(result: &ScenarioResult) { fn emit_json(result: &ScenarioResult) { let mut outcomes = HashMap::new(); outcomes.insert("jobs".to_string(), result.samples.jobs); + outcomes.insert("requested".to_string(), result.samples.requested); outcomes.insert("batches".to_string(), result.samples.batches); BenchmarkResult { schema_version: SCHEMA_VERSION, @@ -626,9 +649,9 @@ fn emit_json(result: &ScenarioResult) { .emit(); } -/// Lane integrity: every job that was acknowledged landed exactly once, under -/// a distinct `(shard, lane_seq)`, and every shard's sequence numbers are -/// dense (no producer lost or duplicated a reservation). +/// Lane integrity: exactly the jobs the inserts returned landed, each once, +/// under a distinct `(shard, lane_seq)` (no producer lost or duplicated a +/// reservation). fn assert_lane_integrity(result: &ScenarioResult) { if let Some((claimed, _)) = result.drained { assert_eq!( @@ -642,12 +665,10 @@ fn assert_lane_integrity(result: &ScenarioResult) { "{}: ready rows share a (shard, lane_seq)", result.scenario.name ); - assert!( - result.landed as u64 >= result.samples.jobs, - "{}: {} jobs acknowledged in the window but only {} rows landed", - result.scenario.name, - result.samples.jobs, - result.landed + assert_eq!( + result.landed as u64, result.samples.acknowledged_total, + "{}: rows landed against jobs the inserts returned", + result.scenario.name ); } @@ -778,7 +799,7 @@ async fn test_enqueue_lane_contention_matrix() { "DROP FUNCTION IF EXISTS awa.insert_jobs_compat_batch(text[], text[], jsonb[], \ text[], smallint[], smallint[], timestamptz[], jsonb[], jsonb[], bytea[], text[], \ text[], boolean); \ - DELETE FROM awa.schema_version WHERE version = 55", + DELETE FROM awa.schema_version WHERE version >= 55", ) .execute(&pool) .await diff --git a/awa/tests/enqueue_lane_lock_order_test.rs b/awa/tests/enqueue_lane_lock_order_test.rs index 74e3fbb4b..d610124d4 100644 --- a/awa/tests/enqueue_lane_lock_order_test.rs +++ b/awa/tests/enqueue_lane_lock_order_test.rs @@ -11,16 +11,22 @@ //! with the lanes in the opposite input order is then observed waiting on the //! first lane's lock through `pg_locks`, and only then is the gate released. //! +//! The first producer to use a lane also writes its `queue_lanes` row and +//! takes its provisioning lock, and every producer must take the row first. +//! Those tests stall the first producer inside the lane's sequence creation +//! with a gate transaction holding an uncommitted sequence of the same name. +//! //! Requires a running Postgres instance. //! Run with: `cargo test --package awa --test enqueue_lane_lock_order_test -- --nocapture` use awa::audited_sql; use awa::model::{ - insert_many, insert_many_copy_from_pool, insert_many_skip_unique_conflicts, AwaError, - QueueStorage, QueueStorageConfig, + insert_many, insert_many_copy_from_pool, insert_many_skip_unique_conflicts, insert_with, + AwaError, QueueStorage, QueueStorageConfig, }; -use awa::{InsertOpts, InsertParams, JobRow, UniqueOpts}; +use awa::{InsertOpts, InsertParams, JobArgs, JobRow, UniqueOpts}; use awa_testing::setup; +use serde::Serialize; use sqlx::PgPool; use std::sync::LazyLock; use std::time::{Duration, Instant}; @@ -522,3 +528,269 @@ async fn insert_many_copy_reserves_sharded_lanes_in_canonical_order() { async fn insert_many_copy_csv_reserves_sharded_lanes_in_canonical_order() { assert_batch_reserves_lanes_in_canonical_order(Producer::CopyLarge).await; } + +/// Producers that can be the first to use a lane. +#[derive(Debug, Clone, Copy)] +enum LaneCreator { + QueueStorageBatch, + PerRowInsert, + InsertMany, + Copy, +} + +#[derive(Debug, Serialize, JobArgs)] +struct LaneCreationJob { + queue: String, + seq: usize, +} + +impl LaneCreator { + async fn run(self, pool: &PgPool, jobs: &[InsertParams]) -> Result { + match self { + LaneCreator::QueueStorageBatch => { + let store = QueueStorage::new(QueueStorageConfig::default())?; + store.enqueue_params_batch(pool, jobs).await + } + LaneCreator::PerRowInsert => { + let mut tx = pool.begin().await?; + for params in jobs { + let args = LaneCreationJob { + queue: params.opts.queue.clone(), + seq: params.args["seq"].as_u64().expect("seq arg") as usize, + }; + insert_with(&mut *tx, &args, params.opts.clone()).await?; + } + tx.commit().await?; + Ok(jobs.len()) + } + LaneCreator::InsertMany => insert_many(pool, jobs).await.map(|rows| rows.len()), + LaneCreator::Copy => insert_many_copy_from_pool(pool, jobs) + .await + .map(|rows| rows.len()), + } + } +} + +/// Wait until a backend of the pool tagged `application_name` is blocked on a +/// heavyweight lock. +async fn wait_for_lock_waiter(pool: &PgPool, application_name: &str) { + let started = Instant::now(); + loop { + let waiting: bool = sqlx::query_scalar( + r#" + SELECT EXISTS ( + SELECT 1 + FROM pg_stat_activity + WHERE application_name = $1 + AND state = 'active' + AND wait_event_type = 'Lock' + ) + "#, + ) + .bind(application_name) + .fetch_one(pool) + .await + .expect("poll pg_stat_activity"); + if waiting { + return; + } + assert!( + started.elapsed() < WAIT_DEADLINE, + "no {application_name} backend started waiting on a lock" + ); + tokio::time::sleep(Duration::from_millis(10)).await; + } +} + +/// Run `creator` on its own pool, tagged `application_name`. +async fn spawn_lane_creator( + creator: LaneCreator, + application_name: &str, + jobs: Vec, +) -> tokio::task::JoinHandle> { + let producer_pool = + setup::pool_with_url(&setup::database_url_with_app_name(application_name), 2).await; + tokio::spawn(async move { creator.run(&producer_pool, &jobs).await }) +} + +/// Two producers create the same two new lanes, A then B. A gate transaction +/// holds an uncommitted copy of lane A's enqueue sequence, so the first +/// producer stalls inside lane A's provisioning, after it has taken whatever +/// it takes before creating the sequence. The second producer is then observed +/// blocked on lane A before the gate rolls back. Both producers must commit. +async fn assert_lane_creators_do_not_deadlock(first: LaneCreator, second: LaneCreator) { + let _guard = LANE_LOCK_TEST_GUARD.lock().await; + let (pool, _config) = setup_queue_storage().await; + + let suffix = Uuid::new_v4().simple().to_string(); + let first_queue = format!("lane_create_a_{suffix}"); + let second_queue = format!("lane_create_b_{suffix}"); + + let sequence_name: String = sqlx::query_scalar( + "SELECT awa.queue_lane_sequence_name('queue_enqueue_seq', $1, 2::smallint, 0::smallint)", + ) + .bind(&first_queue) + .fetch_one(&pool) + .await + .expect("derive lane sequence name"); + let mut gate = pool.begin().await.expect("begin gate transaction"); + sqlx::query(audited_sql(format!( + "CREATE SEQUENCE awa.\"{sequence_name}\" AS bigint START WITH 1 MINVALUE 1 CACHE 1" + ))) + .execute(&mut *gate) + .await + .expect("gate lane sequence"); + + let jobs: Vec = [&first_queue, &second_queue] + .into_iter() + .flat_map(|queue| (0..2).map(move |seq| job(queue, seq, false))) + .collect(); + + let first_name = format!("lane_first_{suffix}"); + let first_task = spawn_lane_creator(first, &first_name, jobs.clone()).await; + wait_for_lock_waiter(&pool, &first_name).await; + + let second_name = format!("lane_second_{suffix}"); + let second_task = spawn_lane_creator(second, &second_name, jobs).await; + wait_for_lock_waiter(&pool, &second_name).await; + + gate.rollback().await.expect("release gate"); + + let first_result = tokio::time::timeout(WAIT_DEADLINE, first_task) + .await + .expect("first producer finished") + .expect("first producer task"); + let second_result = tokio::time::timeout(WAIT_DEADLINE, second_task) + .await + .expect("second producer finished") + .expect("second producer task"); + assert_eq!( + first_result.unwrap_or_else(|err| panic!("{first:?} after {second:?}: {err}")), + 4 + ); + assert_eq!( + second_result.unwrap_or_else(|err| panic!("{second:?} after {first:?}: {err}")), + 4 + ); + + for queue in [&first_queue, &second_queue] { + let stored: i64 = + sqlx::query_scalar("SELECT count(*) FROM awa.ready_entries WHERE queue = $1") + .bind(queue) + .fetch_one(&pool) + .await + .expect("count stored jobs"); + assert_eq!(stored, 4, "queue {queue}"); + } +} + +#[tokio::test] +async fn queue_storage_batch_and_per_row_insert_create_lanes_without_deadlock() { + assert_lane_creators_do_not_deadlock(LaneCreator::QueueStorageBatch, LaneCreator::PerRowInsert) + .await; +} + +#[tokio::test] +async fn queue_storage_batch_and_insert_many_create_lanes_without_deadlock() { + assert_lane_creators_do_not_deadlock(LaneCreator::QueueStorageBatch, LaneCreator::InsertMany) + .await; +} + +#[tokio::test] +async fn queue_storage_batch_and_insert_many_copy_create_lanes_without_deadlock() { + assert_lane_creators_do_not_deadlock(LaneCreator::QueueStorageBatch, LaneCreator::Copy).await; +} + +#[tokio::test] +async fn per_row_insert_and_queue_storage_batch_create_lanes_without_deadlock() { + assert_lane_creators_do_not_deadlock(LaneCreator::PerRowInsert, LaneCreator::QueueStorageBatch) + .await; +} + +#[tokio::test] +async fn queue_storage_batches_create_lanes_without_deadlock() { + assert_lane_creators_do_not_deadlock( + LaneCreator::QueueStorageBatch, + LaneCreator::QueueStorageBatch, + ) + .await; +} + +#[tokio::test] +async fn per_row_inserts_create_lanes_without_deadlock() { + assert_lane_creators_do_not_deadlock(LaneCreator::PerRowInsert, LaneCreator::PerRowInsert) + .await; +} + +#[tokio::test] +async fn insert_many_copies_create_lanes_without_deadlock() { + assert_lane_creators_do_not_deadlock(LaneCreator::Copy, LaneCreator::Copy).await; +} + +/// A direct `provision_lane` call that meets a producer's uncommitted lane row +/// waits for it without holding the lane lock, which the producer takes next. +#[tokio::test] +async fn provision_lane_writes_the_lane_row_before_the_lane_lock() { + let _guard = LANE_LOCK_TEST_GUARD.lock().await; + let (pool, _config) = setup_queue_storage().await; + + let queue = format!("provision_row_first_{}", Uuid::new_v4().simple()); + let provisioning = provisioning_lock_key(&pool, &queue).await; + + let mut producer = pool.begin().await.expect("begin producer transaction"); + sqlx::query("INSERT INTO awa.queue_lanes (queue, priority) VALUES ($1, 2)") + .bind(&queue) + .execute(&mut *producer) + .await + .expect("write the lane row"); + + let provision = { + let pool = pool.clone(); + let queue = queue.clone(); + tokio::spawn(async move { + sqlx::query("SELECT awa.provision_lane($1, 2::smallint, 0::smallint)") + .bind(queue) + .execute(&pool) + .await + }) + }; + let started = Instant::now(); + loop { + let waiting: bool = sqlx::query_scalar( + r#" + SELECT EXISTS ( + SELECT 1 + FROM pg_stat_activity + WHERE wait_event_type = 'Lock' + AND query LIKE 'SELECT awa.provision_lane(%' + ) + "#, + ) + .fetch_one(&pool) + .await + .expect("poll pg_stat_activity"); + if waiting { + break; + } + assert!( + started.elapsed() < WAIT_DEADLINE, + "provision_lane never waited for the producer's lane row" + ); + tokio::time::sleep(Duration::from_millis(10)).await; + } + + let acquired: bool = sqlx::query_scalar("SELECT pg_try_advisory_xact_lock($1)") + .bind(provisioning) + .fetch_one(&mut *producer) + .await + .expect("try the lane lock"); + assert!( + acquired, + "provision_lane must not hold the lane lock while it waits for the lane row" + ); + producer.commit().await.expect("commit producer"); + provision + .await + .expect("provision task") + .expect("provision_lane after the producer commits"); +} diff --git a/awa/tests/insert_many_batch_test.rs b/awa/tests/insert_many_batch_test.rs index 16088a4cb..a1ac1910a 100644 --- a/awa/tests/insert_many_batch_test.rs +++ b/awa/tests/insert_many_batch_test.rs @@ -1,22 +1,25 @@ //! Compatibility inserts and the batch insert function -//! `awa.insert_jobs_compat_batch` (migration v055). +//! `awa.insert_jobs_compat_batch` (migrations v055 and v057). //! //! A batch does its lane-independent work (job ids, unique claims) before it -//! registers and reserves its lanes one at a time in canonical order, and -//! holds a lane lock only for the registration, the reservation, the ready-row -//! writes and the commit. These tests cover that ordering, skip-on-conflict +//! registers and reserves its lanes one at a time in canonical order, taking +//! each lane's lock only when it reaches that lane; the lock is then held until +//! the transaction ends. These tests cover that ordering, skip-on-conflict //! inserts, and the shape of what lands in the lanes. //! //! Requires a running Postgres instance. //! Run with: `cargo test --package awa --test insert_many_batch_test -- --nocapture` +use awa::model::unique::compute_unique_key; use awa::model::{ insert_many, insert_many_copy_from_pool, insert_many_skip_unique_conflicts, AwaError, QueueStorage, }; use awa::{InsertOpts, InsertParams, JobState, QueueStorageConfig, UniqueOpts}; use awa_testing::setup; +use sqlx::postgres::{PgConnectOptions, PgPoolOptions}; use sqlx::PgPool; +use std::str::FromStr; use std::sync::LazyLock; use std::time::{Duration, Instant}; use tokio::sync::Mutex; @@ -112,13 +115,22 @@ enum Path { impl Path { const ALL: [Path; 3] = [Path::PerRow, Path::SkipUniqueConflicts, Path::Copy]; - async fn insert(self, pool: &PgPool, jobs: &[InsertParams]) -> Vec { + async fn try_insert( + self, + pool: &PgPool, + jobs: &[InsertParams], + ) -> Result, AwaError> { match self { Path::PerRow => insert_many(pool, jobs).await, Path::SkipUniqueConflicts => insert_many_skip_unique_conflicts(pool, jobs).await, Path::Copy => insert_many_copy_from_pool(pool, jobs).await, } - .unwrap_or_else(|err| panic!("{self:?} insert: {err:?}")) + } + + async fn insert(self, pool: &PgPool, jobs: &[InsertParams]) -> Vec { + self.try_insert(pool, jobs) + .await + .unwrap_or_else(|err| panic!("{self:?} insert: {err:?}")) } /// Ready segments a batch of `rows` ready rows in one lane, sharing a @@ -376,6 +388,154 @@ async fn canonical_storage_inserts_and_skips_row_by_row() { ); } +/// Call `awa.insert_jobs_compat_batch` directly with `p_skip_unique_conflicts` +/// set explicitly. The Rust entry points pass `true` whenever a batch has a +/// unique key, so only a direct caller reaches the failing mode with one. +async fn insert_through_batch_function( + pool: &PgPool, + jobs: &[InsertParams], + skip_unique_conflicts: bool, +) -> Result, sqlx::Error> { + let unique_keys: Vec>> = jobs + .iter() + .map(|job| { + job.opts.unique.as_ref().map(|unique| { + compute_unique_key( + &job.kind, + unique.by_queue.then_some(job.opts.queue.as_str()), + unique.by_args.then_some(&job.args), + unique.by_period, + ) + }) + }) + .collect(); + // PostgreSQL's leftmost bit is UniqueOpts bit 0. + let unique_states: Vec> = jobs + .iter() + .map(|job| { + job.opts.unique.as_ref().map(|unique| { + (0..8) + .map(|bit| { + if unique.states & (1 << bit) != 0 { + '1' + } else { + '0' + } + }) + .collect() + }) + }) + .collect(); + sqlx::query_as( + r#" + SELECT * + FROM awa.insert_jobs_compat_batch( + $1::text[], $2::text[], $3::jsonb[], + array_fill('available'::text, ARRAY[cardinality($1::text[])]), + array_fill(2::smallint, ARRAY[cardinality($1::text[])]), + array_fill(25::smallint, ARRAY[cardinality($1::text[])]), + array_fill(NULL::timestamptz, ARRAY[cardinality($1::text[])]), + array_fill('{}'::jsonb, ARRAY[cardinality($1::text[])]), + array_fill('[]'::jsonb, ARRAY[cardinality($1::text[])]), + $4::bytea[], $5::text[], + array_fill(NULL::text, ARRAY[cardinality($1::text[])]), + $6 + ) + "#, + ) + .bind(jobs.iter().map(|job| job.kind.as_str()).collect::>()) + .bind( + jobs.iter() + .map(|job| job.opts.queue.as_str()) + .collect::>(), + ) + .bind(jobs.iter().map(|job| &job.args).collect::>()) + .bind(unique_keys) + .bind(unique_states) + .bind(skip_unique_conflicts) + .fetch_all(pool) + .await +} + +async fn jobs_in_queue(pool: &PgPool, queue: &str) -> Vec { + sqlx::query_scalar("SELECT (args->>'seq')::bigint FROM awa.jobs WHERE queue = $1 ORDER BY 1") + .bind(queue) + .fetch_all(pool) + .await + .expect("read queue jobs") +} + +/// With `p_skip_unique_conflicts = false` a unique conflict, with an already +/// claimed key or within the batch, fails the whole batch with `23505` and +/// leaves nothing behind, on queue storage and on canonical storage. +#[tokio::test] +async fn batch_function_without_skip_fails_the_batch_on_a_unique_conflict() { + let _guard = ENGINE_GUARD.lock().await; + for queue_storage in [true, false] { + let pool = if queue_storage { + queue_storage_pool().await + } else { + let pool = setup::setup(8).await; + setup::reset_to_canonical(&pool).await; + pool + }; + let queue = unique_queue(if queue_storage { + "noskip_qs" + } else { + "noskip_canonical" + }); + insert_many(&pool, &[job(&queue, 1, true)]) + .await + .expect("seed"); + + for batch in [ + vec![ + job(&queue, 0, true), + job(&queue, 1, true), + job(&queue, 2, true), + ], + vec![ + job(&queue, 3, true), + job(&queue, 4, true), + job(&queue, 4, true), + ], + ] { + let err = insert_through_batch_function(&pool, &batch, false) + .await + .expect_err("a unique conflict must fail the batch"); + assert!( + is_unique_violation(&AwaError::Database(err)), + "queue_storage={queue_storage}" + ); + assert_eq!( + jobs_in_queue(&pool, &queue).await, + vec![1], + "queue_storage={queue_storage}: nothing from the failed batch landed" + ); + } + + let inserted = insert_through_batch_function( + &pool, + &[ + job(&queue, 0, true), + job(&queue, 1, true), + job(&queue, 2, true), + ], + true, + ) + .await + .expect("the same batch with skipping"); + assert_eq!( + inserted + .iter() + .map(|row| row.args["seq"].as_i64().unwrap()) + .collect::>(), + vec![0, 2], + "queue_storage={queue_storage}" + ); + } +} + /// While another transaction holds a lane lock, a batch bound for that lane /// has already done its lane-independent work (it holds its unique claims) and /// has not yet written a ready row. Every row's unique claim, not just the @@ -384,14 +544,28 @@ async fn canonical_storage_inserts_and_skips_row_by_row() { async fn batch_waits_for_the_lane_after_its_lane_independent_work() { let _guard = ENGINE_GUARD.lock().await; let pool = queue_storage_pool().await; + // The batch keeps its claims only for its first attempt's wait budget + // (500 ms by default), so a probe that lands later shows nothing; such a + // run is repeated rather than judged. + for _ in 0..3 { + if lane_window_holds_claims(&pool).await { + return; + } + } + panic!("every probe of the batch's claims landed after its first attempt's wait budget"); +} + +/// Returns false when the probe landed too late to judge. +async fn lane_window_holds_claims(pool: &PgPool) -> bool { + const JUDGEABLE: Duration = Duration::from_millis(400); let queue = unique_queue("window"); // Provision the lane so the gate lock is the only thing to wait for. - insert_many(&pool, &[job(&queue, -1, false)]) + insert_many(pool, &[job(&queue, -1, false)]) .await .expect("provision lane"); - let key = lane_lock_key(&pool, &queue).await; + let key = lane_lock_key(pool, &queue).await; let mut gate = pool.begin().await.expect("begin gate"); sqlx::query("SELECT pg_advisory_xact_lock($1)") .bind(key) @@ -400,13 +574,13 @@ async fn batch_waits_for_the_lane_after_its_lane_independent_work() { .expect("take lane lock"); let jobs_for_probe: Vec = (0..50).map(|seq| job(&queue, seq, true)).collect(); + let started = Instant::now(); let producer = { let pool = pool.clone(); let jobs = jobs_for_probe.clone(); tokio::spawn(async move { insert_many_skip_unique_conflicts(&pool, &jobs).await }) }; - let started = Instant::now(); let waiter = loop { let pid: Option = sqlx::query_scalar( "SELECT pid FROM pg_locks @@ -415,33 +589,16 @@ async fn batch_waits_for_the_lane_after_its_lane_independent_work() { LIMIT 1", ) .bind(key) - .fetch_optional(&pool) + .fetch_optional(pool) .await .expect("poll pg_locks"); if let Some(pid) = pid { break pid; } assert!(started.elapsed() < WAIT_DEADLINE, "producer never waited"); - tokio::time::sleep(Duration::from_millis(10)).await; + tokio::time::sleep(Duration::from_millis(5)).await; }; - let holds_relation_lock = |relation: &'static str| { - let pool = pool.clone(); - async move { - sqlx::query_scalar::<_, bool>( - "SELECT EXISTS ( - SELECT 1 FROM pg_locks - WHERE pid = $1 AND locktype = 'relation' AND granted - AND relation = $2::regclass - )", - ) - .bind(waiter) - .bind(relation) - .fetch_one(&pool) - .await - .expect("read relation locks") - } - }; // The claim of the batch's last row is already held, uncommitted: an // insert of the same key waits on the producer's transaction. let last = jobs_for_probe.last().expect("probe job"); @@ -452,7 +609,7 @@ async fn batch_waits_for_the_lane_after_its_lane_independent_work() { .expect("unique key") .to_vec(); let mut probe = pool.begin().await.expect("begin probe"); - sqlx::query("SET LOCAL lock_timeout = '300ms'") + sqlx::query("SET LOCAL lock_timeout = '20ms'") .execute(&mut *probe) .await .expect("set lock timeout"); @@ -463,17 +620,38 @@ async fn batch_waits_for_the_lane_after_its_lane_independent_work() { .bind(&last_key) .execute(&mut *probe) .await; + let probed_after = started.elapsed(); match probe_result { - Err(sqlx::Error::Database(db_err)) => assert_eq!( - db_err.code().as_deref(), - Some("55P03"), - "the last row's claim must already be held while the lane lock is awaited" + Err(sqlx::Error::Database(db_err)) if db_err.code().as_deref() == Some("55P03") => {} + Ok(_) if probed_after > JUDGEABLE => { + probe.rollback().await.expect("roll back probe"); + gate.rollback().await.expect("release lane lock"); + tokio::time::timeout(WAIT_DEADLINE, producer) + .await + .expect("producer finished") + .expect("producer task") + .expect("insert batch"); + return false; + } + other => panic!( + "claim of the last row was not held while waiting for the lane \ + ({probed_after:?} after the batch started): {other:?}" ), - other => panic!("claim of the last row was not held while waiting for the lane: {other:?}"), } drop(probe); + let writes_ready_rows: bool = sqlx::query_scalar( + "SELECT EXISTS ( + SELECT 1 FROM pg_locks + WHERE pid = $1 AND locktype = 'relation' AND granted + AND relation = 'awa.ready_entries'::regclass + )", + ) + .bind(waiter) + .fetch_one(pool) + .await + .expect("read relation locks"); assert!( - !holds_relation_lock("awa.ready_entries").await, + !writes_ready_rows, "no ready row is written while the lane lock is awaited" ); @@ -484,7 +662,8 @@ async fn batch_waits_for_the_lane_after_its_lane_independent_work() { .expect("producer task") .expect("insert batch"); assert_eq!(inserted.len(), 50); - assert_eq!(ready_lane_seqs(&pool, &queue).await.len(), 51); + assert_eq!(ready_lane_seqs(pool, &queue).await.len(), 51); + true } /// Producers on the per-row function, as released 0.6 binaries are, and @@ -655,3 +834,745 @@ async fn assert_batch_does_not_hold_a_new_lane_while_waiting(pool: &PgPool, path assert_eq!(ready_lane_seqs(pool, &lane_a).await.len(), 3, "{path:?}"); assert_eq!(ready_lane_seqs(pool, &lane_b).await.len(), 2, "{path:?}"); } + +/// Polls until some session waits for the lane lock `key`. +async fn wait_until_lane_awaited(pool: &PgPool, key: i64, what: &str) { + let started = Instant::now(); + loop { + let waiting: bool = sqlx::query_scalar( + "SELECT EXISTS (SELECT 1 FROM pg_locks + WHERE locktype = 'advisory' AND NOT granted + AND ((classid::bigint << 32) | objid::bigint) = $1)", + ) + .bind(key) + .fetch_one(pool) + .await + .expect("poll pg_locks"); + if waiting { + return; + } + assert!(started.elapsed() < WAIT_DEADLINE, "{what} never waited"); + tokio::time::sleep(Duration::from_millis(10)).await; + } +} + +/// A per-row producer holds the lane and then inserts a unique key that a +/// waiting batch has claimed but not committed. The batch gives up its claims +/// within its bounded lane wait, so the per-row insert proceeds instead of +/// both transactions waiting on each other until PostgreSQL aborts one with +/// `deadlock detected`. The batch then finds the key claimed and skips it. +#[tokio::test] +async fn per_row_insert_of_a_key_a_waiting_batch_claimed_does_not_deadlock() { + let _guard = ENGINE_GUARD.lock().await; + let pool = queue_storage_pool().await; + for path in [Path::SkipUniqueConflicts, Path::Copy] { + assert_batch_gives_up_claims_while_waiting_for_a_lane(&pool, &pool, path).await; + } +} + +/// The same cycle with the batch's session at a `deadlock_timeout` far above +/// the per-row producer's default of one second. The batch's wait budget is +/// capped, so it still gives way before the per-row producer's deadlock check. +#[tokio::test] +async fn batch_with_a_long_deadlock_timeout_still_gives_way_to_a_per_row_producer() { + let _guard = ENGINE_GUARD.lock().await; + let pool = queue_storage_pool().await; + let batch_pool = PgPoolOptions::new() + .max_connections(2) + .connect_with( + PgConnectOptions::from_str(&setup::database_url()) + .expect("parse database url") + .options([("deadlock_timeout", "20s")]), + ) + .await + .expect("connect with a long deadlock_timeout"); + for path in [Path::SkipUniqueConflicts, Path::Copy] { + assert_batch_gives_up_claims_while_waiting_for_a_lane(&pool, &batch_pool, path).await; + } +} + +async fn assert_batch_gives_up_claims_while_waiting_for_a_lane( + pool: &PgPool, + batch_pool: &PgPool, + path: Path, +) { + let queue = unique_queue("claim_cycle"); + insert_many(pool, &[job(&queue, -1, false)]) + .await + .expect("provision lane"); + + let mut per_row = pool.begin().await.expect("begin per-row producer"); + insert_many(&mut *per_row, &[job(&queue, 100, false)]) + .await + .expect("per-row insert takes the lane"); + + let batch = { + let batch_pool = batch_pool.clone(); + let jobs = vec![job(&queue, 0, true)]; + tokio::spawn(async move { path.try_insert(&batch_pool, &jobs).await }) + }; + wait_until_lane_awaited(pool, lane_lock_key(pool, &queue).await, "batch").await; + + let per_row_insert = tokio::time::timeout( + WAIT_DEADLINE, + insert_many(&mut *per_row, &[job(&queue, 0, true)]), + ) + .await + .unwrap_or_else(|_| panic!("{path:?}: per-row insert of the claimed key never finished")); + let inserted_by_row = per_row_insert + .unwrap_or_else(|err| panic!("{path:?}: per-row insert of the claimed key: {err:?}")); + assert_eq!(inserted_by_row.len(), 1, "{path:?}"); + per_row.commit().await.expect("commit per-row producer"); + + let inserted_by_batch = tokio::time::timeout(WAIT_DEADLINE, batch) + .await + .expect("batch finished") + .expect("batch task") + .unwrap_or_else(|err| panic!("{path:?}: batch: {err:?}")); + assert!( + inserted_by_batch.is_empty(), + "{path:?}: the batch skips the key the per-row producer committed" + ); + let seqs: Vec = ready_lane_seqs(pool, &queue) + .await + .into_iter() + .map(|(seq, _)| seq) + .collect(); + assert_eq!(seqs, vec![-1, 100, 0], "{path:?}"); + assert_eq!(claims_for_queue(pool, &queue).await, 1, "{path:?}"); +} + +/// A transaction that enqueues lane B and then lane A in two statements, and +/// a batch for lanes A and B that holds A while it waits for B. The batch +/// releases A within its bounded wait instead of deadlocking, then waits for +/// B without holding anything and lands once the transaction commits. +#[tokio::test] +async fn multi_statement_producer_and_batch_in_opposite_lane_order_do_not_deadlock() { + let _guard = ENGINE_GUARD.lock().await; + let pool = queue_storage_pool().await; + for path in [Path::SkipUniqueConflicts, Path::Copy] { + let suffix = &Uuid::new_v4().simple().to_string()[..12]; + let lane_a = format!("batch_order_a_{suffix}"); + let lane_b = format!("batch_order_b_{suffix}"); + insert_many(&pool, &[job(&lane_a, -1, false), job(&lane_b, -1, false)]) + .await + .expect("provision lanes"); + + let mut producer = pool.begin().await.expect("begin producer"); + insert_many(&mut *producer, &[job(&lane_b, 100, false)]) + .await + .expect("producer takes lane B"); + + let batch = { + let pool = pool.clone(); + let jobs = vec![job(&lane_a, 0, false), job(&lane_b, 0, false)]; + tokio::spawn(async move { path.try_insert(&pool, &jobs).await }) + }; + wait_until_lane_awaited(&pool, lane_lock_key(&pool, &lane_b).await, "batch").await; + + tokio::time::timeout( + WAIT_DEADLINE, + insert_many(&mut *producer, &[job(&lane_a, 100, false)]), + ) + .await + .unwrap_or_else(|_| panic!("{path:?}: producer never got lane A")) + .unwrap_or_else(|err| panic!("{path:?}: producer insert into lane A: {err:?}")); + producer.commit().await.expect("commit producer"); + + let inserted = tokio::time::timeout(WAIT_DEADLINE, batch) + .await + .expect("batch finished") + .expect("batch task") + .unwrap_or_else(|err| panic!("{path:?}: batch: {err:?}")); + assert_eq!(inserted.len(), 2, "{path:?}"); + let lane_a_seqs: Vec = ready_lane_seqs(&pool, &lane_a) + .await + .into_iter() + .map(|(seq, _)| seq) + .collect(); + assert_eq!(lane_a_seqs, vec![-1, 100, 0], "{path:?}"); + let lane_b_seqs: Vec = ready_lane_seqs(&pool, &lane_b) + .await + .into_iter() + .map(|(seq, _)| seq) + .collect(); + assert_eq!(lane_b_seqs, vec![-1, 100, 0], "{path:?}"); + } +} + +/// The caller's `lock_timeout` still ends a batch's lock wait: a lane held +/// throughout fails it with `lock_not_available`, as it fails a per-row +/// insert, and the batch leaves its unique claims behind. +#[tokio::test] +async fn batch_gives_up_at_the_callers_lock_timeout() { + let _guard = ENGINE_GUARD.lock().await; + let pool = queue_storage_pool().await; + let queue = unique_queue("caller_timeout"); + insert_many(&pool, &[job(&queue, -1, false)]) + .await + .expect("provision lane"); + + let mut gate = pool.begin().await.expect("begin gate"); + sqlx::query("SELECT pg_advisory_xact_lock($1)") + .bind(lane_lock_key(&pool, &queue).await) + .execute(&mut *gate) + .await + .expect("take lane lock"); + + let mut producer = pool.begin().await.expect("begin producer"); + sqlx::query("SET LOCAL lock_timeout = '300ms'") + .execute(&mut *producer) + .await + .expect("set lock timeout"); + let started = Instant::now(); + let err = insert_many_skip_unique_conflicts(&mut *producer, &[job(&queue, 0, true)]) + .await + .expect_err("the lane stays held past the caller's lock_timeout"); + assert!( + matches!(&err, AwaError::Database(sqlx::Error::Database(db_err)) + if db_err.code().as_deref() == Some("55P03")), + "{err:?}" + ); + assert!(started.elapsed() < WAIT_DEADLINE); + producer.rollback().await.expect("roll back producer"); + gate.rollback().await.expect("release lane lock"); + + let inserted = insert_many_skip_unique_conflicts(&pool, &[job(&queue, 0, true)]) + .await + .expect("insert once the lane is free"); + assert_eq!(inserted.len(), 1, "the key was not left claimed"); +} + +/// Batch and per-row producers race on the same unique keys and lanes, round +/// after round. Per-row producers enqueue the last lane in one statement and +/// the others in a second, so they hold a lane while they claim keys and wait +/// for lanes that batches hold. No producer may fail with `deadlock +/// detected`, every key lands exactly once, and the rows in the lanes are +/// exactly the rows the committed producers returned. +#[tokio::test] +async fn mixed_producers_on_shared_keys_and_lanes_never_deadlock() { + const TASKS: usize = 8; + const ROUNDS: usize = 12; + const KEYS: usize = 6; + + let _guard = ENGINE_GUARD.lock().await; + let pool = setup::setup(TASKS as u32 + 4).await; + setup::activate_queue_storage(&pool).await; + let suffix = Uuid::new_v4().simple().to_string()[..12].to_string(); + let lanes: Vec = ["a", "b", "c"] + .iter() + .map(|lane| format!("batch_mix_{lane}_{suffix}")) + .collect(); + insert_many( + &pool, + &lanes + .iter() + .map(|lane| job(lane, -1, false)) + .collect::>(), + ) + .await + .expect("provision lanes"); + + let barrier = std::sync::Arc::new(tokio::sync::Barrier::new(TASKS)); + let mut tasks = tokio::task::JoinSet::new(); + for task in 0..TASKS { + let pool = pool.clone(); + let barrier = barrier.clone(); + let lanes = lanes.clone(); + tasks.spawn(async move { + let mut committed: Vec = Vec::new(); + let mut deadlocks = 0usize; + for round in 0..ROUNDS { + // Each task takes the round's keys in its own order. + let mut keys: Vec = (0..KEYS).collect(); + keys.rotate_left((task * 5 + round) % KEYS); + if (task + round) % 2 == 1 { + keys.reverse(); + } + let mut jobs: Vec = keys + .iter() + .map(|&key| keyed_job(&lanes, round, key)) + .collect(); + jobs.extend((0..lanes.len()).map(|lane| filler_job(&lanes, round, task, lane))); + barrier.wait().await; + + let outcome: Result, AwaError> = match task % 4 { + 0 | 1 => { + // Per-row producers share one statement order, last + // lane first and keys ascending, so they cannot + // deadlock with each other. + let last_lane = &lanes[lanes.len() - 1]; + let mut sorted = jobs.clone(); + sorted.sort_by_key(|job| { + job.args + .get("key") + .and_then(|key| key.as_u64()) + .unwrap_or(u64::MAX) + }); + let (first, second): (Vec, Vec) = sorted + .into_iter() + .partition(|job| &job.opts.queue == last_lane); + async { + let mut tx = pool.begin().await?; + let mut rows = insert_many(&mut *tx, &first).await?; + rows.extend(insert_many(&mut *tx, &second).await?); + tx.commit().await?; + Ok(rows) + } + .await + } + 2 => insert_many_skip_unique_conflicts(&pool, &jobs).await, + _ => insert_many_copy_from_pool(&pool, &jobs).await, + }; + match outcome { + Ok(rows) => committed.extend(rows.iter().map(|row| row.id)), + Err(err) if err.is_deadlock() => deadlocks += 1, + Err(err) if task % 4 < 2 && is_unique_violation(&err) => {} + Err(err) => panic!("task {task} round {round}: {err:?}"), + } + } + (committed, deadlocks) + }); + } + + let mut committed: Vec = Vec::new(); + let mut deadlocks = 0usize; + while let Some(result) = tasks.join_next().await { + match result { + Ok((ids, task_deadlocks)) => { + committed.extend(ids); + deadlocks += task_deadlocks; + } + Err(err) => { + tasks.abort_all(); + std::panic::resume_unwind(err.into_panic()); + } + } + } + assert_eq!(deadlocks, 0, "producers failed with deadlock detected"); + + let landed: Vec<(i64, serde_json::Value)> = sqlx::query_as( + "SELECT job_id, args FROM awa.ready_entries + WHERE queue = ANY($1) AND args ? 'round'", + ) + .bind(&lanes) + .fetch_all(&pool) + .await + .expect("read lanes"); + let mut landed_ids: Vec = landed.iter().map(|(id, _)| *id).collect(); + landed_ids.sort_unstable(); + committed.sort_unstable(); + assert_eq!( + landed_ids, committed, + "the lanes hold exactly the rows committed producers returned" + ); + for round in 0..ROUNDS { + for key in 0..KEYS { + let copies = landed + .iter() + .filter(|(_, args)| { + args.get("round") == Some(&serde_json::json!(round)) + && args.get("key") == Some(&serde_json::json!(key)) + }) + .count(); + assert_eq!(copies, 1, "round {round} key {key} lands exactly once"); + } + } +} + +/// Round `round`'s unique job `key`, in lane `key % lanes.len()`. +fn keyed_job(lanes: &[String], round: usize, key: usize) -> InsertParams { + InsertParams { + kind: "insert_many_batch_job".to_string(), + args: serde_json::json!({ "round": round, "key": key }), + opts: InsertOpts { + queue: lanes[key % lanes.len()].clone(), + unique: Some(UniqueOpts { + by_queue: true, + by_args: true, + ..Default::default() + }), + ..Default::default() + }, + } +} + +/// A job without a unique key that only `task` inserts. +fn filler_job(lanes: &[String], round: usize, task: usize, lane: usize) -> InsertParams { + InsertParams { + kind: "insert_many_batch_job".to_string(), + args: serde_json::json!({ "round": round, "task": task, "lane": lane }), + opts: InsertOpts { + queue: lanes[lane].clone(), + ..Default::default() + }, + } +} + +/// A batch whose caller already holds a lane from an earlier statement, and a +/// per-row producer that holds one of the batch's keys and then waits for that +/// lane. Rolling back the batch's attempts cannot release the caller's lane, +/// so this is a real deadlock. It must end, as it did before the batch had a +/// wait budget, with PostgreSQL aborting one side, not with the batch retrying +/// forever. The per-row producer's session has a long `deadlock_timeout`, so +/// only the batch's side can run the deadlock check in time. +#[tokio::test] +async fn batch_in_a_transaction_that_already_holds_a_lane_ends_a_cycle() { + let _guard = ENGINE_GUARD.lock().await; + let pool = queue_storage_pool().await; + let patient_pool = PgPoolOptions::new() + .max_connections(1) + .connect_with( + PgConnectOptions::from_str(&setup::database_url()) + .expect("parse database url") + .options([("deadlock_timeout", "60s")]), + ) + .await + .expect("connect with a long deadlock_timeout"); + let suffix = &Uuid::new_v4().simple().to_string()[..12]; + let lane_1 = format!("batch_caller_1_{suffix}"); + let lane_2 = format!("batch_caller_2_{suffix}"); + insert_many(&pool, &[job(&lane_1, -1, false), job(&lane_2, -1, false)]) + .await + .expect("provision lanes"); + + // The per-row producer claims the key and holds lane 2. + let mut per_row = patient_pool.begin().await.expect("begin per-row producer"); + insert_many(&mut *per_row, &[job(&lane_2, 0, true)]) + .await + .expect("per-row producer claims the key"); + + // The caller holds lane 1, then its batch waits for the key. + let (holds_lane_1, lane_1_held) = tokio::sync::oneshot::channel(); + let caller = { + let pool = pool.clone(); + let lane_1 = lane_1.clone(); + let lane_2 = lane_2.clone(); + tokio::spawn(async move { + let mut tx = pool.begin().await?; + insert_many(&mut *tx, &[job(&lane_1, 100, false)]).await?; + let _ = holds_lane_1.send(()); + let inserted = + insert_many_skip_unique_conflicts(&mut *tx, &[job(&lane_2, 0, true)]).await?; + tx.commit().await?; + Ok::<_, AwaError>(inserted) + }) + }; + lane_1_held.await.expect("the caller took lane 1"); + let per_row_outcome = tokio::time::timeout(WAIT_DEADLINE, async { + insert_many(&mut *per_row, &[job(&lane_1, 200, false)]).await?; + per_row.commit().await?; + Ok::<_, AwaError>(()) + }) + .await + .expect("the per-row producer's side of the cycle ended"); + let caller_outcome = tokio::time::timeout(WAIT_DEADLINE, caller) + .await + .expect("the caller's side of the cycle ended") + .expect("caller task"); + + let deadlocks = [ + per_row_outcome + .as_ref() + .err() + .is_some_and(AwaError::is_deadlock), + caller_outcome + .as_ref() + .err() + .is_some_and(AwaError::is_deadlock), + ]; + assert_eq!( + deadlocks.iter().filter(|aborted| **aborted).count(), + 1, + "exactly one side is aborted with deadlock detected: per-row {per_row_outcome:?}, \ + caller {caller_outcome:?}" + ); + assert!(per_row_outcome.is_ok() || caller_outcome.is_ok()); +} + +/// Two lanes held for longer than the batch's bounded phase: the batch's last +/// attempt waits for them as an ordinary producer would, and lands as soon as +/// they are released. +#[tokio::test] +async fn batch_lands_once_two_long_held_lanes_are_released() { + let _guard = ENGINE_GUARD.lock().await; + let pool = queue_storage_pool().await; + let suffix = &Uuid::new_v4().simple().to_string()[..12]; + let lane_a = format!("batch_held_a_{suffix}"); + let lane_b = format!("batch_held_b_{suffix}"); + insert_many(&pool, &[job(&lane_a, -1, false), job(&lane_b, -1, false)]) + .await + .expect("provision lanes"); + + let mut gates = Vec::new(); + for lane in [&lane_a, &lane_b] { + let mut gate = pool.begin().await.expect("begin gate"); + sqlx::query("SELECT pg_advisory_xact_lock($1)") + .bind(lane_lock_key(&pool, lane).await) + .execute(&mut *gate) + .await + .expect("take lane lock"); + gates.push(gate); + } + let started = Instant::now(); + let batch = { + let pool = pool.clone(); + let jobs = vec![job(&lane_a, 0, true), job(&lane_b, 0, true)]; + tokio::spawn(async move { insert_many_skip_unique_conflicts(&pool, &jobs).await }) + }; + // Past the bounded phase (four 500 ms budgets). + tokio::time::sleep(Duration::from_millis(3_000)).await; + assert!( + !batch.is_finished(), + "the batch waits while the lanes are held" + ); + for gate in gates { + gate.commit().await.expect("release lane"); + } + let inserted = tokio::time::timeout(WAIT_DEADLINE, batch) + .await + .expect("batch finished") + .expect("batch task") + .expect("insert batch"); + assert_eq!(inserted.len(), 2); + assert!( + started.elapsed() < Duration::from_secs(5), + "landed promptly on release" + ); + for lane in [&lane_a, &lane_b] { + let seqs: Vec = ready_lane_seqs(&pool, lane) + .await + .into_iter() + .map(|(seq, _)| seq) + .collect(); + assert_eq!(seqs, vec![-1, 0]); + } +} + +/// The claim cycle with a large batch whose last key in key order is the one +/// the per-row producer inserts. Preparing the result rows happens before the +/// wait budget starts, but inserting the claims counts against it: the batch +/// can only give way while its claims take less than the budget (500 ms), and +/// a larger batch behaves as before v057. The test first times inserting as +/// many claims. When that fits well inside the budget the batch must give way; otherwise the cycle must still end, with both sides done or +/// exactly one aborted by `deadlock detected`. +#[tokio::test] +async fn large_batch_gives_way_while_its_claims_fit_the_wait_budget() { + const ROWS: i64 = 5_000; + const FITS: Duration = Duration::from_millis(250); + let _guard = ENGINE_GUARD.lock().await; + let pool = queue_storage_pool().await; + + // What inserting this many claims costs here, the part of the batch's + // work that counts against its wait budget. + let mut calibration = pool.begin().await.expect("begin calibration"); + let started = Instant::now(); + sqlx::query( + "INSERT INTO awa.job_unique_claims (unique_key, job_id) + SELECT decode(md5($1 || g::text), 'hex'), -g + FROM generate_series(1, $2) AS g + ORDER BY 1 + ON CONFLICT (unique_key) DO NOTHING", + ) + .bind(unique_queue("large_calibration")) + .bind(ROWS) + .execute(&mut *calibration) + .await + .expect("time a claim insert"); + let claim_cost = started.elapsed(); + calibration.rollback().await.expect("roll back calibration"); + let fits_budget = claim_cost < FITS; + + let queue = unique_queue("large_cycle"); + insert_many(&pool, &[job(&queue, -1, false)]) + .await + .expect("provision lane"); + let jobs: Vec = (0..ROWS).map(|seq| job(&queue, seq, true)).collect(); + let last_in_key_order = jobs + .iter() + .max_by_key(|job| { + awa::model::prepare_raw_job_insert(job.kind.clone(), job.args.clone(), job.opts.clone()) + .expect("prepare job") + .unique_key() + .expect("unique key") + .to_vec() + }) + .expect("a job") + .clone(); + + let mut per_row = pool.begin().await.expect("begin per-row producer"); + insert_many(&mut *per_row, &[job(&queue, ROWS + 1, false)]) + .await + .expect("per-row insert takes the lane"); + let batch = { + let pool = pool.clone(); + tokio::spawn(async move { insert_many_skip_unique_conflicts(&pool, &jobs).await }) + }; + wait_until_lane_awaited(&pool, lane_lock_key(&pool, &queue).await, "batch").await; + + let per_row_outcome = tokio::time::timeout(WAIT_DEADLINE, async { + insert_many(&mut *per_row, &[last_in_key_order]).await?; + per_row.commit().await?; + Ok::<_, AwaError>(()) + }) + .await + .expect("the per-row producer's side of the cycle ended"); + let batch_outcome = tokio::time::timeout(WAIT_DEADLINE, batch) + .await + .expect("the batch's side of the cycle ended") + .expect("batch task"); + + if fits_budget { + per_row_outcome.unwrap_or_else(|err| { + panic!("per-row insert of the batch's last key (claims take {claim_cost:?}): {err:?}") + }); + let inserted = batch_outcome + .unwrap_or_else(|err| panic!("batch (claims take {claim_cost:?}): {err:?}")); + assert_eq!( + inserted.len() as i64, + ROWS - 1, + "the batch skips the committed key" + ); + assert_eq!(ready_lane_seqs(&pool, &queue).await.len() as i64, ROWS + 2); + } else { + let aborted = [ + &per_row_outcome.as_ref().err(), + &batch_outcome.as_ref().err(), + ] + .iter() + .filter(|err| err.is_some_and(AwaError::is_deadlock)) + .count(); + let both_done = per_row_outcome.is_ok() && batch_outcome.is_ok(); + assert!( + both_done || (aborted == 1 && (per_row_outcome.is_ok() || batch_outcome.is_ok())), + "a batch whose claims take {claim_cost:?} ends as before v057: per-row {per_row_outcome:?}, \ + batch {batch_outcome:?}" + ); + } +} + +/// The caller's `lock_timeout` applies to each lock wait, as it does in any +/// statement, not to the whole call: a batch whose own work before its lane +/// wait takes longer than the caller's `lock_timeout` still lands when the +/// lane is released within that timeout once it starts waiting. +#[tokio::test] +async fn callers_lock_timeout_applies_to_each_wait() { + const ROWS: i64 = 10_000; + let _guard = ENGINE_GUARD.lock().await; + let pool = queue_storage_pool().await; + let queue = unique_queue("caller_wait"); + insert_many(&pool, &[job(&queue, -1, false)]) + .await + .expect("provision lane"); + + let key = lane_lock_key(&pool, &queue).await; + let mut gate = pool.begin().await.expect("begin gate"); + sqlx::query("SELECT pg_advisory_xact_lock($1)") + .bind(key) + .execute(&mut *gate) + .await + .expect("take lane lock"); + + let jobs: Vec = (0..ROWS).map(|seq| job(&queue, seq, true)).collect(); + let batch = { + let pool = pool.clone(); + tokio::spawn(async move { + let mut tx = pool.begin().await?; + sqlx::query("SET LOCAL lock_timeout = '100ms'") + .execute(&mut *tx) + .await?; + let inserted = insert_many_skip_unique_conflicts(&mut *tx, &jobs).await?; + tx.commit().await?; + Ok::<_, AwaError>(inserted.len()) + }) + }; + let started = Instant::now(); + let pre_lane_work = loop { + let waited_after: Option = sqlx::query_scalar( + "SELECT EXTRACT(EPOCH FROM locks.waitstart - activity.query_start)::float8 + FROM pg_locks AS locks + JOIN pg_stat_activity AS activity USING (pid) + WHERE locks.locktype = 'advisory' AND NOT locks.granted + AND locks.waitstart IS NOT NULL + AND ((locks.classid::bigint << 32) | locks.objid::bigint) = $1", + ) + .bind(key) + .fetch_optional(&pool) + .await + .expect("poll pg_locks"); + if let Some(seconds) = waited_after { + break Duration::from_secs_f64(seconds); + } + assert!( + !batch.is_finished(), + "batch finished without waiting for the lane" + ); + assert!(started.elapsed() < WAIT_DEADLINE, "batch never waited"); + tokio::time::sleep(Duration::from_millis(2)).await; + }; + gate.commit().await.expect("release lane lock"); + + let inserted = tokio::time::timeout(WAIT_DEADLINE, batch) + .await + .expect("batch finished") + .expect("batch task") + .expect("batch with a 100 ms lock_timeout"); + assert_eq!(inserted as i64, ROWS); + assert!( + pre_lane_work > Duration::from_millis(100), + "the batch's work before its lane wait outlasts the caller's lock_timeout ({pre_lane_work:?})" + ); +} + +/// The documented limit of the bounded phase: a per-row producer that holds a +/// lane for longer than the batch's retries (about 2 s), and only then inserts +/// a key the batch has claimed, meets the batch's ordinary last attempt. The +/// cycle then ends as before v057, with one side aborted by `deadlock detected`. +#[tokio::test] +async fn per_row_producer_holding_a_lane_past_the_bounded_phase_meets_an_ordinary_wait() { + let _guard = ENGINE_GUARD.lock().await; + let pool = queue_storage_pool().await; + let queue = unique_queue("long_hold"); + insert_many(&pool, &[job(&queue, -1, false)]) + .await + .expect("provision lane"); + + let mut per_row = pool.begin().await.expect("begin per-row producer"); + insert_many(&mut *per_row, &[job(&queue, 100, false)]) + .await + .expect("per-row insert takes the lane"); + let batch = { + let pool = pool.clone(); + let queue = queue.clone(); + tokio::spawn(async move { + insert_many_skip_unique_conflicts(&pool, &[job(&queue, 0, true)]).await + }) + }; + tokio::time::sleep(Duration::from_millis(3_000)).await; + let per_row_outcome = tokio::time::timeout(WAIT_DEADLINE, async { + insert_many(&mut *per_row, &[job(&queue, 0, true)]).await?; + per_row.commit().await?; + Ok::<_, AwaError>(()) + }) + .await + .expect("the per-row producer's side of the cycle ended"); + let batch_outcome = tokio::time::timeout(WAIT_DEADLINE, batch) + .await + .expect("the batch's side of the cycle ended") + .expect("batch task"); + let aborted = [ + per_row_outcome + .as_ref() + .err() + .is_some_and(AwaError::is_deadlock), + batch_outcome + .as_ref() + .err() + .is_some_and(AwaError::is_deadlock), + ]; + assert_eq!( + aborted.iter().filter(|aborted| **aborted).count(), + 1, + "exactly one side is aborted: per-row {per_row_outcome:?}, batch {batch_outcome:?}" + ); +} diff --git a/awa/tests/insert_many_schema_fallback_test.rs b/awa/tests/insert_many_schema_fallback_test.rs index 81b62a575..38fd39342 100644 --- a/awa/tests/insert_many_schema_fallback_test.rs +++ b/awa/tests/insert_many_schema_fallback_test.rs @@ -1,5 +1,6 @@ //! `insert_many` and `insert_many_copy` against a schema without -//! `awa.insert_jobs_compat_batch` (migration v055), and the migration itself. +//! `awa.insert_jobs_compat_batch` (migration v055), and the migrations that +//! install it (v055) and replace its body (v057). //! //! A binary may run ahead of `awa migrate`. Generic `insert_many` keeps using //! the per-row function; COPY probes its own connection and uses the batch @@ -25,6 +26,7 @@ use uuid::Uuid; const BATCH_FUNCTION: &str = "insert_jobs_compat_batch"; const BATCH_FUNCTION_VERSION: i32 = 55; +const BOUNDED_WAIT_VERSION: i32 = 57; const DROP_BATCH_FUNCTION: &str = "DROP FUNCTION awa.insert_jobs_compat_batch(text[], text[], \ jsonb[], text[], smallint[], smallint[], timestamptz[], jsonb[], jsonb[], bytea[], text[], \ text[], boolean)"; @@ -361,6 +363,87 @@ async fn exported_migration_reapplies_in_one_transaction() { db.drop().await; } +/// TRUE when the installed batch function has the body v057 installs, which +/// bounds its lock waits and applies the caller's `lock_timeout` to each. +async fn batch_function_gives_way(pool: &PgPool) -> bool { + sqlx::query_scalar( + "SELECT p.prosrc ~ 'v_bounded_until := clock_timestamp\\(\\) \\+ 4 \\* v_wait_budget' \ + AND p.prosrc ~ 'v_caller_wait_ms BETWEEN 1 AND v_wait_ms' \ + FROM pg_proc AS p WHERE p.oid = to_regprocedure(\ + 'awa.insert_jobs_compat_batch(text[],text[],jsonb[],text[],smallint[],smallint[],\ + timestamptz[],jsonb[],jsonb[],bytea[],text[],text[],boolean)')", + ) + .fetch_one(pool) + .await + .expect("read the batch function body") +} + +/// The database as v056 left it: the batch function has v055's body, which +/// waited for a lane without bound while holding the batch's unique claims. +async fn roll_back_to_the_unbounded_batch_function(pool: &PgPool) { + let (_, _, v055) = + migrations::migration_sql_range(BATCH_FUNCTION_VERSION - 1, BATCH_FUNCTION_VERSION) + .pop() + .expect("v055 is a migration"); + sqlx::raw_sql(audited_sql(v055)) + .execute(pool) + .await + .expect("install v055's body"); + sqlx::query("DELETE FROM awa.schema_version WHERE version >= $1") + .bind(BOUNDED_WAIT_VERSION) + .execute(pool) + .await + .expect("roll the schema version back"); +} + +/// v057 replaces v055's body of the batch function: through `awa migrate`, +/// and through the exported SQL applied twice in one explicit transaction and +/// then over the installed function. +#[tokio::test] +async fn migration_replaces_the_unbounded_batch_function_body() { + let db = ScratchDatabase::create().await; + let pool = &db.pool; + assert!(batch_function_gives_way(pool).await); + + roll_back_to_the_unbounded_batch_function(pool).await; + assert!(!batch_function_gives_way(pool).await); + migrations::run(pool).await.expect("migrate"); + assert!(batch_function_gives_way(pool).await); + assert_eq!( + migrations::current_version(pool).await.expect("version"), + migrations::CURRENT_VERSION + ); + + let (_, _, sql) = + migrations::migration_sql_range(BOUNDED_WAIT_VERSION - 1, BOUNDED_WAIT_VERSION) + .pop() + .expect("v057 is a migration"); + roll_back_to_the_unbounded_batch_function(pool).await; + sqlx::raw_sql(audited_sql(format!("BEGIN;\n{sql}\n{sql}\nCOMMIT;"))) + .execute(pool) + .await + .expect("apply v057 twice in one transaction"); + sqlx::raw_sql(audited_sql(sql)) + .execute(pool) + .await + .expect("apply over the installed function"); + assert!(batch_function_gives_way(pool).await); + let recorded: i64 = + sqlx::query_scalar("SELECT count(*) FROM awa.schema_version WHERE version = $1") + .bind(BOUNDED_WAIT_VERSION) + .fetch_one(pool) + .await + .expect("read schema versions"); + assert_eq!(recorded, 1); + let copied = insert_many_copy_from_pool(pool, &jobs("bounded", 0..4, true)) + .await + .expect("COPY through the exported migration"); + assert_eq!(seqs(&copied), vec![0, 1, 2, 3]); + assert_eq!(segments(pool, "bounded").await, 1); + + db.drop().await; +} + /// A role that may not execute the batch function keeps COPY on the per-row /// path instead of failing it with `permission denied`. #[tokio::test] diff --git a/awa/tests/lane_ownership_roles_test.rs b/awa/tests/lane_ownership_roles_test.rs index 056990287..1d1cc329a 100644 --- a/awa/tests/lane_ownership_roles_test.rs +++ b/awa/tests/lane_ownership_roles_test.rs @@ -568,17 +568,18 @@ async fn concurrent_first_touch_of_one_lane_by_two_roles() -> TestResult { let mut blocked = false; for _ in 0..100 { blocked = sqlx::query_scalar( - "SELECT EXISTS (SELECT 1 FROM pg_locks WHERE locktype = 'advisory' AND NOT granted \ - AND database = (SELECT oid FROM pg_database WHERE datname = current_database()))", + "SELECT EXISTS (SELECT 1 FROM pg_stat_activity \ + WHERE datname = $1 AND wait_event_type = 'Lock')", ) - .fetch_one(&cluster.owner) + .bind(&cluster.database) + .fetch_one(&cluster.admin) .await?; if blocked { break; } tokio::time::sleep(Duration::from_millis(20)).await; } - assert!(blocked, "B's first touch should wait on A's lane lock"); + assert!(blocked, "B's first touch should wait on A's lane"); holding.commit().await?; assert_eq!(waiting.await??, 1); @@ -867,6 +868,163 @@ async fn set_only_schema_owner_membership_repairs_legacy_lanes_and_restores_role cluster.teardown().await } +/// Who may execute the default schema's `ensure_claim_batch_job_index`. +#[derive(Debug, PartialEq, sqlx::FromRow)] +struct ClaimBatchIndexAcl { + owner: String, + public: bool, + runtime: bool, + migrator: bool, +} + +async fn claim_batch_job_index_function_acl( + cluster: &Cluster, +) -> Result { + sqlx::query_as( + "SELECT pg_get_userbyid(routine.proowner)::text AS owner, \ + has_function_privilege('public', routine.oid, 'EXECUTE') AS public, \ + has_function_privilege($1, routine.oid, 'EXECUTE') AS runtime, \ + has_function_privilege($2, routine.oid, 'EXECUTE') AS migrator \ + FROM pg_proc AS routine \ + WHERE routine.oid = to_regprocedure('awa.ensure_claim_batch_job_index(integer)')", + ) + .bind(&cluster.runtime_name) + .bind(&cluster.migrator_name) + .fetch_one(&cluster.owner) + .await +} + +/// Give `ensure_claim_batch_job_index` to the migration role, executable by +/// `PUBLIC`, as a refresh by the migration role acting as itself would +/// create it. Needs the migration role to hold the owner's privileges. +async fn hand_claim_batch_job_index_to_migrator(cluster: &Cluster) -> TestResult { + cluster + .migrator + .execute(sqlx::raw_sql(sql(format!( + "ALTER FUNCTION awa.ensure_claim_batch_job_index(integer) OWNER TO {migrator}; \ + GRANT EXECUTE ON FUNCTION awa.ensure_claim_batch_job_index(integer) TO PUBLIC", + migrator = cluster.migrator_name + )))) + .await?; + Ok(()) +} + +/// `ensure_claim_batch_job_index` ends up owned by the table owner and +/// executable by exactly the roles holding `TRUNCATE` on +/// `lease_claim_batches`, never `PUBLIC`, however the migration role refreshes +/// it: as itself with ordinary membership in the owner, as itself with +/// SET-only membership (PostgreSQL 16+: it may `SET ROLE` to the owner but +/// does not inherit its privileges), and through the migrations that install +/// it with the owner as effective role, as the database-roles guide says. The +/// migration role holds `TRUNCATE` itself, so it keeps `EXECUTE` after handing +/// the function over, even when it no longer inherits the owner's. +#[tokio::test] +async fn set_only_schema_owner_membership_resets_claim_batch_job_index_execute() -> TestResult { + let cluster = Cluster::new(Schema::Current, false).await?; + let server_version: i32 = + sqlx::query_scalar("SELECT current_setting('server_version_num')::int") + .fetch_one(&cluster.owner) + .await?; + if server_version < 160_000 { + return cluster.teardown().await; + } + let expected = ClaimBatchIndexAcl { + owner: cluster.owner_name.clone(), + public: false, + runtime: true, + migrator: true, + }; + assert_eq!( + claim_batch_job_index_function_acl(&cluster).await?, + expected + ); + + cluster + .owner + .execute(sqlx::raw_sql(sql(format!( + "GRANT USAGE, CREATE ON SCHEMA awa TO {migrator}; \ + GRANT TRUNCATE ON awa.lease_claim_batches TO {migrator}; \ + GRANT EXECUTE ON FUNCTION awa.refresh_claim_batch_job_index_helper(text) TO {migrator}", + migrator = cluster.migrator_name + )))) + .await?; + let refresh = "SELECT awa.refresh_claim_batch_job_index_helper('awa')"; + + // Ordinary membership: the migration role holds the owner's privileges. + hand_claim_batch_job_index_to_migrator(&cluster).await?; + sqlx::query(refresh).execute(&cluster.migrator).await?; + assert_eq!( + claim_batch_job_index_function_acl(&cluster).await?, + expected, + "a refresh by a migration role that inherits the owner" + ); + + // SET-only membership. + hand_claim_batch_job_index_to_migrator(&cluster).await?; + cluster + .admin + .execute(sqlx::raw_sql(sql(format!( + "GRANT {owner} TO {migrator} WITH INHERIT FALSE; \ + GRANT {owner} TO {migrator} WITH SET TRUE", + owner = cluster.owner_name, + migrator = cluster.migrator_name + )))) + .await?; + let mut conn = cluster.migrator.acquire().await?; + let (inherits_owner, can_set_owner): (bool, bool) = sqlx::query_as( + "SELECT pg_has_role(current_user, $1, 'USAGE'), pg_has_role(current_user, $1, 'SET')", + ) + .bind(&cluster.owner_name) + .fetch_one(&mut *conn) + .await?; + assert!(!inherits_owner); + assert!(can_set_owner); + let role_query = "SELECT current_user::text, current_setting('role')"; + let role_before: (String, String) = sqlx::query_as(role_query).fetch_one(&mut *conn).await?; + sqlx::query(refresh).execute(&mut *conn).await?; + let role_after: (String, String) = sqlx::query_as(role_query).fetch_one(&mut *conn).await?; + assert_eq!( + role_after, role_before, + "the refresh restores the caller's role" + ); + drop(conn); + assert_eq!( + claim_batch_job_index_function_acl(&cluster).await?, + expected, + "a refresh by a migration role that may only SET ROLE to the owner" + ); + + // The migrations from v054 on, with the owner as the effective role. The + // migration preflight refuses a schema another role may create in. + cluster + .owner + .execute(sqlx::raw_sql(sql(format!( + "REVOKE CREATE ON SCHEMA awa FROM {migrator}; \ + DROP FUNCTION awa.ensure_claim_batch_job_index(integer); \ + DELETE FROM awa.schema_version WHERE version >= 54", + migrator = cluster.migrator_name + )))) + .await?; + let migrate_pool = cluster.migrator_as_owner().await?; + migrations::run(&migrate_pool).await?; + let role_after: (String, String) = sqlx::query_as(role_query).fetch_one(&migrate_pool).await?; + let version: i32 = sqlx::query_scalar("SELECT max(version) FROM awa.schema_version") + .fetch_one(&migrate_pool) + .await?; + migrate_pool.close().await; + assert_eq!( + role_after, + (cluster.owner_name.clone(), cluster.owner_name.clone()) + ); + assert_eq!(version, migrations::CURRENT_VERSION); + assert_eq!( + claim_batch_job_index_function_acl(&cluster).await?, + expected, + "the migrations applied by the migration role as the owner" + ); + cluster.teardown().await +} + /// Reassigning a lane sequence waits on any open enqueue or claim on the lane, /// so the repair bounds that wait and holds each sequence's lock only for its /// own short transaction. A held sequence is skipped and reported; the rest diff --git a/awa/tests/queue_storage_runtime_test.rs b/awa/tests/queue_storage_runtime_test.rs index 656c90753..89a974120 100644 --- a/awa/tests/queue_storage_runtime_test.rs +++ b/awa/tests/queue_storage_runtime_test.rs @@ -28352,9 +28352,121 @@ async fn test_claim_batch_job_index_is_built_only_on_empty_partitions() { assert_eq!(claim_batch_job_index_slots(&pool, schema).await, vec![0, 1]); } +/// `(index name, partial)` for every GIN index on one claim partition. +async fn claim_partition_gin_indexes( + pool: &sqlx::PgPool, + schema: &str, + slot: i32, +) -> Vec<(String, bool)> { + sqlx::query_as( + r#" + SELECT index_class.relname::text, index_meta.indpred IS NOT NULL + FROM pg_index AS index_meta + JOIN pg_class AS index_class ON index_class.oid = index_meta.indexrelid + JOIN pg_am AS method ON method.oid = index_class.relam + WHERE index_meta.indrelid = to_regclass(format('%I.lease_claim_batches_%s', $1::text, $2::int)) + AND method.amname = 'gin' + ORDER BY 1 + "#, + ) + .bind(schema) + .bind(slot) + .fetch_all(pool) + .await + .expect("Failed to list claim partition GIN indexes") +} + +/// Only a non-partial GIN index on `job_ids` counts as the claim batch index. +/// A partial one under another name does not stop the real index being +/// built; an index that already has the real index's name but another +/// definition is reported and left in place, never dropped or mistaken for +/// the real one. The check does not depend on the caller's +/// `quote_all_identifiers`. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_claim_batch_job_index_requires_a_full_job_ids_index() { + let (_db_guard, pool) = setup_pool(4).await; + let schema = "awa_qs_cbji_shape"; + let _store = create_store_with_config( + &pool, + QueueStorageConfig { + schema: schema.to_string(), + queue_slot_count: 4, + lease_slot_count: 2, + queue_stripe_count: 1, + lease_claim_receipts: true, + claim_slot_count: 2, + }, + ) + .await; + drop_claim_batch_job_index(&pool, schema, 0).await; + drop_claim_batch_job_index(&pool, schema, 1).await; + + sqlx::raw_sql(audited_sql(format!( + "CREATE INDEX lease_claim_batches_0_job_ids_idx \ + ON {schema}.lease_claim_batches_0 USING GIN (job_ids) \ + WHERE cardinality(job_ids) > 1; \ + CREATE INDEX operator_partial_job_ids \ + ON {schema}.lease_claim_batches_1 USING GIN (job_ids) \ + WHERE cardinality(job_ids) > 1" + ))) + .execute(&pool) + .await + .expect("create partial job_ids indexes"); + + assert!( + !ensure_claim_batch_job_index(&pool, schema, 0).await, + "a partial index under the real index's name is not the real index" + ); + assert_eq!( + claim_partition_gin_indexes(&pool, schema, 0).await, + vec![("lease_claim_batches_0_job_ids_idx".to_string(), true)], + "the conflicting index is left in place" + ); + + assert!(ensure_claim_batch_job_index(&pool, schema, 1).await); + assert_eq!( + claim_partition_gin_indexes(&pool, schema, 1).await, + vec![ + ("lease_claim_batches_1_job_ids_idx".to_string(), false), + ("operator_partial_job_ids".to_string(), true), + ], + "a partial index under another name does not stand in for the real one" + ); + + // With the real index present the check returns before it would try + // to lock the partition, so a busy partition still reports indexed, even + // for a caller that quotes every identifier. + let mut writer = pool.begin().await.unwrap(); + sqlx::query(audited_sql(format!( + "LOCK TABLE {schema}.lease_claim_batches_1 IN ROW EXCLUSIVE MODE" + ))) + .execute(&mut *writer) + .await + .unwrap(); + let mut caller = pool.acquire().await.unwrap(); + sqlx::query("SET quote_all_identifiers = on") + .execute(&mut *caller) + .await + .unwrap(); + let indexed = tokio::time::timeout( + ci_timing::scaled_timeout(Duration::from_secs(5)), + ensure_claim_batch_job_index(&mut *caller, schema, 1), + ) + .await + .expect("ensure_claim_batch_job_index must not wait for a busy partition"); + sqlx::query("RESET quote_all_identifiers") + .execute(&mut *caller) + .await + .unwrap(); + drop(caller); + writer.rollback().await.unwrap(); + assert!(indexed, "the existing job_ids index must be recognised"); +} + /// v054 is idempotent, installs the index function without building any /// index, and claim prune then indexes each slot it truncates. A schema -/// without the function keeps working with unindexed lookups. +/// without the function keeps working with unindexed lookups. v056 replaces +/// an index function that predates it. #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_claim_batch_job_index_migration_leaves_indexing_to_claim_prune() { let (_db_guard, pool) = setup_pool(4).await; @@ -28513,6 +28625,47 @@ async fn test_claim_batch_job_index_migration_leaves_indexing_to_claim_prune() { ); assert_eq!(schema_versions().await, versions_before); + // An index function from before v056 is replaced by it. + sqlx::raw_sql(audited_sql(format!( + "CREATE OR REPLACE FUNCTION {schema}.ensure_claim_batch_job_index(p_slot INTEGER) \ + RETURNS BOOLEAN LANGUAGE plpgsql SECURITY DEFINER \ + SET search_path = pg_catalog, pg_temp \ + AS $$ BEGIN RETURN TRUE; END $$; \ + DELETE FROM awa.schema_version WHERE version >= 56" + ))) + .execute(&pool) + .await + .expect("install an earlier index function"); + migrations::run(&pool).await.expect("apply v056"); + let requires_full_index = || async { + sqlx::query_scalar::<_, bool>( + "SELECT routine.prosrc LIKE '%indpred IS NULL%' \ + AND 'quote_all_identifiers=off' = ANY (routine.proconfig) \ + FROM pg_proc AS routine \ + WHERE routine.oid = to_regprocedure($1)", + ) + .bind(format!("{schema}.ensure_claim_batch_job_index(integer)")) + .fetch_one(&pool) + .await + .unwrap() + }; + assert!(requires_full_index().await, "v056 replaces the function"); + assert_eq!(schema_versions().await, versions_before); + let (_, _, v056_sql) = migrations::migration_sql_range(55, 56) + .pop() + .expect("v056 is a migration"); + sqlx::raw_sql(audited_sql(v056_sql)) + .execute(&pool) + .await + .expect("v056 re-applies cleanly"); + assert!(requires_full_index().await); + assert_eq!( + claim_batch_job_index_slots(&pool, schema).await, + Vec::::new(), + "v056 builds no index" + ); + assert_eq!(schema_versions().await, versions_before); + let job = store .load_job(&pool, second[0].job.id) .await diff --git a/correctness/README.md b/correctness/README.md index 8a2059bf0..9d6d64c15 100644 --- a/correctness/README.md +++ b/correctness/README.md @@ -52,7 +52,7 @@ What is intentionally not modeled: - `storage/AwaSegmentedStorageRaces.tla` / `storage/AwaSegmentedStorageRaces.cfg` / `storage/AwaSegmentedStorageRacesSafe.cfg`: focused race-exposure spec that splits `Claim` into `BeginClaim` / `CommitClaim` to model the claim path's cursor-read-without-lock behaviour. The race config produces a counterexample trace (claim snapshots segment, rotate+prune fire, commit lands lease in pruned segment — simultaneously the claim-vs-rotate race and the prune check-then-act race). The safe config uses a checked commit and passes. Production uses CAS on ring state plus child-partition locks and busy checks, not `FOR SHARE` on `lease_ring_state` — see `storage/MAPPING.md` for the full analysis. - `storage/AwaShardedPrune.tla` / `storage/AwaShardedPrune.cfg` / `storage/AwaShardedPruneBroken.cfg`: focused ADR-025 regression model for queue-ring prune across enqueue shards. The passing config requires ready rows to match done rows by `(enqueue_shard, lane_seq)` before prune can drop a sealed queue slot. The broken config intentionally ignores `enqueue_shard` and produces the historical counterexample: shard 0 has a done row at `lane_seq = 1`, shard 1 still has a pending ready row at `lane_seq = 1`, and broken prune drops both. - `storage/AwaPartitionedQueueRouting.tla` / `storage/AwaPartitionedQueueRouting.cfg` / `storage/AwaPartitionedQueueRoutingBroken.cfg`: focused ADR-031 routing model for `PartitionedQueue`. Partitioned queues remain client-side routing over ordinary Awa queues, so this spec checks the refinement boundary above storage: abstract key hashes route to their selected partition, the same base hash still determines the enqueue shard inside that partition, lane sequence identity stays scoped to `(partition, shard)`, and the configured hash domain reaches every shard from every partition. The broken config reuses the base hash modulo at both routing levels and demonstrates the correlated-hash defect. -- `storage/AwaStorageLockOrder.tla` / `storage/AwaStorageLockOrder.cfg` / `storage/AwaStorageLockOrderDeadlockDemo.cfg` / `storage/AwaStorageLockOrderOldStripedClaimDeadlock.cfg` / `storage/AwaStorageLockOrderPrepareQueue.cfg` / `storage/AwaStorageLockOrderPrepareQueueNumeric.cfg`: lock-ordering protocol spec. Models each storage-engine transaction (enqueue, prepare queue, claim, complete, close-receipt, the stale-heartbeat and deadline rescue transactions including their receipt sweep and the exhausted attempt's terminal insert, ensure-running, cancel, rotate, and prune) as an ordered sequence of Postgres lock acquisitions with a shared/exclusive compatibility matrix. The striped enqueue and queue preparation paths take multiple physical queue lanes in the same string order; the current striped runtime claim path takes at most one physical stripe per transaction. Checks `NoDeadlock` via a waits-for cycle detector. The numeric-order preparation witness uses `q#2` and `q#10` to expose the order inversion; the other demo configs check the detector and historical striped claim plan. +- `storage/AwaStorageLockOrder.tla` / `storage/AwaStorageLockOrder.cfg` / `storage/AwaStorageLockOrderDeadlockDemo.cfg` / `storage/AwaStorageLockOrderOldStripedClaimDeadlock.cfg` / `storage/AwaStorageLockOrderOldLaneProvisioningDeadlock.cfg` / `storage/AwaStorageLockOrderPrepareQueue.cfg` / `storage/AwaStorageLockOrderPrepareQueueNumeric.cfg`: lock-ordering protocol spec. Models each storage-engine transaction (enqueue, first enqueue to a new lane, prepare queue, claim, complete, close-receipt, the stale-heartbeat and deadline rescue transactions including their receipt sweep and the exhausted attempt's terminal insert, ensure-running, cancel, rotate, and prune) as an ordered sequence of Postgres lock acquisitions with a shared/exclusive compatibility matrix. The striped enqueue and queue preparation paths take multiple physical queue lanes in the same string order; the current striped runtime claim path takes at most one physical stripe per transaction. Checks `NoDeadlock` via a waits-for cycle detector. The numeric-order preparation witness uses `q#2` and `q#10` to expose the order inversion; the other demo configs check the detector, the historical striped claim plan, and the historical `QueueStorage` lane-provisioning order (provisioning lock before the `queue_lanes` row). The first enqueue to a new lane takes its `queue_lanes` row, then its provisioning lock, then its enqueue lock, in every producer. - `storage/AwaUniqueClaimLockOrder.tla` and its six configs: focused row-level lock-order checks for unique claims before enqueue-lane reservations, sorted multi-key claim acquisition, and queue-ring state before terminal-table locks in counter rebuild and reset. Each safe plan has a matching old-order deadlock witness. - `storage/AwaRotationCadence.tla`: observed fill-age and physical-storage scheduling, with eventual drain under stable leadership and fair ticks/prune opportunities. Negative configurations demonstrate allocated empty heaps, old age clocks, and sampled-target overshoot. - `storage/AwaCanonicalUniqueRescue.tla` / `storage/AwaCanonicalUniqueRescue.cfg` / `storage/AwaCanonicalUniqueRescueBatchOnly.cfg`: focused model for the canonical rescue sweeps versus the unique-claim trigger. A unique job with a pending-only `unique_states` mask is stuck running while a duplicate holds the claim; the batched rescue is blocked forever (BatchOnly config: expected `Convergence` liveness counterexample — the production wedge fixed in 0.6.1), while the per-row fallback rescues the innocent job and cancels the superseded one (main config passes, `ClaimConsistency` holds in both). @@ -117,6 +117,7 @@ Expected counterexample or positive-witness configs: ./correctness/run-tlc.sh storage/AwaPartitionedQueueRouting.tla storage/AwaPartitionedQueueRoutingBroken.cfg ./correctness/run-tlc.sh storage/AwaStorageLockOrder.tla storage/AwaStorageLockOrderDeadlockDemo.cfg ./correctness/run-tlc.sh storage/AwaStorageLockOrder.tla storage/AwaStorageLockOrderOldStripedClaimDeadlock.cfg +./correctness/run-tlc.sh storage/AwaStorageLockOrder.tla storage/AwaStorageLockOrderOldLaneProvisioningDeadlock.cfg ./correctness/run-tlc.sh storage/AwaCanonicalDirtyMarks.tla storage/AwaCanonicalDirtyMarksOnConflict.cfg ./correctness/run-tlc.sh storage/AwaCanonicalDirtyMarks.tla storage/AwaCanonicalDirtyMarksIncident.cfg ./correctness/run-tlc.sh storage/AwaStorageTransition.tla storage/AwaStorageTransitionCurrentGate.cfg diff --git a/correctness/run-tlc-suite.sh b/correctness/run-tlc-suite.sh index f21b09e3f..f09ba974c 100755 --- a/correctness/run-tlc-suite.sh +++ b/correctness/run-tlc-suite.sh @@ -67,6 +67,8 @@ AwaRotationCadence.tla ;; AwaRotationCadenceHardCap.cfg ;; Targe AwaStorageLockOrder.tla ;; AwaStorageLockOrderPrepareQueue.cfg ;; pass ;; prepared lanes and producer lock order AwaStorageLockOrder.tla ;; AwaStorageLockOrderPrepareQueueNumeric.cfg ;; NoDeadlock is violated ;; numeric stripe order inversion witness AwaStorageLockOrder.tla ;; AwaStorageLockOrderDeadlockDemo.cfg ;; NoDeadlock is violated ;; deadlock detector witness +AwaStorageLockOrder.tla ;; AwaStorageLockOrderOldStripedClaimDeadlock.cfg ;; NoDeadlock is violated ;; old striped-claim order witness +AwaStorageLockOrder.tla ;; AwaStorageLockOrderOldLaneProvisioningDeadlock.cfg ;; NoDeadlock is violated ;; old QueueStorage lane-provisioning order witness AwaUniqueClaimLockOrder.tla ;; AwaUniqueClaimLockOrder.cfg ;; pass ;; unique claim before enqueue lane AwaUniqueClaimLockOrder.tla ;; AwaUniqueClaimLockOrderBroken.cfg ;; NoDeadlock is violated ;; old retry lane-before-claim witness AwaUniqueClaimLockOrder.tla ;; AwaUniqueClaimMultiKey.cfg ;; pass ;; unique claims in key order diff --git a/correctness/storage/AwaStorageLockOrder.tla b/correctness/storage/AwaStorageLockOrder.tla index d315728ab..843e8b677 100644 --- a/correctness/storage/AwaStorageLockOrder.tla +++ b/correctness/storage/AwaStorageLockOrder.tla @@ -129,6 +129,15 @@ Compatible(held, wanted) == held = ModeShared /\ wanted = ModeShared \* deadlock; two transactions on the same lane and shard reduce to \* the existing `LaneResource(q, p)` model. LaneResource(q, p) == [k |-> "queue_lane", q |-> q, p |-> p] +\* Creating a new lane takes two more locks before the enqueue lock. The +\* `queue_lanes` row: an uncommitted insert of a `(queue, priority)` key +\* makes a second inserter of that key wait for its transaction. The +\* provisioning advisory lock `{schema}:{queue}:{priority}:{shard}`, under +\* which the lane's sequences and head rows are created. Once a lane +\* exists neither blocks, so modelling both as held to commit +\* over-approximates every enqueue after the first. +LaneRowResource(q, p) == [k |-> "queue_lane_row", q |-> q, p |-> p] +LaneProvisionResource(q, p) == [k |-> "queue_lane_provision", q |-> q, p |-> p] \* #371 staged upgrade: in compat authority the rotate/prune/rollup paths \* serialize on `{ring}_ring_state WHERE singleton FOR UPDATE` (a single-row \* lock target -> ModeExclusive, restoring the pre-#415 discipline) instead @@ -166,6 +175,8 @@ ClosureChildResource(s) == [k |-> "closure_child", s |-> s] ClosureBatchChildResource(s) == [k |-> "closure_batch_child", s |-> s] LaneResources == { LaneResource(q, p) : q \in Queues, p \in Priorities } +LaneRowResources == { LaneRowResource(q, p) : q \in Queues, p \in Priorities } +LaneProvisionResources == { LaneProvisionResource(q, p) : q \in Queues, p \in Priorities } LeaseRingSlotResources == { LeaseRingSlotResource(s) : s \in LeaseSlots } LeaseChildResources == { LeaseChildResource(s) : s \in LeaseSlots } QueueRingSlotResources == { QueueRingSlotResource(s) : s \in ReadySlots } @@ -185,6 +196,8 @@ ClosureBatchChildResources == { ClosureBatchChildResource(s) : s \in ClaimSlots Resources == LaneResources \cup + LaneRowResources \cup + LaneProvisionResources \cup {QueueRingStateResource} \cup {LeaseRingStateResource} \cup {ClaimRingStateResource} \cup @@ -255,6 +268,31 @@ PrepareNumericStripePlan(p) == << Step(LaneResource(StripeB, p), ModeExclusive), Step(LaneResource(StripeA, p), ModeExclusive) >> +\* First enqueue to a `(queue, priority)` lane, by any producer: +\* SQL inserts (`awa.insert_job_compat`, `insert_jobs_compat_batch`): +\* INSERT INTO queue_lanes ... ON CONFLICT DO NOTHING +\* INSERT INTO queue_enqueue_heads, whose BEFORE INSERT trigger takes +\* the provisioning lock to create the lane's sequences +\* reserve_enqueue_seq takes the lane's enqueue lock +\* QueueStorage::ensure_lane_inserts (batch enqueue, promotion of +\* scheduled or retried jobs, prepare_queue): +\* INSERT INTO queue_lanes ... ON CONFLICT DO NOTHING +\* pg_advisory_xact_lock on the provisioning key, then provision_lane +\* or the head-row inserts +\* reserve_enqueue_seq takes the lane's enqueue lock +ProvisionLanePlan(q, p) == + << Step(LaneRowResource(q, p), ModeExclusive), + Step(LaneProvisionResource(q, p), ModeExclusive), + Step(LaneResource(q, p), ModeExclusive) >> + +\* QueueStorage::ensure_lane_inserts once took the provisioning lock +\* before writing the `queue_lanes` row, the reverse of the SQL inserts. +\* Kept only as a negative regression harness. +OldQueueStorageProvisionLanePlan(q, p) == + << Step(LaneProvisionResource(q, p), ModeExclusive), + Step(LaneRowResource(q, p), ModeExclusive), + Step(LaneResource(q, p), ModeExclusive) >> + \* The claim CTE \* (`claim_runtime_batch_with_aging_for_instance` in queue_storage.rs) \* FOR UPDATE OF queue_claim_heads SKIP LOCKED (per-(queue,priority) row) @@ -827,6 +865,16 @@ StartPrepareNumericStripe(t, p) == /\ txNextStep' = [txNextStep EXCEPT ![t] = 1] /\ UNCHANGED <> +StartProvisionLane(t, q, p) == + /\ t \in TxIds + /\ txState[t] = "idle" + /\ q \in Queues + /\ p \in Priorities + /\ txState' = [txState EXCEPT ![t] = "running"] + /\ txPlan' = [txPlan EXCEPT ![t] = ProvisionLanePlan(q, p)] + /\ txNextStep' = [txNextStep EXCEPT ![t] = 1] + /\ UNCHANGED <> + StartComplete(t, claimSlot, readySlot) == /\ t \in TxIds /\ txState[t] = "idle" @@ -1086,6 +1134,7 @@ Stutter == /\ UNCHANGED vars Next == \/ \E t \in TxIds, p \in Priorities : StartEnqueueTwoStripe(t, p) \/ \E t \in TxIds, p \in Priorities : StartPrepareTwoStripe(t, p) + \/ \E t \in TxIds, q \in Queues, p \in Priorities : StartProvisionLane(t, q, p) \/ \E t \in TxIds, q \in Queues, p \in Priorities, rs \in ReadySlots, cs \in ClaimSlots : StartClaimReceipts(t, q, p, rs, cs) @@ -1227,6 +1276,37 @@ NextOldStripedClaimDeadlockDemo == SpecOldStripedClaimDeadlockDemo == ConfigOK /\ Init /\ [][NextOldStripedClaimDeadlockDemo]_vars +\* ---- Historical lane-provisioning deadlock harness ---- +\* +\* A QueueStorage process with the earlier order and a SQL producer that +\* are both first to use a lane: QueueStorage holds the provisioning lock +\* and waits for the SQL producer's uncommitted `queue_lanes` row, while +\* the SQL producer holds that row and waits for the provisioning lock. +\* TLC is expected to flag NoDeadlock on this spec. + +StartOldQueueStorageProvisionLane(t, q, p) == + /\ t \in TxIds + /\ txState[t] = "idle" + /\ q \in Queues + /\ p \in Priorities + /\ txState' = [txState EXCEPT ![t] = "running"] + /\ txPlan' = [txPlan EXCEPT ![t] = OldQueueStorageProvisionLanePlan(q, p)] + /\ txNextStep' = [txNextStep EXCEPT ![t] = 1] + /\ UNCHANGED <> + +NextOldLaneProvisioningDeadlockDemo == + \/ \E t \in TxIds, q \in Queues, p \in Priorities : StartProvisionLane(t, q, p) + \/ \E t \in TxIds, q \in Queues, p \in Priorities : + StartOldQueueStorageProvisionLane(t, q, p) + \/ \E t \in TxIds : AbortUnavailable(t) + \/ \E t \in TxIds : AcquireNext(t) + \/ \E t \in TxIds : Commit(t) + \/ \E t \in TxIds : Recycle(t) + \/ Stutter + +SpecOldLaneProvisioningDeadlockDemo == + ConfigOK /\ Init /\ [][NextOldLaneProvisioningDeadlockDemo]_vars + \* ---- Invariants ---- TypeOK == diff --git a/correctness/storage/AwaStorageLockOrderOldLaneProvisioningDeadlock.cfg b/correctness/storage/AwaStorageLockOrderOldLaneProvisioningDeadlock.cfg new file mode 100644 index 000000000..4e42d74fc --- /dev/null +++ b/correctness/storage/AwaStorageLockOrderOldLaneProvisioningDeadlock.cfg @@ -0,0 +1,21 @@ +\* Historical lane-provisioning harness. QueueStorage once took a new +\* lane's provisioning lock before its queue_lanes row, while SQL inserts +\* write the row first. TLC is expected to flag NoDeadlock when one of +\* each is the first to use the same lane. +CONSTANTS + TxIds = {"t1", "t2"} + Queues = {"q1#0", "q1#1"} + StripeA = "q1#0" + StripeB = "q1#1" + Priorities = {1} + LeaseSlots = {1} + ReadySlots = {1} + ClaimSlots = {1} + +SPECIFICATION SpecOldLaneProvisioningDeadlockDemo + +INVARIANTS + TypeOK + LockCompatibility + HeldOnlyByRunningTxs + NoDeadlock diff --git a/correctness/storage/MAPPING.md b/correctness/storage/MAPPING.md index 1bc093e7d..4bca2fb84 100644 --- a/correctness/storage/MAPPING.md +++ b/correctness/storage/MAPPING.md @@ -154,6 +154,8 @@ The TLA+ storage model treats `EnqueueReady` and `EnqueueDeferred` as logical pe Uniqueness itself is intentionally outside this storage model: duplicate rejection is covered by Rust integration tests around `job_unique_claims`. The model's enqueue preconditions start after a job has been admitted to the storage state, so batching uniqueness claims changes implementation granularity rather than the modeled lifecycle, lane, lease, or prune invariants. +`awa.insert_jobs_compat_batch` takes a batch's unique claims before its lanes and, for a bounded phase, gives way to a per-row producer by limiting each lock wait and rolling back to retry. `AwaUniqueClaimLockOrder` checks the claim-before-lane and key orders, and `AwaStorageLockOrder` deliberately excludes lock timeouts and aborts, so the give-way protocol is covered by `awa/tests/insert_many_batch_test.rs` rather than TLC. + ## Partitioned queue routing note `PartitionedQueue` lives in `awa-model/src/partitioned_queue.rs`, is re-exported by `awa` and `awa-worker`, and is wrapped for Python as `awa.PartitionedQueue`. It maps one logical queue to a caller-declared list of ordinary physical queue names. The worker builder expands that list into ordinary `QueueConfig` declarations; producer helpers stamp the selected physical queue onto `InsertOpts`. @@ -232,6 +234,8 @@ The spec plus `AwaSegmentedStorageRaces.cfg` (race-exposing) and `AwaSegmentedSt `AwaStorageLockOrder.tla` (see [`README.md`](./README.md)) is the complementary positive artifact: it models the Postgres locks directly and checks that no interleaving of prepare-queue / enqueue / claim / complete / close-receipt / rescue-receipts / ensure-running / cancel / rotate / prune transactions produces a waits-for cycle. The producer and preparer both take multiple physical queue lane locks in string order; the current runtime claim path claims only one physical stripe per transaction. `AwaStorageLockOrderPrepareQueue.cfg` checks the `q#10` before `q#2` ordering, and `AwaStorageLockOrderPrepareQueueNumeric.cfg` demonstrates the cycle caused by numeric preparation order. The other demo configs check the detector and the historical unsafe logical-claim shape. +`ProvisionLanePlan` models the first enqueue to a new `(queue, priority)` lane, which every producer takes in one order: the `queue_lanes` row (SQL inserts in `awa.insert_job_compat` and `insert_jobs_compat_batch`; `QueueStorage::ensure_lane_inserts` for batch enqueue, promotion and `prepare_queue`; `.provision_lane` from v058), then the provisioning advisory lock (the head-table trigger, or `ensure_lane_inserts` before `provision_lane`), then the lane's enqueue lock. `AwaStorageLockOrderOldLaneProvisioningDeadlock.cfg` keeps the earlier `QueueStorage` order, provisioning lock before the row, and must violate `NoDeadlock` against a SQL producer of the same new lane (#692). + Together the two specs cover complementary risks: - `AwaSegmentedStorageRaces` catches data-level races that would occur if the locks were removed — proves the locks are necessary @@ -320,8 +324,9 @@ Model checking results: - `AwaSegmentedStorageRacesMultiWorker.cfg`: safe commit with 3 workers, 332 distinct states, clean. - `AwaShardedPrune.cfg`: 1,028 distinct states, clean. This checks the fixed queue-ring prune predicate that matches ready rows to terminal rows by `(enqueue_shard, lane_seq)`. - `AwaShardedPruneBroken.cfg`: expected failure. It ignores `enqueue_shard` in the ready/terminal match and produces the counterexample where shard 0's completed `lane_seq = 1` row masks shard 1's still-pending `lane_seq = 1` row. -- `AwaStorageLockOrder.cfg`: 153,515 distinct states, clean. Models the receipts and legacy claim modes separately, plus `CloseReceiptPlan`, `RescueReceiptsPlan`, `EnsureRunningPlan`, `CancelReceiptOnlyPlan`, and `CancelRunningPlan`. +- `AwaStorageLockOrder.cfg`: 2,629,707 distinct states, clean. Models the receipts and legacy claim modes separately, plus `CloseReceiptPlan`, `RescueReceiptsPlan`, `EnsureRunningPlan`, `CancelReceiptOnlyPlan`, `CancelRunningPlan`, and `ProvisionLanePlan`. - `AwaStorageLockOrderDeadlockDemo.cfg`: trips `NoDeadlock` in 5 steps, confirming the detector works. +- `AwaStorageLockOrderOldLaneProvisioningDeadlock.cfg`: trips `NoDeadlock` in 5 steps, one transaction holding the new lane's `queue_lanes` row and the other its provisioning lock. - `AwaDeadTupleContract.cfg`: 1 distinct state, clean. TLC generated 2 states because the contract is an ASSUME-style architectural check. The checks (`HotTablesAreNotRowVacuum`, `PartitionTruncateTablesAreReclaimed`, `WarmTablesDocumentTheirBound`, `BacklogRowVacuumTablesDocumentTheirBound`, `OnlyBoundedKindsHaveBoundedBy`, `AppendOnlyAcceptsOnlyInsert`, `VacuumKindTablesNotTruncated`) all hold for the current schema and transaction list. Workflow: when adding a new table to `prepare_schema()` or a new SQL site to queue_storage.rs, register it in `AwaDeadTupleContract.tla` with the correct reclaim kind, hotness, optional `bounded_by`, and mutation list. An `open_receipt_claims`-style proposal (hot table, RowVacuum reclaim, INSERT+DELETE traffic) fires `HotTablesAreNotRowVacuum` at parse time; a `Warm` table without a declared `bounded_by` fires `WarmTablesDocumentTheirBound`. ### Receipt-plane shape diff --git a/correctness/storage/README.md b/correctness/storage/README.md index f0f6325c5..9f574bc0b 100644 --- a/correctness/storage/README.md +++ b/correctness/storage/README.md @@ -157,7 +157,7 @@ Three negative configurations reproduce using allocated bytes as row presence, u Configs: -- [`AwaStorageLockOrder.cfg`](./AwaStorageLockOrder.cfg): main run against the real Rust lock plans. Models claim (receipts and legacy modes), complete, close-receipt, the whole stale-heartbeat and deadline rescue transactions (lease delete, receipt sweep, closing the rescued attempt's own receipt, and the `done_entries` insert when the attempt was the last), ensure-running, terminal-delete closure materialization, the two cancel branches, plus rotate / prune for the queue, lease, and claim rings. It checks the represented lock plans; implicit resources absent from a plan are outside the proof. +- [`AwaStorageLockOrder.cfg`](./AwaStorageLockOrder.cfg): main run against the real Rust lock plans. Models enqueue, the first enqueue to a new lane (`queue_lanes` row, provisioning lock, enqueue lock), claim (receipts and legacy modes), complete, close-receipt, the whole stale-heartbeat and deadline rescue transactions (lease delete, receipt sweep, closing the rescued attempt's own receipt, and the `done_entries` insert when the attempt was the last), ensure-running, terminal-delete closure materialization, the two cancel branches, plus rotate / prune for the queue, lease, and claim rings. It checks the represented lock plans; implicit resources absent from a plan are outside the proof. - [`AwaQueuePruneLockOrder.cfg`](./AwaQueuePruneLockOrder.cfg): segment-first claim readers and entry-first producers versus queue pruning. Prune waits on routing first and aborts the entire transaction if any later `NOWAIT` lock conflicts. Both authority modes use that tail. This proves cycle avoidance at the represented lock level, not successful pruning under perpetual contention. - [`AwaQueuePruneLockOrderBlocking.cfg`](./AwaQueuePruneLockOrderBlocking.cfg): old body-first blocking prune versus the segment-first reader; TLC must find the cycle captured during the saturation benchmark. The main claim plans now include the previously omitted routing-child read. - [`AwaWholeSchemaLockOrder.cfg`](./AwaWholeSchemaLockOrder.cfg): counter rebuild and reset versus every ring's rotate, prune and rollup, across the authority flip. Both whole-schema operations wait for the per-ring rotation locks before the ring-state singletons and their table locks. A ledger tick holds only the rotation lock before its child locks, and a tick promoted to ledger while queued on the singleton try-locks it. @@ -165,6 +165,8 @@ Configs: - [`AwaStorageLockOrderPrepareQueue.cfg`](./AwaStorageLockOrderPrepareQueue.cfg): producer and preparer both lock `q#10` before `q#2`, matching the string order of the batched Rust producer path. - [`AwaStorageLockOrderPrepareQueueNumeric.cfg`](./AwaStorageLockOrderPrepareQueueNumeric.cfg): numeric preparation order locks `q#2` before `q#10`; TLC finds a waits-for cycle against a producer. - [`AwaStorageLockOrderDeadlockDemo.cfg`](./AwaStorageLockOrderDeadlockDemo.cfg): sanity harness using a deliberately cycle-creating pair of plans — **NoDeadlock tripped in 5 steps** (confirms the checker works). +- [`AwaStorageLockOrderOldStripedClaimDeadlock.cfg`](./AwaStorageLockOrderOldStripedClaimDeadlock.cfg): the historical claim shape that walked several physical stripes against the enqueue order — **NoDeadlock tripped**. +- [`AwaStorageLockOrderOldLaneProvisioningDeadlock.cfg`](./AwaStorageLockOrderOldLaneProvisioningDeadlock.cfg): the historical `QueueStorage` order for a new lane, provisioning lock before the `queue_lanes` row, against a SQL producer of the same lane — **NoDeadlock tripped in 5 steps** (#692). Run: @@ -173,6 +175,8 @@ Run: ./correctness/run-tlc.sh storage/AwaStorageLockOrder.tla storage/AwaStorageLockOrderPrepareQueue.cfg ./correctness/run-tlc.sh storage/AwaStorageLockOrder.tla storage/AwaStorageLockOrderPrepareQueueNumeric.cfg ./correctness/run-tlc.sh storage/AwaStorageLockOrder.tla storage/AwaStorageLockOrderDeadlockDemo.cfg +./correctness/run-tlc.sh storage/AwaStorageLockOrder.tla storage/AwaStorageLockOrderOldStripedClaimDeadlock.cfg +./correctness/run-tlc.sh storage/AwaStorageLockOrder.tla storage/AwaStorageLockOrderOldLaneProvisioningDeadlock.cfg ``` Coverage note: the plans model the lock steps that actually appear in the Rust SQL (`FOR UPDATE` / `FOR SHARE` / `LOCK TABLE ACCESS EXCLUSIVE` / the implicit AccessShare of SELECT on partition children). They do NOT model implicit table-level locks beyond what is named, or Postgres's lock-timeout / deadlock-detector abort choice. The spec treats a waits-for cycle as a safety violation, which is conservative — Postgres would abort one transaction and let the other proceed. For our purposes "this sequence of lock requests could produce a cycle" is the thing we want to catch, regardless of how the runtime resolves it. diff --git a/docs/security/database-roles.md b/docs/security/database-roles.md index e6673ade6..96d238c53 100644 --- a/docs/security/database-roles.md +++ b/docs/security/database-roles.md @@ -126,7 +126,7 @@ FROM awa.lane_sequence_problems('awa') \gexec ## Claim batch index -From schema v054 each queue-storage schema has `ensure_claim_batch_job_index(slot)`. Claim prune calls it right after truncating a claim slot to give that `lease_claim_batches` partition its GIN index on `job_ids`. It is `SECURITY DEFINER`, owned by the owner of `lease_claim_batches` (creating an index needs table ownership), with a pinned `search_path`. It only creates that one index, and only on a partition that is empty and unlocked. `PUBLIC` cannot execute it. Each time the migration or installer runs, `EXECUTE` is reset to exactly the roles that then hold `TRUNCATE` on `lease_claim_batches`: a runtime role given `TRUNCATE` later gets it on the next run (or from `GRANT EXECUTE ON ALL FUNCTIONS IN SCHEMA` above until then), and a role that has lost `TRUNCATE` loses it. A runtime that cannot execute it skips the step and claims keep working. +From schema v054 each queue-storage schema has `ensure_claim_batch_job_index(slot)`. Claim prune calls it right after truncating a claim slot to give that `lease_claim_batches` partition its GIN index on `job_ids`. It is `SECURITY DEFINER`, owned by the owner of `lease_claim_batches` (creating an index needs table ownership), with a pinned `search_path`. It only creates that one index, and only on a partition that is empty and unlocked. From v056 only a valid, non-partial GIN index on `job_ids` counts; a different index or relation already named `lease_claim_batches__job_ids_idx` is left in place with a warning naming it, and that slot keeps scanning until it is dropped or renamed. `PUBLIC` cannot execute it. Each time the migration or installer runs, `EXECUTE` is reset to exactly the roles that then hold `TRUNCATE` on `lease_claim_batches`: a runtime role given `TRUNCATE` later gets it on the next run (or from `GRANT EXECUTE ON ALL FUNCTIONS IN SCHEMA` above until then), and a role that has lost `TRUNCATE` loses it. A runtime that cannot execute it skips the step and claims keep working. ## Custom queue-storage schemas diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 0b9fb35d7..4a7a3dbdd 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -234,7 +234,7 @@ Use an upsert: first-enqueue may create lane rows before any operator inserts a Many connections wait with `wait_event = 'advisory'` while the database is otherwise idle, batch inserts take seconds although the work per batch is milliseconds, and promotion into the same queue runs late while producers saturate it. -On queue storage every enqueue holds a transaction-scoped advisory lock on its `(queue, priority, shard)` lane until it commits, so concurrent producers for one lane run one at a time; a lane's ceiling is a batch's rows divided by the time the lock is held. Generic `insert_many` and per-row COPY take the lock at the batch's first ready row and hold it while the rest of the batch is processed. From migration v055, `insert_many_copy` and `insert_many_skip_unique_conflicts` prepare the whole batch (job ids, unique claims, payloads) before they take any lane lock, and hold it only for the lane write and the commit. +On queue storage every enqueue holds a transaction-scoped advisory lock on its `(queue, priority, shard)` lane until it commits, so concurrent producers for one lane run one at a time; a lane's ceiling is a batch's rows divided by the time the lock is held. Generic `insert_many` and per-row COPY take the lock at the batch's first ready row and hold it while the rest of the batch is processed. From migration v055, `insert_many_copy` and `insert_many_skip_unique_conflicts` prepare the whole batch (job ids, unique claims, payloads) before they take any lane lock, and take each lane's lock when they write that lane; it is still held until the transaction ends. - Run `awa migrate` and move hot producers to `insert_many_copy` or `insert_many_skip_unique_conflicts`. - When `insert_many` fails on a unique conflict, retry with `insert_many_skip_unique_conflicts` rather than inserting the batch again row by row: each single-row insert queues for the lane separately. @@ -492,10 +492,11 @@ awa --database-url "$DATABASE_URL" migrate Cause: - on queue storage every enqueue holds a transaction-scoped advisory lock on its `(queue, priority, shard)` lane until the transaction commits; the `DETAIL` names two 64-bit advisory keys, and two transactions that took lanes in opposite orders hold one each and wait on the other's -- `insert_many`, `insert_many_copy`, `insert_many_skip_unique_conflicts` and the `QueueStorage` batch enqueues register and lock their lanes one at a time in one canonical order, `(queue, priority, shard)` ascending with queue names compared bytewise, so two batch calls cannot hold each other's lanes in opposite orders. One exception remains when a `QueueStorage` batch enqueue and an `insert_many` / `insert_many_copy` call both create the same new `(queue, priority)` lane: the two paths take that lane's provisioning lock and its `queue_lanes` row in opposite orders and can deadlock (#692); retry the transaction -- from migration v055, `insert_many_copy` and `insert_many_skip_unique_conflicts` take all of a batch's unique claims before any lane lock, while per-row inserts (generic `insert_many`, single-job inserts, and binaries that predate the batch function) take each job's claim and lane together; a per-row transaction that holds a lane the batch needs and then inserts a job whose unique key the batch has claimed but not committed deadlocks with it, where it used to skip the job or fail with `23505` +- `insert_many`, `insert_many_copy`, `insert_many_skip_unique_conflicts` and the `QueueStorage` batch enqueues register and lock their lanes one at a time in one canonical order, `(queue, priority, shard)` ascending with queue names compared bytewise, so two batch calls cannot hold each other's lanes in opposite orders +- a `QueueStorage` process (batch enqueue, or a worker promoting scheduled or retried jobs) from 0.6.15 or earlier, or from a 0.7 build before this fix, creates a new `(queue, priority)` lane by taking its provisioning lock before writing its `queue_lanes` row; every other producer, and current `QueueStorage`, writes the row first, so one of each that are both the first to use the lane can deadlock (#692). Upgrade those processes, or create a queue's lanes before traffic reaches it with `awa storage prepare-queue` +- from migration v055, `insert_many_copy` and `insert_many_skip_unique_conflicts` take all of a batch's unique claims before any lane lock, while per-row inserts (generic `insert_many`, single-job inserts, and binaries that predate the batch function) take each job's claim and lane together. From migration v057, for 2 s the function waits at most 500 ms (or half of its session's `deadlock_timeout`, if lower) while it holds claims or lanes, then releases them and retries, so it gives way to a per-row transaction that holds a lane the batch needs and waits for one of its keys, provided that transaction's `deadlock_timeout` is above 500 ms (the default is 1 s) and the batch inserts its unique claims within 500 ms (10,000 to 20,000 keys on a fast server, fewer under load; split larger batches). Its last attempt waits as any statement does, so a per-row transaction that holds the lane for longer than the retries (about 2.5 s) before it inserts the key still deadlocks with it, and a cycle that retrying cannot break, for example through a lock the batch's own transaction took in an earlier statement, still ends in `40P01`, after 3 to 4 s and usually by aborting the batch's transaction. The v055 body of the function waited instead, and the two deadlocked; `awa migrate` replaces it - unique keys and the application's own locks can form other cycles -- a transaction that enqueues with several separate statements (`insert` per job, handler-staged follow-ups, `Transaction.insert` in a loop) takes locks in statement order and can deadlock against a batch or another such transaction that touches the same lanes in a different order +- a transaction that enqueues with several separate statements (`insert` per job, handler-staged follow-ups, `Transaction.insert` in a loop) takes locks in statement order and can deadlock against a generic `insert_many`, a `QueueStorage` batch enqueue or another such transaction that touches the same lanes in a different order; a batch through `awa.insert_jobs_compat_batch` normally gives way to it instead Fix: diff --git a/docs/upgrade-0.6-to-0.7.md b/docs/upgrade-0.6-to-0.7.md index 439c61aa4..9026dc919 100644 --- a/docs/upgrade-0.6-to-0.7.md +++ b/docs/upgrade-0.6-to-0.7.md @@ -53,6 +53,10 @@ External migration runners apply the generated `awa migrate --sql --from 52 --to 53` bundle, which does not repair existing lane sequences, then run `awa storage lane-sequences --repair` as the migration role itself. +### New queue lanes during the roll + +When `QueueStorage` is the first to use a `(queue, priority)` lane, through a batch enqueue or a worker promoting scheduled or retried jobs, it writes the lane's `queue_lanes` row before taking its provisioning lock, the order SQL inserts already use. 0.6.15 and earlier, and 0.7 builds before this fix, take them the other way round. Until every `QueueStorage` process is upgraded, an older one and any other producer that are both first to use the same lane can still deadlock, and one transaction fails with `40P01`. Lanes that already exist are unaffected. To avoid it during the roll, create the lanes of a queue or priority before traffic first reaches it with `awa storage prepare-queue`, or retry the failed transaction. No schema change is involved. + ## Custom-schema discovery during migration Migration bundles discover custom substrates using catalog metadata without @@ -293,7 +297,7 @@ Binaries before v054 neither use nor build the index, and binaries that use it r ## v055: batch compatibility insert -v055 adds one function, `awa.insert_jobs_compat_batch`, and changes nothing else: no table DDL, no table or sequence locks, no change to `awa.insert_job_compat` or to storage routing. It inserts a whole batch the way `awa.insert_job_compat` inserts one job, with the same validation, unique claims, lane assignment and storage routing. It does everything that does not depend on a lane first (job ids, unique claims in key order, scheduled rows, the result rows). Then, in `(queue, priority, shard)` order, it registers each lane it is the first to use, reserves each lane's range of positions once and writes the ready rows. Each lane's enqueue lock is therefore held only for the reservation, the ready-row write and the commit, instead of from the batch's first ready row to the commit, so concurrent producers and promotion on the same lane wait for that much shorter window. +v055 adds one function, `awa.insert_jobs_compat_batch`, and changes nothing else: no table DDL, no table or sequence locks, no change to `awa.insert_job_compat` or to storage routing. It inserts a whole batch the way `awa.insert_job_compat` inserts one job, with the same validation, unique claims, lane assignment and storage routing. It does everything that does not depend on a lane first (job ids, unique claims in key order, scheduled rows, the result rows). Then, in `(queue, priority, shard)` order, it registers each lane it is the first to use, reserves each lane's range of positions once and writes the ready rows. Each lane's enqueue lock is therefore taken when the batch reaches that lane instead of at the batch's first ready row. The lock is transaction-scoped: it is held until the calling transaction commits or rolls back, including any work the caller does after the batch. Concurrent producers and promotion on the same lane no longer wait for the batch's lane-independent work or its earlier lanes. What callers see: @@ -304,7 +308,7 @@ What callers see: Binaries before v055 never call the function. They keep calling `awa.insert_job_compat`, which takes the same lane lock, so old and new producers share a lane without reordering it. A database coming from 0.6.15 already has the function from the 0.6 `insert_jobs_compat_batch` schema patch, and v055 replaces it in place. -The batch function takes all of a batch's unique claims before any lane lock, while per-row inserts (generic `insert_many`, single-job inserts, and binaries before v055) take each job's claim and its lane together, lane by lane. When a per-row transaction already holds a lane the batch needs and then inserts a job whose unique key the batch has claimed but not yet committed, PostgreSQL aborts one of them with `deadlock detected` (SQLSTATE `40P01`), where before the per-row insert would have waited and then skipped the job or failed with a unique violation (`23505`). Retry the aborted transaction. Two batches through the function take claims in key order and lanes in lane order, so they do not deadlock with each other this way. +The batch function takes all of a batch's unique claims before any lane lock, while per-row inserts (generic `insert_many`, single-job inserts, and binaries before v055) take each job's claim and its lane together, lane by lane. When a per-row transaction already holds a lane the batch needs and then inserts a job whose unique key the batch has claimed but not yet committed, PostgreSQL aborts one of them with `deadlock detected` (SQLSTATE `40P01`), where before the per-row insert would have waited and then skipped the job or failed with a unique violation (`23505`). [v057](#v057-batch-inserts-give-way-to-per-row-producers) replaces the function's body so the batch gives way instead. Two batches through the function take claims in key order and lanes in lane order, so they do not deadlock with each other this way. New functions are executable by `PUBLIC` unless your deployment revokes that through default privileges. If it does and the runtime role's default privileges do not cover functions created by the migration role, grant it explicitly: @@ -319,6 +323,43 @@ Without `EXECUTE`, `insert_many_copy` keeps the per-row path and `insert_many_sk External migration runners apply the complete `awa migrate --sql --from 54 --to 55` bundle, which refreshes trusted queue-storage discovery before it replaces the function. +## v056: claim batch index recognition + +v056 replaces `.ensure_claim_batch_job_index(slot)` in the default and every discovered custom queue-storage schema. The v054 function counted any valid GIN index whose definition named `job_ids` as the claim batch index, a partial one included, and treated another index or relation that already had the index's name as the index, so claim prune could skip building the real one. The function now accepts only a valid, non-partial GIN index on `job_ids` alone. When something else already has the name `lease_claim_batches__job_ids_idx`, it raises a warning naming it, returns false and leaves it in place; drop or rename it and claim prune builds the index the next time it empties that slot. + +The migration also fixes who may execute the function after a refresh by a migration role that owns it but may only `SET ROLE` to the table owner (PostgreSQL 16+ membership `WITH INHERIT FALSE`). The refresh handed the function to the table owner first, after which the migration role could no longer revoke or grant on it, PostgreSQL only warned, and `EXECUTE` stayed with `PUBLIC`. It now resets `EXECUTE` after the transfer, acting as the new owner when it cannot as itself, so exactly the roles holding `TRUNCATE` on `lease_claim_batches` can execute it. Migrations run with the owner as the effective role, as [database roles](security/database-roles.md) describes, were not affected. + +Like v054 it replaces functions only: it builds and drops no index and takes no lock on any `awa` or queue-storage table. Binaries before v056 call the new function unchanged, and an index the v054 function built is still recognised. A database coming from 0.6.16 already has this function from its `claim_batch_job_index` schema patch. External migration runners apply the complete `awa migrate --sql --from 55 --to 56` bundle, which defines the helper before the tail installs the function. + +## v057: batch inserts give way to per-row producers + +v057 replaces the body of `awa.insert_jobs_compat_batch`, which `insert_many_copy` and `insert_many_skip_unique_conflicts` call. It is one `CREATE OR REPLACE FUNCTION` with no table DDL and no table locks, and the function's signature, privileges and results are unchanged. The fix is in the function, so producers that already call it get it as soon as the migration commits, before or after their binaries roll. + +The function takes a batch's unique claims before its lane locks, while per-row inserts (generic `insert_many`, single-job inserts, and binaries before v055) take each job's claim and its lane together. With the v055 body, a per-row transaction that held a lane the batch needed and then inserted a key the batch had claimed but not committed waited on the batch while the batch waited on it, and PostgreSQL aborted one of them with `deadlock detected` (SQLSTATE `40P01`). + +From v057 the function takes its claims and lanes in a subtransaction and, while it holds any of them, waits at most 500 ms per lock, or half of its own session's `deadlock_timeout` if that is lower. When a wait would run longer, the batch rolls back what it took and tries again after a short randomised pause. The per-row transaction gets the key or the lane, proceeds and commits, and on its next attempt the batch skips the keys it committed, or fails with a unique violation when it does not skip conflicts, as it would for any key committed before it. The same applies to a transaction that enqueues lanes in separate statements in the opposite order to a batch. Two batches through the function still take claims in key order and lanes in lane order, so they cannot deadlock with each other. + +After 2 s of retries, the batch makes one last attempt that waits as the v055 function did. Contention that outlasts the retries is then served in lock order, and a cycle that rolling back cannot break ends in `deadlock detected` rather than endless retries. The protection therefore lasts only for the bounded phase: a per-row transaction that holds a lane for longer than about 2.5 s and only then inserts one of the batch's keys meets the ordinary attempt, and one of the two is aborted with `40P01` as before. + +What callers see: + +- A batch insert and a per-row producer that share keys or lanes no longer abort each other with `40P01` when the batch call is the first thing its transaction locks and the per-row producer's `deadlock_timeout` is above 500 ms (PostgreSQL's default is 1 s). The batch cannot see another session's setting, so the cap is fixed; a per-row session configured at or below 500 ms can still be aborted by its own, earlier deadlock check. +- A per-row transaction that holds a lane longer than the bounded phase (about 2.5 s) before it inserts one of the batch's keys can still deadlock with the batch. +- A batch whose transaction already holds locks from earlier statements can still be part of a deadlock: rolling back the batch cannot release them. That cycle now ends later, after about 3 to 4 s instead of 1 to 2 s, and the transaction usually aborted is the batch's caller rather than the per-row producer. +- A batch can only give way while inserting its unique claims takes less than the 500 ms budget. Each claim costs roughly 20 µs on a fast database server and more on a slow or busy one, so the limit is in the range of 10,000 to 20,000 unique keys. A larger batch, or one whose claims are slowed by load, uses up each bounded attempt before it reaches its lanes and ends in the ordinary attempt, so it behaves as with the v055 body. Split such batches if they share keys or lanes with per-row producers. +- Under contention that outlasts the retries, a batch waits for its lanes in lock order, as before. +- A caller's `lock_timeout` applies to each of the batch's lock waits, as it does to any statement's, and a wait it ends fails the call with `lock_not_available` (SQLSTATE `55P03`). Time the batch spends on its own work or on retries does not count against it. +- The 500 ms budget applies to each wait, not to a statement: a claim statement that meets several keys still in flight, or a lane reservation that waits for the lane's provisioning lock and then its lock, can wait up to the remaining budget each time. +- The lane lock is still taken when the batch reaches the lane and held until the transaction ends. + +Binaries before v057 that call the function pass the same arguments, so they can run before or after the migration, and rolling binaries back needs no schema change. A database coming from 0.6.16 already has this body from its `insert_jobs_compat_batch` schema patch, and v057 replaces it in place. External migration runners apply the complete `awa migrate --sql --from 56 --to 57` bundle, which refreshes trusted queue-storage discovery before it replaces the function. + +## v058: lane row before lane lock + +v058 replaces `.provision_lane` in the default and every discovered custom queue-storage schema. The v053 function took the lane's advisory lock and then inserted its `queue_lanes` row, while producers and `QueueStorage` insert the row first and take the lock after it. A `provision_lane` call and a producer that were both first to use the same new `(queue, priority)` could each wait for the other until PostgreSQL aborted one with `deadlock detected` (`40P01`). The function now inserts the row first. + +It replaces functions only and takes no lock on any `awa` or queue-storage table. Binaries before v058 call the function with the same arguments, so they can run before or after the migration. + ## The one gate `awa migrate` on a 0.7 binary refuses to apply migrations unless one of these holds: