Repository navigation
fix(postgres): keep a deleted table DELETING until its data tables drop - #504
LeeroyHannigan wants to merge 2 commits into
Conversation
…does A second DeleteTable and an UpdateTable during a table's DELETING window answer 400 ResourceInUseException with "Attempt to change a resource which is still in use: Table is being deleted: <name>" on all four backends. PutItem, GetItem, Query and Scan are served during the window on PostgreSQL and SQLite and answer ResourceNotFoundException once the table is gone; a write made in the window goes away with the table. Two concurrent DeleteTable calls on one ACTIVE table get one description and one ResourceInUseException on PostgreSQL, SQLite and MongoDB. All measured against the service on 2026-10-24 and 2026-10-25. DeleteTable on PostgreSQL and SQLite looked the name up with a status filter that excluded DELETING rows and answered ResourceNotFoundException. On MongoDB and Cassandra it had no status filter and ran the delete again. UpdateTable on every backend answered "Table <name> is not in ACTIVE state", the sentence for the other non-ACTIVE statuses. The data plane on PostgreSQL and SQLite answered ResourceNotFoundException for the whole window, because the key-info read that authorization and the handlers share mapped a DELETING row to not found. On SQLite the DeleteTable status lookup ran outside the write transaction and on MongoDB the DELETING mark was unconditioned, so two concurrent calls both proceeded. New StorageError::TableBeingDeleted, worded once in the engine's message table and mapped to ResourceInUseException by the DeleteTable, UpdateTable and TTL error mappers. The key-info read on PostgreSQL and SQLite reads a DELETING row as found; a statement or transaction that reaches a data table the control plane has dropped is answered as not found instead of as an internal error. SQLite's DeleteTable looks the row up inside its write transaction and fences the DELETING mark on the table id; MongoDB's mark is an update_one conditioned on the table id and status, and its final delete_one is keyed on both. On PostgreSQL and SQLite the DELETING row exists only on the asynchronous DeleteTable path (control_plane_delay_seconds of 1 or more; the default is 0.25), so on a default deployment the new answers are reachable after #504, which marks the row on both paths. On Cassandra DeleteTable never marks the row, so the check is reachable only for an abandoned restore target, and the data plane there is unchanged. MongoDB drops the collections inside the DeleteTable call a few milliseconds after the mark, so its data plane keeps answering not found during that window. The differences document carries the per-backend shape. Tests: engine unit tests for the three mappers; storage-level tests on each backend for both control-plane requests and for a CREATING table keeping its answer; on PostgreSQL and SQLite, for the four data-plane requests during the window, for a put, a get and the two transaction calls after the data table is dropped under a DELETING row, and for ten rounds of two concurrent DeleteTable calls (also on MongoDB); a dual-target wire test for the PostgreSQL case that holds the data table locked from a second connection, with the data-plane rows, which skips on a server whose delay is under 1 s until #504 lands. The PostgreSQL storage-level CI step runs the new binary. Closes #505
…e dropped DeleteTable on PostgreSQL deleted and committed the catalog row first and dropped the data tables afterwards, inside the request future. A client that gave up while the drop waited on a lock dropped the future and rolled the drop back, and a server killed between the two commits left the _ddb_ tables with no catalog row. Nothing removed them later: the startup pass acts on DELETING rows only. The row is now marked DELETING and committed on both control-plane paths, and the drop is done by the control-plane pass, which deletes the catalog row only after the data transaction has committed. The immediate path runs that pass for the one table in a spawned task and awaits it before answering, so a disconnect cannot cancel the drop. In the common case the name is free when the response arrives; when the drop waits past the pass's 3 s lock timeout, or is the victim of a deadlock with the GSI propagation worker, the response is DELETING, the task wakes the poller, and the poller finishes the drop on its next tick. A crash at any point leaves a DELETING row the startup pass finishes. The scoped pass waits for a poller pass that holds the row instead of skipping it, so the request answers only after that pass has committed. During the window DescribeTable reports TableStatus DELETING and CreateTable with the name answers ResourceInUseException with the service's sentence. Four storage tests on a scratch database cover the client disconnect, the process kill, the poller wake after a lock timeout and the scope of the inline pass; a wire test pins the two responses, holding the window open on ExtendDB with an outside lock. Closes #449
9b0fd87 to
0585b92
Compare
| AND ($1::TEXT IS NULL OR table_id = $1) \ | ||
| {lock_clause}" | ||
| ); | ||
| let candidates: Vec<(String, String, String)> = sqlx::query_as(&candidate_sql) |
There was a problem hiding this comment.
Where is the 3s timeout being applied here (FOR UPDATE will block indefinitely)?
There was a problem hiding this comment.
You're right, it wasn't. The 3 s was set inside drop_table_storage on the data-pool transaction, so it bounded the drops and not this select. The select waited on whichever transaction held the row. After DELETING commits the only thing that holds the row past one statement is a poller pass, which takes due DELETING rows with SKIP LOCKED and keeps the locks until it commits, so the wait was bounded by that pass's progress rather than by a timeout, and the comment overstated what the 3 s covered.
5f437d5 sets lock_timeout to the same 3 s on this transaction (one constant now, shared with the drop) before the select. On SQLSTATE 55P03 the pass rolls back and returns no transitions, so finish_table_delete gives Ok(false), DeleteTable answers with the table still DELETING and wakes the poller, which is the existing path for a drop that times out. The poller pass doesn't set it, since SKIP LOCKED doesn't wait.
New test delete_table_lock_wait: holds the row from a second connection, expects the pass to give up at about 3 s with the row still DELETING, then releases the row, runs the pass to completion and reads lock_timeout on every pooled connection, expecting 0. It fails with the set_config removed, with the value changed to 30 s, and with the setting made session-level (a committed pass then leaves 3 s on the pooled connection). The interrupt tests, fmt, clippy and the 1.88 build pass. Comments in worker_store.rs and delete_table.rs now say what bounds the wait.
Two related waits are on main and not in this PR: DeleteTable's first FOR UPDATE select has no lock timeout either and can wait behind CreateBackup's FOR SHARE, and with one database the scoped pass holds a catalog connection while the drop acquires a second one from the same pool, so pool_size concurrent DeleteTable calls all wait out the 30 s acquire timeout (measured: 24 calls at pool size 10, all answered after 30.07 s, rows finished by the poller afterwards). I'll file both.
jcshepherd
left a comment
jcshepherd
left a comment
There was a problem hiding this comment.
Catalog delete appears to be lacking a time-out on a pessimistic lock, but I could be wrong. Please verify.
The pass that DeleteTable runs for its own table selects the table's DELETING catalog row FOR UPDATE on a catalog transaction with no lock timeout. After DELETING commits, a control-plane poller pass can hold that row. The poller pass takes due DELETING rows with SKIP LOCKED and keeps the locks until it commits, after dropping the data tables of the rows it took. The 3 s lock timeout that the data drop sets applies to the data transaction only, so the select waited until the poller pass committed, and the spawned DeleteTable task held a catalog pool connection for that time. The scoped pass now sets lock_timeout to 3 s on its catalog transaction, the value the data drop uses, kept in one constant. When the select fails with lock_not_available (SQLSTATE 55P03), the pass rolls back and reports the table as not removed. DeleteTable then answers with the table still DELETING and wakes the poller, which finishes the delete. The timeout also bounds the later lock waits in that transaction; a timeout there is returned as an error and leaves the row DELETING for the poller. Other errors propagate as before. The poller pass does not set the timeout, since SKIP LOCKED does not wait for a row. finish_table_delete is public and hidden from the docs so that the new test delete_table_lock_wait can call it. The test holds the catalog row from a second connection and checks that the pass gives up after about 3 s with the row still DELETING; it then releases the row, runs the pass to completion, and reads lock_timeout on every pooled connection, which must be 0. The setting is transaction-local (set_config with is_local true); a session-level setting survives a committed transaction on that pooled connection and the read catches it. The integration workflow runs the new test in its PostgreSQL storage-level step.
Summary
On PostgreSQL,
DeleteTabledeleted and committed the catalog row first and dropped the_ddb_data tables afterwards, inside the request future. A client that gave up while the drop waited on a lock dropped the future and rolled the drop back; a server killed between the two commits left the data tables with no catalog row. Nothing removed them later, because the startup pass acts onDELETINGrows only. Both interruptions reproduced onmainwith the issue's recipe (anACCESS SHARElock held on the base data table): two orphan_ddb_tables each time, still present 75 s after a restart.The row is now marked
DELETINGand committed on both control-plane paths, and the drop runs in the control-plane pass, which deletes the catalog row only after the data transaction has committed. With the default delay the request runs that pass for its own table in a spawned task and awaits it, so a disconnect cannot cancel the drop. A crash at any point leaves aDELETINGrow the startup pass finishes (Recovered table 't': DELETING → deletedon restart in the reproduction).In the common case the name is free when the response arrives, as before. When the drop waits past the pass's 3 s
lock_timeout, or is the victim of a deadlock with the GSI propagation worker, the response isDELETING, the task wakes the poller, and the poller finishes the drop on its next tick. The scoped pass waits for a poller pass that holds the row instead of skipping it, so the request answers only after that pass has committed.What a client sees in the window
Measured on the service 2026-10-24 (two repeats, identical) and matched here:
DescribeTablereportsTableStatus DELETING;CreateTablewith the name answersResourceInUseExceptionTable already exists: <name>.Tests
crates/storage-postgres/tests/delete_table_interrupt.rs, four tests on a scratch database: the request future dropped while the drop waits (rowDELETING, both tables present,DescribeTableandCreateTableas above, then everything gone after the lock is released); the process killed while the drop waits (a second runtime dropped mid-request; the startup pass on a fresh engine finishes it); the poller woken after a lock timeout; the inline pass removing only its own table. 0 of 2 of the first two pass with the three fixed source files frommain. Mutants on the committed tree: catalog row deleted before the data commit, task not awaited,DELETINGcommitted after the spawn, wake removed, scope ignored; each fails at least one test.tests/python/test_delete_table_deleting_window.py:DeleteTableandDescribeTablereportDELETING;CreateTablewith the name answers the measured sentence. On ExtendDB the window is held open with an outsideACCESS SHARElock throughEXTENDDB_TEST_PG_ADMIN_CONNECTION_STRING(the PostgreSQL integration job exports it; the other jobs skip). Against the service the tests run on its own window.delete_table_interrupt.tests/python/333 passed on a server built from the branch.cargo fmt,clippy1.97.0 with-D warnings, build on 1.88.0: clean.git merge-tree: clean with Fix PostgreSQL global secondary index creation under concurrent writers #454. One conflict with fix(postgres): build a GSI added by UpdateTable correctly while writes continue #401 indelete_table.rs(its inline fast path callsdrop_table_storage; this branch removes the inline drop): resolution is this branch's file, verified by a trial merge on which the crate builds and both branches' tests pass. On the merged tree fix(postgres): build a GSI added by UpdateTable correctly while writes continue #401'sdrop_table_storageis called only from the pass, so its deadlock-safe drop order covers the request path too.Not in this PR
Pre-existing on
main, measured during this work and filed separately: aDeleteTableduring anUpdateTableGSI backfill orphans the new index data table (same statement order onmain; #401's base-table-first order closes it); the drop can deadlock with the GSI propagation worker (8 of 50 deletes under write load; #401 closes it); a secondDeleteTableor anUpdateTableduring the window answers a different sentence from the service;PutItemandGetItemon aDELETINGtable answerResourceNotFoundExceptionwhere the service serves them. This change does not sweep orphan tables an earlier release left behind.Closes #449
Rebase on #401
delete_table.rsis this branch's file, as the trial merge above describes; the doc comment on #401'sdrop_table_storagesaid "DeleteTable itself waits" and now says that DeleteTable's pass for its own table (finish_table_delete) uses it with the pass's lock timeout.