Skip to content

fix(postgres): build a GSI added by UpdateTable correctly while writes continue - #401

Open
robinnsc wants to merge 1 commit into
mainfrom
fix/pg-online-gsi-backfill
Open

robinnsc wants to merge 1 commit into
mainfrom
fix/pg-online-gsi-backfill

Conversation

@robinnsc

@robinnsc robinnsc commented Oct 16, 2026 •

Copy link
Copy Markdown
Collaborator

What

Adding a GSI with UpdateTable on PostgreSQL now produces a correct index when writes keep going during the build. Before this change the index ended up wrong or the call failed. The same cached-definitions bug on SQLite is fixed alongside.

The new module crates/storage-postgres/src/data/gsi_build.rs documents the ordering argument. The build:

  • update_table.rs creates the GSI's data table before it commits the indexes row, and commits that row as CREATING instead of ACTIVE. Every write reads the catalog, so once the row commits, writes maintain the index exactly as they maintain an ACTIVE one, inline or through the propagation queue.
  • After the commit, UpdateTable waits 1 s. It then takes LOCK TABLE <base> IN SHARE MODE and releases it at once; the lock waits for writes already in flight. Writes arriving meanwhile queue behind the request, each holding a data pool connection, so the request has a lock timeout (one second over deadlock_timeout, which is what lets it cancel an autovacuum) and is retried up to six times with growing pauses: with the default deadlock_timeout, about 12 s of write stalls spread over 20 s in the worst case before the build is left to recovery.
  • The backfill runs in keyset pages of 256 over the full base key, up to the last key present when it starts. Each page locks its rows FOR SHARE SKIP LOCKED and writes each item's current state into the index. Rows a writer held are then done one at a time with a waiting lock, so a page never holds row locks while it waits on another one. Pages are retried on 40P01 and 40001.
  • A last pass deletes index rows whose item no longer exists. It locks the rows first and then deletes those exact rows by ctid with a fresh snapshot, so a writer re-creating the item at that moment keeps its row.
  • When the backfill finishes, the index flips to ACTIVE. If the build fails for a transient reason (a conflict, a lock timeout, a lost connection or an empty pool, a shutdown), the index stays CREATING and the request still succeeds. A new background worker (recovery_worker, every 60 s, with backoff per index) finishes such indexes, and also those left behind by a crash. Any other error, including a row that does not decode, counts as one that would recur: the index is removed and the request fails, as before, and the recovery worker does the same when it meets one. The removal only touches a row still CREATING, so a second build of the same index (the recovery worker next to an UpdateTable that could not take build ownership) cannot remove an index the first one has published. The data table is dropped a second after the catalog row, and the synchronous write path treats a table that is gone as an index that is gone, so a write that read the catalog just before the removal does not fail.

Queue and write path:

  • gsi_queue.rs: a queued GSI update now applies the item's current state, read under its row lock (resync_base_key), instead of the image the row carried. A row applied late, twice, out of order by a second server process, or after the backfill already wrote a newer state can no longer leave the index behind the table.
  • index.rs: the inline (synchronous) path writes a GSI the way the backfill and the queue do (write_state): delete the item's rows by base key, then upsert the current state on the index primary key. LSIs keep the old image-based delete.
  • delete_table.rs and worker_store.rs drop a table's storage through one helper (drop_table_storage) that stops the propagation queue for the table first (a vector_index_holds row, which the claim query already honours), deletes the table's pending rows, which waits out the claims in flight, and only then drops the data tables. The queue worker holds a claimed row and takes FOR SHARE on the base row; a drop that took the tables first deadlocked with it, and as the victim left the data tables behind with the catalog rows already gone. With 16 writers and DeleteTable 1 s in, 8 rounds: main failed 7 of 8 with 14 orphan tables and 20 deadlocks; this branch 0 of 8, none, none. Main has this through the index table rather than the base table.
  • gsi_queue.rs: the worker's transaction sets lock_timeout = 2s, so a client holding an item's row open stalls that worker's partition for at most two seconds per claim instead of for the hold; the claim rolls back and is retried after 200 ms.
  • update_table.rs (both backends): a GSI Create while another GSI on the table is CREATING is refused with the service's LimitExceededException sentence ("Only 1 online index can be created or deleted simultaneously per table"), now a shared constant. That is the case measured against the service. A Delete, including of the index being built (how a stuck one is cleared by hand), and a throughput Update are not refused.
  • ddl.rs (both backends): a Query or Scan naming a CREATING GSI resolves as IndexNotFound, which the engine already answers with ValidationException "The table does not have the specified index", as the service does. On SQLite this was a 500 from the partial or missing data table, on main too.
  • update_item.rs (PostgreSQL): the post-update item's index keys are validated for type and emptiness, as PutItem, TransactWriteItems and SQLite already did. Setting a GSI key to the wrong type was written and left out of the index; it is now the service's ValidationException. Same on main. The check covers the whole updated item, so an UpdateItem that does not touch a GSI key attribute that already has the wrong type (an item written before the index existed) is refused too, as it is on SQLite and in TransactWriteItems.
  • Index attribute definitions now come from the write's catalog read, which now joins tables, instead of the cached TableKeyInfo. The engine drops that cache only when UpdateTable returns. Until then, writes built rows for a new GSI with a RANGE key on a new attribute without the sort key column, and skipped the type check on that attribute.

