Repository navigation
fix: report Backfilling on a CREATING GSI and refuse its reads by phase - #454
Open
LeeroyHannigan wants to merge 11 commits into
Open
LeeroyHannigan wants to merge 11 commits into
LeeroyHannigan wants to merge 11 commits into
Conversation
LeeroyHannigan
requested review from
amrith,
c33howard,
jcshepherd,
pdf-amzn,
robinnsc and
yesyayen
as code owners
October 22, 2026 19:52
LeeroyHannigan
force-pushed
the
fix/pg-gsi-create-race
branch
from
October 24, 2026 00:22
7716312 to
8afad5f
Compare
This was referenced Oct 24, 2026
jcshepherd
reviewed
Oct 26, 2026
| if idx.index_status != "ACTIVE" || effective_delay(idx, system_default) != 0 { | ||
| return Ok(true); | ||
| } | ||
| index_has_queued_rows(tx, &idx.index_id).await |
Collaborator
There was a problem hiding this comment.
This will be invoked on every update to an active, sync GSI. How much overhead does it introduce?
jcshepherd
reviewed
Oct 26, 2026
| if index_type == IndexType::Gsi && index_status != "ACTIVE" { | ||
| return Err(StorageError::Validation(format!( | ||
| "{}{index_name}", | ||
| extenddb_core::types::VECTOR_INDEX_BACKFILLING_SCAN_PREFIX |
jcshepherd
previously approved these changes
Oct 26, 2026
jcshepherd
left a comment
Collaborator
There was a problem hiding this comment.
Question about performance for synchronous GSIs and nit on error messaging, but overall looks reasonable.
LeeroyHannigan
added a commit
that referenced
this pull request
Oct 29, 2026
On DynamoDB a GSI added by UpdateTable is refused to Query and Scan while it is CREATING and served once its backfill is done and it is ACTIVE (measured 2026-10-26 in us-east-1 on a 3000-item table). The table takes writes while the index builds, and the ACTIVE index has to include them. These two tests check that for the PostgreSQL build #401 made state-based. Each seeds 3000 rows, adds a KEYS_ONLY GSI with UpdateTable while four writers move index keys, insert rows and delete rows, waits for the propagation queue to drain, and checks that the index table equals the base projection row for row. Each runs ten iterations, one test with the propagation delay at 5 ms and the other at 0, where an ACTIVE index is maintained inline. Ported from #454 (gsi_creation_converges_under_concurrent_writers and its _zero_delay variant, with the helpers they use) into a new file. The UpdateTable input is built from JSON, so the file does not depend on the struct's field list.
LeeroyHannigan
force-pushed
the
fix/pg-gsi-create-race
branch
from
October 29, 2026 22:53
940b769 to
cea91ba
Compare
Two runs against DynamoDB on 2026-10-26 (us-east-1) added a GSI to a 3000-item table with UpdateTable. The index stayed CREATING for 512 s and 519 s. For about the first 30 s DescribeTable reported Backfilling false, and a Query or Scan naming the index got the ValidationException "The table does not have the specified index: <name>". For the rest of the build it reported Backfilling true, and both reads got the ValidationException "Cannot read from backfilling global secondary index: <name>". Once the index was ACTIVE the member was absent and both reads were served. Before this change no backend reported the member. PostgreSQL and SQLite answered a read of a CREATING GSI with the first sentence for the whole build, and MongoDB and Cassandra served the read from the partial index. GsiDescription gains a Backfilling member that is absent when None. Each backend answers one question, whether the backfill has started, and the storage crate's gsi_backfill module derives both the member and the refusal from that answer, so DescribeTable and a read report the same phase. Each backend takes the answer from its build: - PostgreSQL and SQLite build in the server process and record no backfill progress in the catalog, so the engine keeps the index ids whose backfill has started. PostgreSQL records an id when the build task reaches its backfill, after the publish wait and the write barrier. SQLite records it before UpdateTable opens the transaction that creates the index's data table and backfills it. The record is dropped after the ACTIVE flip or when the build gives up. - MongoDB's backfill worker sets backfilling on the index document before its first batch, and the ACTIVE flip removes it. Restored indexes report no member. - On Cassandra the recovery worker writes backfill_heartbeat when it starts work on the index, after checking the row is still CREATING and before it rebuilds the data table. A later scan of the worker then finds a fresh heartbeat and leaves the index to the running build. A new test setting, gsi_creating_phase_delay_ms:<table>, lengthens each phase by the given milliseconds, so a client can observe both on a small table. It is writable through the settings API (0 to 60000) and documented with the other test settings in the admin guide. Tests: a storage test per backend walks the three states and checks the refusal in each (Cassandra stages the catalog row, since its direct tests run no worker), and tests/python/test_gsi_backfilling.py checks the member set, both refusals byte for byte, and the ACTIVE state over the wire.
On DynamoDB a GSI added by UpdateTable is refused to Query and Scan while it is CREATING and served once its backfill is done and it is ACTIVE (measured 2026-10-26 in us-east-1 on a 3000-item table). The table takes writes while the index builds, and the ACTIVE index has to include them. These two tests check that for the PostgreSQL build #401 made state-based. Each seeds 3000 rows, adds a KEYS_ONLY GSI with UpdateTable while four writers move index keys, insert rows and delete rows, waits for the propagation queue to drain, and checks that the index table equals the base projection row for row. Each runs ten iterations, one test with the propagation delay at 5 ms and the other at 0, where an ACTIVE index is maintained inline. Ported from #454 (gsi_creation_converges_under_concurrent_writers and its _zero_delay variant, with the helpers they use) into a new file. The UpdateTable input is built from JSON, so the file does not depend on the struct's field list.
gsi_can_be_added_to_a_table_with_no_sort_key waited for the table to be ACTIVE after UpdateTable and then queried the new index. On MongoDB and Cassandra UpdateTable returns while the index is still CREATING, and a CREATING index now refuses reads as the service does. On MongoDB the first Query got "ValidationException: The table does not have the specified index: hashOnlyIdx" and the test failed. Before the CREATING phases were added, MongoDB served that read from the partial index and the test's loop polled until the row appeared. The test now waits for the index itself to be ACTIVE, the way the second test in the file and gsi_update_table_attr_defs.rs already do. No other test in tests/rust or tests/ reads a GSI that UpdateTable created before waiting for that index to be ACTIVE.
… failure
A PostgreSQL GSI build that fails for a passing reason (a conflict, a
timeout, a lost connection) leaves its index CREATING for the recovery
worker, and UpdateTable still succeeds. The backfill record was a guard
local to the build loop, so that failure dropped it: the index went back
from Backfilling true to false, and a Query or Scan went back from
"Cannot read from backfilling global secondary index: <name>" to "The
table does not have the specified index: <name>", until the recovery
worker's next pass, which runs every minute and backs off up to an hour
after another failure. The service is not known to go back a phase.
The record now stays until the index leaves CREATING. The build removes
it when it publishes the index, finds the index deleted or already
published, or when remove_failed_build deletes the index after a failure
that would recur. A passing failure returns with the record in place, and
the retry records the same id again. The set no longer counts builds of
one index: an id is removed only once no build of it is needed. SQLite
keeps a record scoped to its build, because a failed SQLite build deletes
its row in the same call.
Test: a_passing_build_failure_keeps_the_index_backfilling_until_the_retry_publishes_it
locks the index's catalog row so the flip to ACTIVE waits, cancels the
waiting statement (SQLSTATE 57014, which the build treats as passing),
then checks that the UpdateTable response, ten readings before the
retry, and the readings while reconcile_creating_gsis runs until ACTIVE
report Backfilling true with the backfilling refusal. On the previous
commit it fails at its first check: the UpdateTable response reports
("CREATING", false). The test file's setup moves into a helper shared by
both tests.
The admin guide lists gsi_creating_phase_delay_ms:<table> among the settings managed with `extenddb settings set`, but the command looked keys up in KNOWN_KEYS alone and refused it with "Unknown setting". Only the management API accepted the key, through its own check for a valid table name. ops_settings now has one lookup, writable_key_validator, that returns the validator for a KNOWN_KEYS entry or for a table's phase hold. The management API's set_setting and the CLI both use it, so they accept the same keys with the same bound (0 to 60000 ms); set_setting also accepts the MongoDB test-gate keys in builds with mongodb-test-hooks, as before. The CLI's checks move into a validate function so they can be tested without a store. Test: a_table_gsi_phase_hold_is_writable_from_the_cli accepts the key with 1000 and 0, refuses 60001 as an invalid value and a two-character table name as an unknown setting. It fails when validate looks keys up in KNOWN_KEYS alone, as the command did before.
PostgreSQL and SQLite keep the ids of GSIs whose backfill has started in the process, and DescribeTable and the read path paired that set with the index's catalog row. They read the row first and the set second. A build removes its id right after it flips the row to ACTIVE, so a call whose row read landed before the flip and whose set read landed after the removal saw a CREATING row with no record, and reported Backfilling false, or the first-phase refusal "The table does not have the specified index", for an index whose backfill had finished. GsiBackfills gains snapshot(), a copy of the set. DescribeTable, DeleteTable's response and the read path on both backends take it before they read the rows they pair it with. An id is removed only after its row has left CREATING, so a set read first can be paired only with the same row or a newer one, and a call never reports an earlier phase than the call before it. On PostgreSQL the test hold inside the second phase now runs between the backfill and the flip instead of before the backfill, so a test can read the index in the window at the end of the build. The second phase is as long as before: the hold plus the backfill. Test: the_index_stays_backfilling_from_the_end_of_its_backfill_to_the_flip waits until the index table holds the table's three items, then reads the index until it is ACTIVE and fails on any reading other than the second phase. It fails when the build removes its record before the flip (the mutant that inserts backfills.finish right after the backfill): "phase 1 after the backfill wrote every item". The walk test and the passing-failure test fail under that mutant too.
The MongoDB storage tests for the phases of a GSI that UpdateTable adds need a live replica set, so they are #[ignore]d like the other live tests and their names start with live_gsi_. devtools/run-mongodb-tests runs the crate's live tests before the Rust suite, the one place in CI with a replica set, but its filter was records_live alone, so CI never selected them. The filter is now `records_live live_gsi_`; libtest runs the tests whose names contain either. With the old filter, `--list` on that command shows none of the live_gsi_ tests; with the new one it lists them.
The Cassandra recovery claim and a MongoDB backfill resumed after a restart had no test. Cassandra: the recovery worker's claim writes backfill_heartbeat before it rebuilds the index's data table and starts the backfill, which is the start of the second phase, and a later scan that finds the heartbeat fresh leaves the index to the build already running. The direct tests run no worker, so workers.rs gains a hidden entry point, recover_stale_gsi_backfills_of_table, that runs one scan limited to one table's CREATING indexes; the limit keeps the scan off the rows the tests beside it stage. The worker itself passes no limit. the_recovery_claim_starts_the_second_phase_and_a_later_scan_leaves_the_build stages the row as UpdateTable inserts it, with the table's phase hold at 3000 ms, and runs one scan. It checks that the index then reports Backfilling true with the backfilling refusal, that the heartbeat's write time is earlier than the write time of the rebuilt data table's schema row, that a second scan during the hold leaves that table in place (same table id), and that the running build publishes the index. It fails when the claim writes no heartbeat. MongoDB: a job resumed after a restart finds the backfilling mark the dead process set (ttl_worker.rs) and goes straight to its scan. live_gsi_job_resumed_after_a_restart_keeps_its_second_phase adds the index with no worker running and a 30 s phase hold, sets the mark as a worker that died would have, then starts the worker. The test fails on any reading other than the second phase before and during the resumed job, and requires the index to be ACTIVE within 15 s. It fails when the resumed job starts its first phase again, which waits the hold twice. The live test file's setup and phase reading move into helpers shared with the existing test.
… keyspace
The Cassandra recovery worker runs its catalog scan once per account
keyspace, and the scan returns the CREATING indexes of every account.
Each scan rebuilt and backfilled every stale row in the keyspace it was
given. The claim this branch added writes backfill_heartbeat on the
first attempt, so when the worker listed another account's keyspace
first, that scan claimed the index, created its data table in the other
account's keyspace, and failed the backfill there ("table items_...
does not exist"). The scan of the index's own keyspace then found the
heartbeat fresh and skipped the index. Each later pass repeated this
once the heartbeat was older than the stale threshold, so the index
stayed CREATING with Backfilling true. Main has no claim, so every scan
tried the index and the one in its own keyspace built it.
The scan now reads the account that owns the base table from its tables
row, and leaves the row alone when that account's keyspace is not the
one being scanned. The check comes before the claim, so a scan of
another keyspace no longer writes the heartbeat, creates a data table or
takes a propagation hold for the index.
The other fix would build each row in its own account's keyspace from
whichever scan reaches it first. That makes the first scan of a pass do
all the work while the other scans repeat the catalog read and build
nothing, and when the claim's UPDATE fails, a later scan in the same pass
that reads the row before the backfill's first page writes a heartbeat
starts a second build of the index. With the skip, only the scan of the index's
own keyspace can take it up, once per pass, and the claim is written
only by the attempt that builds the index.
Test: a_recovery_pass_builds_each_index_in_its_own_account_keyspace
creates a table in each of two accounts, puts three items in each, adds
gidx to both with UpdateTable, and runs one pass limited to the two
tables: the first account's keyspace, then the second's. After the
first scan the second account's index must still report Backfilling
false, with no data table for it in the first keyspace. After the
second scan both indexes must become ACTIVE with three rows in their
data tables. On the previous commit it fails at the first check: the
second account's index reports ("CREATING", true). The claim test's
index id query moves into a helper the new test shares.
…the flip The SQLite phase-walk test held the second CREATING phase before the build, so no test read the index between the build's commit and the flip to ACTIVE. A build that ended its record there, with the row still CREATING, passed the test, although DescribeTable then reported the first phase. The second hold now runs after the build commits and before the flip, as on PostgreSQL, and a record dropped before the flip fails the walk test. The snapshot() doc comment and the build's comment said the answer never goes back to the first phase. On SQLite it does on two failure paths: when the flip to ACTIVE fails, or the cleanup DELETE after a failed build fails, the record ends while the row stays CREATING until the startup reconciler rebuilds it. Both comments now say so.
CodeQL reported the two-account recovery test as cleartext logging of sensitive information: the assertion message printed the table name of a helper built from the account id. The loop now names each account "first" or "second", and the message prints that label.
LeeroyHannigan
force-pushed
the
fix/pg-gsi-create-race
branch
from
October 30, 2026 10:42
3bb0c38 to
af36b96
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.
A GSI that UpdateTable adds now goes through the two CREATING phases the service has. DescribeTable reports Backfilling false and then true, and a Query or Scan naming the index gets the refusal for its phase.
I replaced this PR's branch on 29 October. #401 fixed the convergence bug its earlier commits addressed, and what remained of the PR was the two phases. jcshepherd's approval of 26 October and his two inline comments are on those earlier commits: this branch does not change
data/index.rs, and its change todata/ddl.rsis the index lookup for a read, which now refuses a CREATING index by phase.What the service does
Measured on the service on 2026-10-26 in us-east-1, in two runs. A GSI added by UpdateTable to a 3,000-item table stayed CREATING for 512 s on one run and 519 s on the other.
While the index is CREATING, DescribeTable lists nine members for it: Backfilling, IndexArn, IndexName, IndexSizeBytes, IndexStatus, ItemCount, KeySchema, Projection and ProvisionedThroughput.
What main does
GsiDescription has no Backfilling member, so no backend reports one. PostgreSQL and SQLite answer a Query or Scan of a CREATING GSI with "The table does not have the specified index" for the whole build. MongoDB and Cassandra serve the read from the partial index.
#401 already made PostgreSQL GSI creation converge under concurrent writers: the build is state-based, and a recovery worker finishes builds left CREATING. This change adds the two CREATING phases on top of that design without changing how the build orders its writes or when the recovery worker runs. This PR's earlier commits fixed the same convergence bug with a design of their own; their two convergence tests are ported here.
The change
GsiDescription gains Backfilling, absent when it is None. Each backend answers one question, whether the backfill of a CREATING index has started, and
extenddb_storage::gsi_backfillderives both the member and the refusal from that answer, so DescribeTable and a read report the same phase.backfillingon the index document before its first batch, and the flip to ACTIVE removes it. A job resumed after a restart finds the mark and goes straight to its scan.backfill_heartbeatwhen it takes the index up, before it rebuilds the data table. A later scan finds the heartbeat fresh and leaves the index to the build that is running. The worker scans the catalog once per account keyspace, and each scan returns the CREATING indexes of every account. A scan takes up only the indexes whose base table belongs to the keyspace's account, so the heartbeat is written only by the scan that builds the index. On main, a scan of another account's keyspace also tries the index, creates its data table in that keyspace and fails the backfill there.A test setting,
gsi_creating_phase_delay_ms:<table>, lengthens each phase by the given milliseconds so a client can observe both phases on a small table. It is writable through the management API and withextenddb settings set, from 0 to 60000, and the admin guide lists it with the other test settings.Two changes to existing tests and CI:
gsi_can_be_added_to_a_table_with_no_sort_keyintests/rustnow waits for its new index to be ACTIVE before it queries it, because a CREATING index refuses reads on MongoDB and Cassandra; anddevtools/run-mongodb-testsnow runs thelive_gsi_storage tests with therecords_liveones, so CI selects them.Tests
extenddb-storagegsi_backfill::tests(6 unit tests)a_gsi_added_by_update_table_walks_both_creating_phases("CREATING", None)a_passing_build_failure_keeps_the_index_backfilling_until_the_retry_publishes_it("CREATING", false)the_index_stays_backfilling_from_the_end_of_its_backfill_to_the_flipgsi_create_converge(2 tests)gsi_backfilling_tests::a_gsi_added_by_update_table_walks_both_creating_phases[3]instead of[1, 2, 3]. A build that drops its record before the flip:[1, 3]live_gsi_added_by_update_table_walks_both_creating_phases("CREATING", None)live_gsi_job_resumed_after_a_restart_keeps_its_second_phasea_creating_gsi_reports_each_phase_and_refuses_reads_by_phase("CREATING", None)the_recovery_claim_starts_the_second_phase_and_a_later_scan_leaves_the_builda_recovery_pass_builds_each_index_in_its_own_account_keyspace("CREATING", true)after the first scanops_settingsa_gsi_phase_hold_is_writable_for_one_valid_table_namecmd_settingsa_table_gsi_phase_hold_is_writable_from_the_cliextenddb settings setaccepts the hold key within the bound and refuses an invalid table nametests/python/test_gsi_backfilling.pyThe wire test passed against PostgreSQL, SQLite, MongoDB and Cassandra servers built from this branch. The full
tests/rustsuite ran against MongoDB and Cassandra servers built from this branch, the way the CI rust jobs run it, and passed 512 of 512 on each. The SQLite wire run used the final commit. The Cassandra runs used the commit before it, and the PostgreSQL and MongoDB runs an earlier build whose server code differs from that commit only in formatting and in the Cassandra recovery worker. All three were made before the branch was rebased onto #471, which changes IAM condition matching. Against the same Cassandra server, a script added an index with UpdateTable in each of two accounts, 8 s apart, in both orders, and both indexes became ACTIVE each time. One of the orders adds the first index to the account whose keyspace the recovery worker lists last.Rebase on #395
Rebased on main at
3051e500(#395) on 30 October. The head is nowaf36b967. Commits 1 and 6 changed; the other nine are unchanged.lib.rs:gsi_backfillsfollows the fields feat(storage): SnapshotEngine, a consistent table snapshot on every backend [backup/restore 4/4] #395 added toPostgresEngine(snapshot_open_slots,shared_data_pooland the test-hooksdrop_attempts), in the struct and innew().table_helpers.rs: feat(storage): SnapshotEngine, a consistent table snapshot on every backend [backup/restore 4/4] #395 moved the catalog reads intobuild_table_description_on, which its snapshot engine also calls. The backfill set is read at the top of that function, before thetablesread, so DescribeTable and the snapshot engine both read the set before the rows.delete_table.rs: feat(storage): SnapshotEngine, a consistent table snapshot on every backend [backup/restore 4/4] #395 moved the index-row read ahead of its new early return for a table already DELETING. The backfill set is read before that read, and both of feat(storage): SnapshotEngine, a consistent table snapshot on every backend [backup/restore 4/4] #395's description builds take it. This branch's own index-row read is gone, since it was the same query in the same transaction.delete_table.rsmerged without a conflict but did not compile: feat(storage): SnapshotEngine, a consistent table snapshot on every backend [backup/restore 4/4] #395's early return for a DELETING table builds a description, so it reads the backfill set as well.DeleteTable reads the backfill set only for the description in its response. When and how the table's data is dropped is #395's code, unchanged.
Runs on
af36b967on 30 October:-D warningsand check on the workspace, and check on Rust 1.88.0.--libtests in the nine crates this branch touches: 1,717 passed, none failed.gsi_backfilling3 passed,gsi_create_converge2,gsi_online_build12,gsi_stale_writer1, and feat(storage): SnapshotEngine, a consistent table snapshot on every backend [backup/restore 4/4] #395'stable_snapshot81, with the 2 that feat(storage): SnapshotEngine, a consistent table snapshot on every backend [backup/restore 4/4] #395 marks ignored.live_gsi_2 passed against a replica set, and Cassandragsi_backfilling::3 passed against a Cassandra 4.1 node.tests/python/test_gsi_backfilling.pypassed on SQLite and PostgreSQL servers built from this head.The paragraph under the Tests table describes runs on the branch before this rebase. Not repeated on
af36b967: the wire test on MongoDB and Cassandra servers, thetests/rustsuite, and the rest of CI's PostgreSQL step.Not tested