Repository navigation
fix(storage-postgres): serve each table in turn in the GSI queue workers - #542
Open
LeeroyHannigan wants to merge 1 commit into
Open
LeeroyHannigan wants to merge 1 commit into
LeeroyHannigan wants to merge 1 commit into
Conversation
LeeroyHannigan
requested review from
amrith,
c33howard,
jcshepherd,
pdf-amzn,
robinnsc and
yesyayen
as code owners
October 25, 2026 02:40
A queue worker claimed the lowest id across its whole partition. Partitions are shared by every table, so a table's rows waited behind every row another table had enqueued earlier: a 50,000 item bulk write to one table with an asynchronous GSI put 21 writes to another table 9 to 11 s behind their 2 s propagation delay, and the wait scales with the backlog. Pool occupancy, the issue's other candidate, does not occur: a GSI build holds one of the pool's twenty connections. Each batch now lists the tables with a ready row in the partition and claims each table's oldest ready row in turn, so a table's latency depends on its own backlog. Within a table the claims stay in id order, which is what per-key FIFO needs. The worker carries the table it served last from batch to batch, so more than 100 tables with ready rows in one partition are all served once per pass. The partition hash is unchanged. Data migration 005 adds idx_gsi_pending_table_claim on (worker_partition, table_id, id): the per-table claim walks it to the first ready row instead of sorting the partition's ready rows, and the per-batch table listing is a recursive probe over it that lists only the tables whose first row is ready (4.5 ms per batch at 500 tables inside their delay; 0.3 ms with none). The drain rate is the same as before within run-to-run spread on the test host (ten runs: 770 to 2,000 rows/s with the change, 938 to 1,902 without; the claim plan costs 0.02 to 0.04 ms on both). Apply with extenddb migrate --yes; a server started without it logs a warning naming the two slower plans and serves. The catalog version is unchanged. Storage-level tests: a 20,000 row backlog for one table with eight writes to a second table applying while more than three quarters of the backlog is still queued; 104 tables' rows in one partition with the four sorting last applied while more than three quarters of the others' rows are still queued; a held table's rows staying queued until the hold is removed; and the migration reported, applied and its index present on a database one migration behind. Closes #476
LeeroyHannigan
force-pushed
the
fix/476-pg-gsi-queue-cross-table-stall
branch
from
October 26, 2026 09:07
bb9b1da to
ce1ccaa
Compare
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
#476 reports that, on the PostgreSQL backend, another table's GSI build left a table's 21 queued index rows unapplied for 38 s, and names two candidate causes: queue partitions shared across tables, and connection pool occupancy. Reproducing on
main(744ea291) withpg_stat_activityand per-tablegsi_pendingcounts sampled every 2 s settles it.The issue's own shape does not stall on
main:UpdateTablebuilds the GSI inside the request and enqueues nothing, and the build holds one of the pool's twenty connections, so the 21 rows apply 1.96 s after the last write with the build running (2.14 s with nothing else running). The issue's measurement was taken on the #401 head, whose online build does enqueue. The shape that stalls onmainis a backlog: with 50,000BatchWriteItemputs to table A queued, 21 puts to table B apply 9 to 11 s after the last write. Each worker claimsORDER BY id LIMIT 1over its whole partition and partitions are shared across tables, so every row A enqueued earlier is applied before B's first row; B's latency is A's backlog divided by the drain rate (about 1,500 rows/s, so 50,000 rows is about 34 s, the magnitude the issue measured). The samples show zero ungranted locks, twenty idle connections and no batch error, and everything applies when the other table's work ends, which is the issue's picture.Change
Each batch now lists the tables with a ready row in the worker's partition and claims each table's oldest ready row in turn (
gsi_rotation.rs,TableRotation). Within a table the claims stay inidorder, so per-key FIFO holds (a key's rows are in one table and one partition). The worker carries the table it served last from batch to batch, so more thanBATCH_SIZE(100) tables with ready rows in one partition are all served once per pass instead of the tail waiting for the head to empty. The partition hash is unchanged, so no drain or operator step follows from the claim order.The listing returns only tables whose lowest-
idrow is ready. The enqueue floorsready_atat the partition's current maximum, so within a partitionready_atis non-decreasing withidand a table's first row carries its earliestready_at; the one exception (two writers reading the floor before either commits) delays a table's listing by at most one jitter with one delay in force, or by the difference between the two rows' delays if an operator has set a per-indexpropagation_delay_msabove the default (nothing writes that column today). Without this filter a partition holding N tables inside their delay cost N claim transactions per wakeup.Data migration
005_gsi_pending_table_claim.sqladdsidx_gsi_pending_table_claimongsi_pending (worker_partition, table_id, id). The per-table claim walks it to the first ready row (0.02 to 0.04 ms whatever the backlog), and the listing is a recursive probe over it, one index entry per table with rows. Without the index the per-table claim sorted the partition's ready rows (drain fell from about 1,600 to about 240 rows/s in the first measurement). The hold check is bound to the claimed table (NOT EXISTS ... WHERE h.table_id = $2), evaluated once per claim in the same snapshot.Operator step
Stop the servers,
extenddb migrate --config extenddb.toml --pg-user <admin> --yes, start the servers. The catalog version does not change (005 is a data migration, recorded in the data database'sschema_history, as #307's 004 was); the preview lists the file underdata migrations.migratebuilds the index with the servers stopped at about 1.7 s per million queued rows (plainCREATE INDEX, which takesSHAREongsi_pendingfor the build; 3.45 s at 2,000,000 rows). A server started without the index serves and propagates every row and logs one warning at startup naming the two slower plans: each claim for a table behind another table's backlog reads past that backlog (3.6 ms at 40,000 queued rows), and the per-batch listing scans the queue table (33 ms at 40,000 rows, once per 100 claims). The upgrade manual carries the entry.Measurements
Single
pgvector/pgvector:pg16container on one host,index_propagation_delay_ms = 2000, release binaries, from the last write on table B to its rows applied:mainmainUpdateTablebuild shapePer-batch cost with tables inside their delay (one listing statement, no claims): 10 tables 0.49 ms, 100 tables 1.26 ms, 500 tables 4.5 ms;
main's one claim is 0.3 to 0.4 ms. Before the ready-only listing the same shapes cost 4.8, 29.7 and 164 ms (one claim transaction per idle table).Drain rate, isolated 36,000 to 40,000 row backlogs, ten runs alternating binaries in the same hour: 770 to 2,000 rows/s with the change, 938 to 1,902 without; the same within run-to-run spread (the apply is one transaction per row and WAL-bound;
WALWriteandWALSyncwaits in every sample). The stable quantity is the claim plan, 0.02 to 0.04 ms on both. Every index count equalled its table in every run, in two-, three- and fifty-table shapes.Tests
tests/gsi_queue_fairness.rs(2): a 20,000-row backlog for table A (whosetable_idsorts first) and eight writes to table B; B's rows apply while more than three quarters of A's backlog is still queued (observed 19,579 to 19,612 of 20,000; 0 under the old claim); and a held table's rows stay queued with zero index rows until thevector_index_holdsrow is removed, then apply.tests/gsi_queue_many_tables.rs(1): 104 tables with rows in one partition, 100 rows for each of the hundred sorting first and 5 for each of the four sorting last; the tail tables' rows apply while more than three quarters of the head rows are still queued (observed 9,494 of 10,000).tests/data_migrations.rs(1): a data database one migration behind reports 005 pending, applies it, andpg_indexeshasidx_gsi_pending_table_claim;claim_index_presentis false before and true after.gsi_rotationunit tests (9): ring order, cursor placement and wrap, a cursor naming a table no longer listed, six tables in batches of four over three batches.DATA_MIGRATIONSas well as the catalog list.main's claim written over the fixed file, the cursor ignored,served()a no-op, the hold predicate removed, 005 emptied, a duplicatedinclude_str!: each fails the test written for it..github/workflows/integration.yml).cargo fmt,clippy1.97.0 with-D warnings, 1.88.0 build: clean. No dependency change. Files added or grown stay under 600 (gsi_rotation.rs314,gsi_queue.rs501 after a pure move of the claim loop);lib.rs,migrations.rsand the upgrade manual (pre-existing over 600) grew by a few lines.Merge notes
fix/pg-online-gsi-backfill): two files, seven hunks..github/workflows/integration.yml(keep both--testlists).crates/storage-postgres/src/gsi_queue.rs: the module doc, theuseblock, the worker loop's state (cursorhere,parkedthere), the body ofprocess_batch(which this branch moved togsi_rotation.rsand fix(postgres): build a GSI added by UpdateTable correctly while writes continue #401 rewrites in place), andapply_claimed_rowagainstdecode_row. They compose: the merged loop lives ingsi_rotation.rs::process_batch(worker_id, q, cursor, parked); the claim takestable_id = $2andNOT (id = ANY($3))together, with the parked filter applied while walking the index; fix(postgres): build a GSI added by UpdateTable correctly while writes continue #401'sbreakon an empty claim becomesrotation.drained(); its twocontinues (an already-parked item, a lock timeout) keep the rotation on the same table, whose next claim excludes the parked id. fix(postgres): build a GSI added by UpdateTable correctly while writes continue #401 bounds the loop asfor _ in 0..BATCH_SIZEwhere this branch useswhile processed < BATCH_SIZE; the merger picks one. fix(postgres): build a GSI added by UpdateTable correctly while writes continue #401'snext_ready_waittakes the parked ids and carries over unchanged.fix/pg-gsi-create-race): one hunk, the same--testlist; keep both.Not in this PR
ready_atfloor is still partition-wide across tables. With one global delay its effect is bounded by one delay; it becomes a stall only if rows carry different delays, and nothing sets the per-indexpropagation_delay_mscolumn today. Scoping the floor to(table_id, worker_partition)needs an index leading ontable_idfor theMAX.DescribeTableonmainreports a GSI added byUpdateTableasACTIVEwhile its backfill is still running inside the request (the service reportsCREATING); fix(postgres): build a GSI added by UpdateTable correctly while writes continue #401 is the change that makes the build online.next_ready_waitkeeps the correlated hold anti-join (13 ms on a 40,000-row queue, once per empty batch).main'sbreakpasses every test; it changes only the drain rate while a held table or a concurrently drained table is listed. Recorded in the test module comment as an accepted gap.Closes #476
Rebase on #401
The branch is rebased onto
mainatabf8082f. #401'sParked,held_item,decode_row,is_lock_timeoutandROW_LOCK_TIMEOUTarepub(crate)ingsi_queue.rs, sogsi_rotation::process_batchparks a row whose lock wait on its base row times out and leaves parked ids out of each table's claim. The loop keeps this branch'swhile processed < BATCH_SIZE, so a passed-over row does not count toward the batch.The rebase also changes two tests:
gsi_queue_support::enqueue_rowswrites the base items as well as the queue rows. fix(postgres): build a GSI added by UpdateTable correctly while writes continue #401 applies a secondary index row from the base item as the table holds it at apply time, so seeded rows with no base item left the index empty (gsi_queue_fairnesscounted 0 index rows for a 20,000 row backlog,gsi_queue_many_tables0 for a table's 100 rows).gsi_online_build::queue_rows_applied_out_of_order_converge_to_the_itemholds the older queue row withFOR KEY SHAREfrom an outside transaction and makes both rows due, in place of making only the newer row due. This branch lists a table only when its lowestidrow is due, so the newer row was never claimed. The changed test passes onmain's code as well.gsi_queue_fairness(2),gsi_queue_many_tables(1),data_migrations(1),gsi_online_build(12),gsi_stale_writer(1) and the crate's unit tests (71) pass together.