The remaining gap. The catalog and the data are separate databases, so a write that read the catalog just before the index row committed cannot learn about the index in its own transaction. Two things cover that write:

  • Each write stamps the time of its catalog read (after the catalog connection is acquired, so pool wait is not counted), on CLOCK_BOOTTIME so suspend time is counted.
  • If more than 500 ms has passed when it is about to commit, it re-reads the catalog and, in the same transaction, queues a repair row for any GSI it did not know about.

A write whose base-table statements all returned within 500 ms holds the table's ROW EXCLUSIVE lock from the first of them until it commits, and the barrier comes at least 1 s after the catalog commit, so the barrier waits for it and the backfill sees it. A slower write repairs itself. Every source of measurement error makes a write look slower or the wait longer, which is the safe direction.

A write with no transaction (a plain put or delete on a table the write believed had no index) takes the transaction path instead if it is already past 500 ms when it decides. One that becomes slow only after that queues its repair in a separate transaction after the write; if the process dies between the two, that item stays out of the index (or, for a delete, its old row stays in) until the item is next written. The module doc records this.

SQLite: SQLite took a new GSI's attribute definitions from the cached TableKeyInfo in the same way. A write landing after the build but before the cache was dropped wrote the index row without its sort key column, so a Query on the full index key missed the item. CI found this with the new pytest. SQLite now reads the definitions with its index read (fetch_indexes_for_table joins tables) and uses them to build rows, queue rows, and validate keys. SQLite already reads its index set under its write lock, so it does not have the cross-database window.

Other changes:

  • The OFFSET-based backfill_gsi in table_helpers.rs is removed.
  • Docs: docs/design/04-component-storage.md §6.2 describes the build as implemented.
  • CI: the two new storage-level test files are added to the PostgreSQL storage-level step in integration.yml.

Why

No issue filed. Found in the 1.0 readiness review (P0-4: adding a GSI under concurrent writes fails on PostgreSQL). The readiness document puts the cause in the index table's primary key. That is not it: the PK already includes the base key (ddl.rs, (pk, base_pk, base_sk...)). The causes on main:

  1. The indexes row committed ACTIVE before the data table existed. Writes in between failed with 42P01 (synchronous GSIs). Asynchronous ones queued rows that the worker skipped as "table dropped", so those rows were lost.
  2. backfill_gsi paged ORDER BY key LIMIT 500 OFFSET n while writes landed. Rows were read twice, which hit duplicate key value violates unique constraint and failed UpdateTable with InternalServerError, or were skipped.
  3. Queue rows applied their own old/new images, and deleted the old row only if the old image had the index keys. A row applied after the backfill wrote the same item collided (23505, retried forever, stalling its partition) or left a second row for the item.
  4. Writes that read the catalog before the index existed maintained nothing. The backfill could pass their item before they committed.

Measured with a script that loads 2,000 items, runs 4 writer threads (put, update of the key, REMOVE of the key, delete, TransactWriteItems), and adds a GSI mid-traffic. On main, 3 of 3 runs ended ACTIVE but wrong: 15 to 18 rows missing, 33 to 38 extra and 40 to 50 stale out of about 1,770. With a RANGE key on a new attribute, UpdateTable failed with InternalServerError. On this branch every run matches the table: hash-only and hash+range GSIs, asynchronous and synchronous propagation, 4 and 8 writers. SQLite is not affected, because it holds its write lock for the whole backfill, and it passes the same test on main.

