Repository navigation
perf(insert): shorten lane lock holds for COPY and skip-conflict batches (0.6) - #690
Conversation
…ts (0.6) insert_many_copy called awa.insert_job_compat once per row. The function takes the (queue, priority, shard) lane lock in reserve_enqueue_seq on the batch's first ready row, and the lock lasts to commit, so it was held while every remaining row was processed and producers on one lane queued behind whole batches. The insert_jobs_compat_batch schema patch (awa 0.7 migration v055, without its schema_version row) adds awa.insert_jobs_compat_batch. It allocates job ids, claims unique keys in key order, registers new lanes and builds the result rows first, then reserves each lane's sequence range once, in (queue, priority, shard) order, and writes the ready rows and segments in two statements. It reads the ready-ring cursor through ring_cursor() where a later schema provides one. insert_many_copy probes its own connection on every call and uses the function when it exists and the role may execute it, otherwise the per-row path. The new insert_many_skip_unique_conflicts requires it. Generic insert_many keeps its per-row statement on every schema. Released 0.6 binaries never call the function and share the same lane lock.
enqueue_lane_contention_test measures lane throughput and batch latency for many producers on one lane, across insert paths, unique keys and enqueue_shards. Its nightly gate runs sixteen producers on one lane and on four shards, checks lane integrity and compares the batch function's lock hold time with the per-row function's. AWA_LANE_BENCH_WITHOUT_PATCH drops the batch function so the matrix can measure the per-row COPY path.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the
|
…tch-insert-lane-lock-0.6 # Conflicts: # CHANGELOG.md
awa.insert_jobs_compat_batch registered every lane it was the first to use before reserving any. The head tables' row trigger provisions a lane under its lock and the uncommitted head row blocks other producers of that lane, so a batch for an existing lane A and a new lane B held B while waiting for A, and a per-row producer (generic insert_many, single inserts, 0.6.14 binaries) holding A and then enqueuing into B deadlocked with it. The batch now registers each missing lane inside the reservation loop, immediately before reserving it, so new lanes follow the register-then-reserve order every other producer uses. The patch probe matches the new body, so `awa migrate` replaces the earlier one. Documents that a per-row transaction sharing a unique key with a batch can now fail with 40P01 rather than skip the job or raise 23505, and corrects the lane-order statements in the changelog, upgrade guide and troubleshooting.
…tch-insert-lane-lock-0.6 # Conflicts: # CHANGELOG.md
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
…tch-insert-lane-lock-0.6 # Conflicts: # awa-model/src/migrations.rs
Backport of #635 to the 0.6 line, for 0.6.15.
On queue storage every enqueue holds a transaction-scoped advisory lock on its
(queue, priority, shard)lane until commit.insert_many_copycalledawa.insert_job_compatonce per row, so the lock was taken at the batch's first ready row and held while the rest of the batch (job ids, unique claims, savepoints, payloads) was processed. Many producers enqueuing large batches into one queue therefore serialised on whole batches, and promotion into that queue waited behind them.Change
insert_jobs_compat_batch(awa-model/migrations/patches/insert_jobs_compat_batch.sql): addsawa.insert_jobs_compat_batch, the body of main's v055 without theschema_versionrow. It does the lane-independent work first (job ids, unique claims in key order, deferred rows, result rows), then, in(queue, priority, shard)order, registers each lane it is the first to use and reserves each lane's range once, and writes ready rows and segments in two statements. The lane lock covers only the registration, the reservation, those writes and the commit.queue_ring_state, or throughring_cursor('queue')when a later schema provides it, so the 0.6 body stays correct on a 0.7 schema until v055 replaces it. Same signature and return type as v055, so v055'sCREATE OR REPLACEapplies cleanly over it.SCHEMA_PATCHES: applied byawa migrateat v40, never writesschema_version, exported asR__insert_jobs_compat_batch.sqlby--sql/--extract-to. The probe is catalog-only and matches the body (prosrcregisteringqueue_lanesfromv_laneinside the reservation loop), soawa migratereplaces the earlier body that registered lanes up front.insert_many_copy(Rust, and Pythoninsert_many_copy) probes its own connection on every call (has_function_privilege(to_regprocedure(...), 'EXECUTE')) and uses the batch function when installed and executable; otherwise it keeps the existing per-row path. Unique conflicts are skipped on both paths, rows come back in input order.insert_many_skip_unique_conflicts(new Rust API) uses the batch function and returns a validation error naming the patch when it is missing.insert_manyis unchanged: one statement,awa.insert_job_compatper row in canonical lane order, on every schema. Its executor cannot be probed first, and there is no process-wide cache, so a process writing to databases with and without the patch never sends unsupported SQL into a caller's transaction.(queue, priority, shard)order. A new lane is registered inside the reservation loop immediately before its reservation, asinsert_job_compatandQueueStoragedo; head rows are proposed only when missing, because the head tables' row trigger provisions the lane before the conflict check.40P01instead of a skip or23505; documented in the CHANGELOG, upgrade guide and troubleshooting, with retry as the remedy. Moving claims into the lane window would undo the change.Rollout
awa.insert_job_compat, which takes the same lane lock. Mixed old/new producers on one lane keeplane_seqdense and each batch contiguous and in input order.insert_many_skip_unique_conflictserrors until then.docs/upgrade-0.6.14-to-0.6.15.md, CHANGELOG[Unreleased], troubleshooting. Deployments that revokeEXECUTEon new functions must grant it; without it COPY stays per-row.Tests
New:
insert_many_batch_test(9): new-lane ordering (a per-row transaction holding provisioned lane A enqueues into unprovisioned lane B while a batch for A and B waits on A; fails with40P01on the earlier body, passes now); lane order, denselane_seq, one segment per batch and claimability through each ofinsert_many/ skip API / COPY; mixed available + scheduled rows; atomic unique failure; skip semantics (claimed keys, in-batch repeats); canonical storage; the lane lock is no longer held across bookkeeping (with a gate holding the lane lock, a waiting batch already holds its last row's unique claim and has written no ready row); old per-row and new batch producers sharing a lane.insert_many_schema_fallback_test(5): the earlier body (fixture) keeps the patch pending andawa migratereplaces it; binary ahead of the patch falls back,awa migrateapplies it (no version row,pending_schema_patchesempty), COPY switches on the next call, dropping the function again does not poison later calls; databases with and without the patch in one process inside caller transactions incl. rollback; exported patch applied twice in one explicit transaction and over an installed function; a role withoutEXECUTEkeeps COPY per-row.enqueue_lane_lock_order_test: ported main's canonical-order check (a batch parked on a middle lane holds exactly the earlier lanes) and added the skip API as a producer (9 tests).enqueue_lane_contention_test(ignored benchmark + nightly gate, ported from perf(insert): shorten lane lock holds for COPY and skip-conflict batches #635;AWA_LANE_BENCH_WITHOUT_PATCH=1measures the per-row COPY path).integration_test: ported perf(insert): shorten lane lock holds for COPY and skip-conflict batches #635's guard serialising the bind-limit test with storage switches (it raced on 0.6 too).Local results (PostgreSQL 17 and 18 in Docker, full Rust suite via the CI shard layout with
--no-fail-fast):* Not caused by this change:
admin_read_cancel_test(2 tests) fails identically on unmodifiedrelease/0.6.6in this environment;test_queue_storage_promotion_does_not_wait_for_the_notify_lock_in_its_lanesneedsfsync=on(commit_delayis a no-op without it) and passes on fsync-enabled PG17 and PG18;descriptor_constraints_test(2) and the bind-limit test (fixed here) pass on rerun. Python suite on PG17: 312 passed, 1 skipped.Released-binary compatibility: the 0.6.14 tree (
a8febdc0)copy_test(13),enqueue_lane_lock_order_test(7) andintegration_test(47) pass against a patched PG17 database, and the function is still installed after itsmigrate.Benchmark
enqueue_lane_contention_testmatrix, release build, 185-row unique batches into one lane, fresh database per run, fsync on, two rounds each (local Docker, shared machine, so ranges rather than baselines):insert_many_skip_unique_conflictsinsert_many(unchanged)The nightly gate passes on both: a single 185-row batch takes 22–25 ms against 135–147 ms through the per-row function, and 16 producers on one lane reach 11.4k (PG18) / 17.9k (PG17) jobs/s, 22–23k on four shards.
After the review fix (
d2de3738) and merges ofrelease/0.6.6(#669, #683, #688)insert_many_batch_test9,insert_many_schema_fallback_test5,enqueue_lane_lock_order_test9,copy_test13,migration_test44,awa-modelmigrations unit 9,migrate_patch_cli_test3,least_privilege_runtime_test4,lane_ownership_roles_test+dirty_marks_wait_free_test16, all passing. fmt, offline clippy (all targets/features,-D warnings) and offline workspace build pass.Not covered