Not closed here

  • DescribeTable does not send Backfilling on a CREATING GSI; the service sends false, then true.
  • UpdateTable still returns after the build rather than right away with CREATING, as before.
  • Vector indexes have the same cross-database window for writes that read the catalog just before an index was created. They are left as they are.
  • On MongoDB, the new pytest is marked xfail. With these writers, concurrent UpdateItem and TransactWriteItems calls that create the same item return HTTP 500 (ImmutableField on _id). That also happens on a table with no index at all, so it is the MongoDB write-race issue that test_put_item_create_race.py already marks, not this change.
  • On Cassandra, the new pytest is skipped. In CI its writers got HTTP 500 from DeleteItem under contention on one item, and the online build had not finished after 10 minutes of them. Both are Cassandra backend issues worth their own issue.
  • If a client disconnects mid-build, the build still finishes, because it runs in its own task. The engine's cached TableKeyInfo on that node is not invalidated, because that happens after the storage call returns. This was already true on main.

Cost

  • No extra SQL on the write path unless a write takes more than 500 ms. Above that, it costs one catalog read, once per table for TransactWriteItems.
  • On tables with a synchronous GSI, each write does a DELETE by base key before its upsert (an overwrite already did; a new item did not), and the GSI maintenance runs inside one SAVEPOINT/RELEASE pair per write so that an index removed meanwhile does not fail the write (the pass is redone without that index). With N synchronous GSIs that is 2N + 2 data round trips per write where there were N to 2N. Asynchronous GSIs, the default, add nothing on the write path.
  • A queue apply now reads the base row FOR SHARE (four statements for a present item, with the lock timeout, instead of one or two). That lock means a queued update and a concurrent write to the same item now wait for each other, where before the worker only touched the index table; a foreground transaction holding a row holds up its queue partition for up to 2 s per claim attempt.
  • The backfill writes two statements per row (delete by base key, upsert) in one transaction per page, against main's one INSERT per row, and ends with an anti-join of the index against the table.
  • Write throughput and queue drain time showed no measurable difference with 8 client threads on one async-GSI table (about 610 writes/s on both, queue drained within 20 ms), on distinct and on 20 hot keys. The client was the limit there, so this is not a ceiling measurement.
  • Each UpdateTable that creates a GSI takes at least 1 s longer.

Testing done

New tests/test_gsi_online_backfill.py: adds a GSI while four writer threads run puts, key updates, key removals, deletes and TransactWriteItems, then compares a Scan of the index with a Scan of the table (missing, extra, stale, and items indexed twice), for a hash-only GSI and a hash+range GSI on a new attribute. On main both fail on PostgreSQL. On this branch both pass on PostgreSQL and SQLite. On main, SQLite fails the hash+range case only intermittently (it did in CI); the deterministic test below covers that case. On MongoDB both are xfail and on Cassandra both are skipped (see above).

New crates/storage-postgres/tests/gsi_online_build.rs. On main every test fails except the recovery test, which needs code this PR adds.

  • queue_rows_applied_out_of_order_converge_to_the_item: on main, the item is indexed under both its old and new key.

  • queue_row_for_a_row_the_backfill_wrote_is_applied: on main, 23505 and the queue never drains.

  • sync_write_over_an_existing_index_row_succeeds: on main, the put fails with 23505.

  • sync_write_survives_a_gsi_whose_table_is_gone: two synchronous GSIs, one whose data table is gone; the write succeeds and the other index is right.

  • a_creating_gsi_refuses_a_second_change_and_reads: with a GSI CREATING, a second Create gets LimitExceeded and the index resolves as not found; it resolves again once ACTIVE. The same test exists for SQLite.

  • delete_table_waits_for_a_claimed_queue_row_then_drops_everything: a queue row for the table is claimed by hand (the pending row locked, the base row share-locked, as a worker mid-apply holds them). DeleteTable does not finish while the claim is held, has stopped the queue and dropped nothing; once the claim is released it finishes with no data table, hold or queue row left.

  • update_item_rejects_a_wrong_typed_gsi_key: SET of a GSI key to a number on a string index is a ValidationException.

  • concurrent_writes_during_a_build_leave_the_index_equal_to_the_table: 1,200 items and 4 writers, synchronous then asynchronous. On main, writes fail with 42P01.

  • recovery_finishes_a_gsi_left_creating: missing, stale and orphan rows are put right, and the index becomes ACTIVE.

  • write_with_key_info_older_than_a_new_gsi_indexes_its_sort_key: a put carrying key info from before the GSI was added. On main, the index row has no sort key. The same test is added for SQLite in crates/storage-sqlite/src/update_table.rs, where it also fails without this change.

New crates/storage-postgres/tests/gsi_stale_writer.rs: a hidden test hook (pause_writes_after_index_read_for_tests) makes writes pause 3 s after their catalog read, so they write after the build has finished. It covers a new item, a key change and a delete, on the autocommit path and the transaction path. With the repair turned off by hand, the test fails: all three items are wrong in the index.

Full PostgreSQL pytest run (tests/, -n 6) against servers built from this branch and from main: no test fails on the branch that passes on main. The failures that are common to both come from the local environment (CLI lifecycle without a connection string, import/export paths not configured). The same comparison for the full SQLite pytest run: no new failures. The Rust integration suite (tests/rust, --test-threads=1) has the same 3 failures on both.

Eight rounds of independent review, and Lee's review of e9e0736 against PostgreSQL, SQLite and the live service; its six items are fixed here, and the Backfilling member is listed above. of the design and the implementation. Each round's findings are fixed here or listed under "Not closed here".

cargo fmt --all -- --check
cargo +1.97.0 clippy --all-targets -- -D warnings
cargo +1.88.0 check --workspace --locked
EXTENDDB_TEST_PG_CONNECTION_STRING=... cargo test --workspace --exclude extenddb-storage-cassandra
EXTENDDB_TEST_PG_CONNECTION_STRING=... cargo test -p extenddb-storage-postgres --test gsi_online_build --test gsi_stale_writer

Checklist

  • I have read CONTRIBUTING.md
  • All tests pass (cargo test --workspace)
  • Code is formatted (cargo fmt --check)
  • Clippy is clean (cargo clippy -- -W clippy::pedantic)
  • I have added or updated tests for new functionality
  • I have updated documentation if behavior changed
  • Breaking changes are noted below (if any)
  • If this changes the wire protocol, Storage trait, auth model, on-disk
    format, or public CLI surface, an RFC has been accepted or is linked
    below. Otherwise, an ADR captures the decision (link below).

ADR / RFC: n/a. There is no schema migration, no wire change and no trait change.

Breaking changes

None on the wire. DescribeTable reports a GSI that is being built as CREATING instead of ACTIVE (PostgreSQL; SQLite already did, for the duration of its shorter build). While it is CREATING, Query and Scan naming it get ValidationException and a further GSI change gets LimitExceededException, as with the service.


By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache License 2.0 and I agree to the Developer Certificate of
Origin (DCO). See CONTRIBUTING.md for details.

@robinnsc
robinnsc force-pushed the fix/pg-online-gsi-backfill branch 3 times, most recently from a979c6f to e9e0736 Compare October 16, 2026 08:06

@LeeroyHannigan LeeroyHannigan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed e9e0736 on PostgreSQL and SQLite against main cc981e2 and against live DynamoDB.

The new pytest fails 2 of 2 on main and passes 2 of 2 here. I also ran six writers on five hot keys (3 of 3 equal), kill -9 mid-build then restart (recovery finished it, index equal to table), two server processes on one catalog (equal, zero failed writes), and two concurrent builds on one table (both equal after the queue drained, 2 of 2). Regression sweep on the item and index suites: 302 of 304 here, 300 of 304 on main, the two gains are the new test, the two shared failures are async timing tests that fail on both. SQLite 304 of 304. fmt, clippy 1.97.0, MSRV 1.88.0, all four notices checks, workspace tests green. UpdateTable +1.01 s, write throughput 843 vs 841 per second on an async GSI, barrier stall at most 57 ms on any write.

Six items to fix, inline below.

  1. DeleteTable deadlocks with the queue worker while writes are in flight and returns 500, leaving data tables with no catalog row. 16 writer threads, DeleteTable 1 s in, 8 rounds: 6 of 8 returned InternalServerError, 19 deadlocks, 12 orphan tables. Main does the same (8 of 8, 16 orphans) through the index table instead of the base table, so this is not new, but the comment at delete_table.rs:116 says the drop order closes it and it does not for the queue path.
  2. The queue apply's FOR SHARE on the base row has no lock_timeout. One client holding a row open stalls that worker's whole partition for the duration (23 s measured against a 25 s hold).
  3. A second UpdateTable Create during a build is accepted on both backends. The service returns LimitExceededException, "Subscriber limit exceeded: Only 1 online index can be created or deleted simultaneously per table".
  4. SQLite returns 500 on Query and Scan of a GSI for most of its build window (38 of 40 reads during a 40,000 item build). The service returns ValidationException 400, "The table does not have the specified index: gsi1". Same on main SQLite. The body describes this gap as "not refused". On SQLite it is a 500.
  5. PostgreSQL accepts UpdateItem SET of a GSI key attribute to the wrong type. The service and SQLite here reject it with "Type mismatch for Index Key gsik Expected: S Actual: N IndexName: gsi1". PutItem is rejected on all three. Same on main.
  6. The body says the CREATING status is a PostgreSQL change. SQLite shows CREATING to a concurrent DescribeTable during its build too. Neither backend sends Backfilling on the CREATING index; the service sends false then true.

Minor: the recovery worker logs a removed index by UUID where the inline path logs table and index name; backoff is in-memory and silent between retries; the sweep's ctid lock has no test (removing it passes all 8 tests, 3 runs). I could not make the lock matter with the writers in this PR since write_state deletes by base key before upserting, so no writer updates an index row in place.

// while the page waited on the dropped index table: a deadlock whose
// victim could be this transaction, after the catalog rows are gone.
// Dropped first, the base table waits for the page to finish.
Self::drop_data_table(&mut data_tx, &row.table_id).await?;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This order still deadlocks with the queue worker. The worker holds its claimed gsi_pending row and waits for RowShareLock on the base table (the FOR SHARE resync); this transaction holds the drop and waits on that row at the DELETE FROM gsi_pending below. With 16 writers in flight, 6 of 8 DeleteTable calls returned 500 and the catalog rows were already gone, so 12 data tables were left behind. Main deadlocks the same way through the index table. A fix is to stop the queue for the table before the drop, using the hold the claim query already honours (vector_index_holds), then wait for claimed rows, delete the pending rows and drop.

let Some(key_item) = new_item.or(old_item) else {
return Ok(());
};
crate::data::gsi_build::resync_base_key(tx, table_id, idx, base_key_schema, attr_defs, key_item)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resync_base_key takes FOR SHARE with no lock_timeout. A client holding this row open (UpdateItem, conditional PutItem, DeleteItem, TransactWriteItems) blocks this worker's partition for the whole hold: 23 s measured against a 25 s hold. The barrier has a bounded lock_timeout and retry; this path needs the same.

(table_id, index_name, index_id, index_type, key_schema, projection,
index_status, provisioned_throughput)
VALUES ($1, $2, $3, 'GSI', $4, $5, 'ACTIVE', $6)",
VALUES ($1, $2, $3, 'GSI', $4, $5, 'CREATING', $6)",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing refuses a second Create while one GSI is CREATING. I added gsi2 1 s into the gsi1 build and both ran. The service returns LimitExceededException, "Subscriber limit exceeded: Only 1 online index can be created or deleted simultaneously per table". SQLite accepts it too.

// the read's stamp lets a write that was slow to commit queue a repair
// for any GSI it did not see (`gsi_build::fix_up_stale_write`), and a fast
// write is one the backfill sees. Vector indexes still have the window.
let (indexes, vector_metas, stamp) =

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UpdateItem SET of a GSI key to the wrong type is written here. gsik is S in the index schema; SET gsik = {N: 9} returned ok and the item now carries N. The service and SQLite on this branch return ValidationException "Type mismatch for Index Key gsik Expected: S Actual: N IndexName: gsi1". PutItem rejects it on all three. Same on main.

.map(|(i, c)| format!("b.{c} = ${}", i + 1))
.collect();
let lock = format!(
"SELECT ctid::text FROM {idx_table} WHERE {} FOR UPDATE",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No test pins this lock. With the lock removed and the delete done by base key with NOT EXISTS in one statement, all 8 tests in gsi_online_build and gsi_stale_writer pass, 3 runs. I could not make it matter either. write_state deletes by base key before upserting, so no writer in this PR updates an index row in place. A test that does would pin it, or a comment saying the guard is for a writer path that does not exist yet.

// The same answer UpdateTable gives: an error that would recur on
// every attempt removes the index rather than rebuilding it hourly.
tracing::error!(
index_id = %index_id,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The inline path logs the table and index name when it removes an index; this one logs the UUID. The operator has to join it to the catalog to learn what disappeared.

);
}

/// A write that carries the key info cached before an UpdateTable added a GSI

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SQLite shows IndexStatus CREATING to a concurrent DescribeTable during its build (0.5 s on 2,000 items), so the breaking-changes note is not PostgreSQL only. Query and Scan of the index during that window return 500 (38 of 40 reads on a 40,000 item build); the service returns ValidationException 400 "The table does not have the specified index". Same on main SQLite.

@robinnsc
robinnsc force-pushed the fix/pg-online-gsi-backfill branch from e9e0736 to 13e5f30 Compare October 20, 2026 00:35
@robinnsc

Copy link
Copy Markdown
Collaborator Author

Thanks for the thorough pass, and for measuring against the service. All six are addressed in 13e5f30 (still one commit); the branch description and the design doc are updated to match.

  1. DeleteTable deadlock. You were right that the drop order only moved the cycle. Both delete paths (delete_table.rs and the deferred one in worker_store.rs) now go through drop_table_storage, which does what you suggested: a vector_index_holds row for the table (committed on its own so the claim query sees it), then DELETE FROM gsi_pending for the table, which waits out the claims already in flight, then the drops, then the hold. Your scenario (16 writers, DeleteTable 1 s in, 8 rounds): main 7 of 8 failed, 14 orphan tables, 20 deadlocks; this branch 0, 0, 0. There is a storage test for it (delete_table_under_queue_traffic_leaves_no_data_tables, three rounds). The comment that claimed the order closed it is gone.
  2. Queue FOR SHARE with no timeout. The worker's transaction now sets lock_timeout = 2s; a 55P03 rolls the claim back and the row is retried after 200 ms, logged at debug rather than error. So a 25 s hold costs that partition 2 s per attempt, not 25.
  3. Second Create during a build. Both backends refuse a GSI Create or Delete while another GSI on the table is CREATING, with the service's LimitExceeded sentence (now ONLINE_INDEX_LIMIT, shared with the two-actions-in-one-request check). Tested on both.
  4. Query/Scan on a CREATING GSI. fetch_index_info_by_table_id on both backends treats a CREATING row as not found, which the engine already answers with ValidationException "The table does not have the specified index". That is the service's response, and it replaces the 500 on SQLite.
  5. UpdateItem wrong-typed GSI key. PostgreSQL's UpdateItem now runs the same post-update index key type and emptiness validation as SQLite and TransactWriteItems. Test added.
  6. CREATING status wording. The breaking-changes note now says SQLite already showed CREATING during its build, and Backfilling is listed as not sent, under "Not closed here".

Minor items: the recovery worker's log lines now carry table and index name. The ctid lock's doc comment now says plainly that no writer in this crate updates an index row in place today, so the single-statement form would not misbehave against them, and that the guard is for a writer that does; it costs one statement on a path taken only for an item that is gone. I left it in rather than remove a guard whose absence a future writer would discover the hard way. The backoff stays in-memory; it is per process and resets on restart, which for a 60 s to 1 h schedule seems acceptable, but say if you'd rather have it in the catalog.

Comment thread crates/storage-sqlite/src/update_table.rs Fixed
@robinnsc

Copy link
Copy Markdown
Collaborator Author

Thanks for the thorough pass, and for measuring against the service. All six are addressed in 13e5f30 (still one commit); the branch description and the design doc are updated to match.

  1. DeleteTable deadlock. You were right that the drop order only moved the cycle. Both delete paths (delete_table.rs and the deferred one in worker_store.rs) now go through drop_table_storage, which does what you suggested: a vector_index_holds row for the table (committed on its own so the claim query sees it), then DELETE FROM gsi_pending for the table, which waits out the claims already in flight, then the drops, then the hold. Your scenario (16 writers, DeleteTable 1 s in, 8 rounds): main 7 of 8 failed, 14 orphan tables, 20 deadlocks; this branch 0, 0, 0. There is a storage test for it (delete_table_under_queue_traffic_leaves_no_data_tables, three rounds). The comment that claimed the order closed it is gone.
  2. Queue FOR SHARE with no timeout. The worker's transaction now sets lock_timeout = 2s; a 55P03 rolls the claim back and the row is retried after 200 ms, logged at debug rather than error. So a 25 s hold costs that partition 2 s per attempt, not 25.
  3. Second Create during a build. Both backends refuse a GSI Create or Delete while another GSI on the table is CREATING, with the service's LimitExceeded sentence (now ONLINE_INDEX_LIMIT, shared with the two-actions-in-one-request check). Tested on both.
  4. Query/Scan on a CREATING GSI. fetch_index_info_by_table_id on both backends treats a CREATING row as not found, which the engine already answers with ValidationException "The table does not have the specified index". That is the service's response, and it replaces the 500 on SQLite.
  5. UpdateItem wrong-typed GSI key. PostgreSQL's UpdateItem now runs the same post-update index key type and emptiness validation as SQLite and TransactWriteItems. Test added.
  6. CREATING status wording. The breaking-changes note now says SQLite already showed CREATING during its build, and Backfilling is listed as not sent, under "Not closed here".

Minor items: the recovery worker's log lines now carry table and index name. The ctid lock's doc comment now says plainly that no writer in this crate updates an index row in place today, so the single-statement form would not misbehave against them, and that the guard is for a writer that does; it costs one statement on a path taken only for an item that is gone. I left it in rather than remove a guard whose absence a future writer would discover the hard way. The backoff stays in-memory; it is per process and resets on restart, which for a 60 s to 1 h schedule seems acceptable, but say if you'd rather have it in the catalog.

@robinnsc
robinnsc force-pushed the fix/pg-online-gsi-backfill branch 2 times, most recently from 01b2288 to 542b6d9 Compare October 20, 2026 02:18
…s continue

Adding a GSI to a PostgreSQL table that was taking writes left the index
with missing, extra and stale rows, or failed UpdateTable with a duplicate
key. The index row was committed ACTIVE before its data table existed, the
backfill paged with OFFSET under concurrent writes, and queue rows applied
the image they carried, so a late row overwrote newer state.

UpdateTable now creates the data table first, publishes the index
CREATING, waits 1 s, takes a brief SHARE lock on the base table to wait out
in-flight writes, backfills in keyset pages under FOR SHARE SKIP LOCKED,
sweeps orphan rows, and flips the index ACTIVE. Writes maintain a CREATING
index like an ACTIVE one. The synchronous write path, the queue and the
backfill all write an item's current state, read under its row lock. A
write that took more than 500 ms from its catalog read re-reads the catalog
and queues a repair for any GSI it did not know about; that and the wait
cover writes that read the catalog just before the index was created, which
no transaction can see because the catalog and the data are separate
databases. A build that fails for a transient reason stays CREATING and a
background worker finishes it; one that would fail every time removes the
index.

On both PostgreSQL and SQLite the write path takes index attribute
definitions from its own catalog read instead of the cached key info, which
predated a new GSI's key attributes until UpdateTable returned, so a GSI
with a sort key on a new attribute got rows without that sort key.

While a GSI is CREATING, Query and Scan naming it answer as the service
does (no such index) rather than reading a partial table, and a second
GSI Create on the table is refused with LimitExceededException. DeleteTable
stops the propagation queue for the table before dropping anything, which
removes a deadlock with the queue worker that left orphan data tables; the
worker bounds its wait on a held base row to 2 s. UpdateItem on PostgreSQL
validates GSI key types on the updated item as PutItem does.

The new pytest is xfail on MongoDB (concurrent creates of one item return
HTTP 500 there regardless of indexes) and skipped on Cassandra (contended
deletes return HTTP 500 and the build did not finish in CI).

Signed-off-by: Scott Robinson <robinnsc@amazon.com>
@robinnsc
robinnsc force-pushed the fix/pg-online-gsi-backfill branch from 542b6d9 to 8c31762 Compare October 20, 2026 03:57

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants