adapter: fail cleanly when a staged CREATE races with DROP ... CASCADE#37650
Draft
ggevay wants to merge 1 commit into
Draft
adapter: fail cleanly when a staged CREATE races with DROP ... CASCADE#37650ggevay wants to merge 1 commit into
ggevay wants to merge 1 commit into
Conversation
Fixes a coordinator panic (SQL-518) seen in Parallel Workload: CREATE SECRET is staged, and its ensure stage writes the secret to the secret store in an off-thread task. CREATE SECRET is exempt from DDL serialization, so a concurrent DROP DATABASE ... CASCADE can commit inside that window. The finish stage then runs a catalog transaction whose Op::CreateItem names a schema in the dropped database, and transact_op panicked resolving the new item's full name against the missing database (OrdMap::index: invalid key in CatalogState::get_database). The CREATE SECRET PlanValidity tracks no dependencies, so nothing caught the drop before the transaction. Fix at the transact_op level rather than in PlanValidity, so it covers every statement that can reach Op::CreateItem with a stale name: CREATE SECRET, CREATE CONNECTION (off-thread validation), and CREATE SOURCE / SINK / TABLE FROM SOURCE (off-thread purification). Op::CreateItem now revalidates that the item's target database and schema still exist and returns AdapterError::ConcurrentDependencyDrop otherwise. This also guards insert_user_item, which would happily write an item under a dangling schema id if name resolution didn't panic first, and covers the analogous dropped-schema case one line later in resolve_full_name. Testing: * a create_secret_between_ensure_and_finish failpoint plus a race-condition workflow (create-secret-db-drop-race, wired into Nightly) that deterministically reproduces the original panic and now verifies the clean error, * a unit test for the new transact_op check. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
Fixes SQL-518
Motivation
Nightly Parallel Workload hit a coordinator panic (
OrdMap::index: invalid keyinCatalogState::get_database, reached viaresolve_full_nameinCatalog::transact_op). CREATE SECRET is staged: its ensure stage writes the secret to the secret store in an off-thread task, and only the finish stage transacts the catalog. CREATE SECRET is exempt from DDL serialization, so a concurrentDROP DATABASE ... CASCADEcan commit inside that window. The finishingOp::CreateItemthen names a schema in the dropped database and the coordinator panics resolving the new item's full name. The CREATE SECRETPlanValiditytracks no dependencies, so nothing catches the drop before the transaction. The same window exists forDROP SCHEMA ... CASCADE(via the schema lookup one step later) and for every other statement that reachesOp::CreateItemwithout holding the DDL lock: CREATE CONNECTION (off-thread validation) and CREATE SOURCE / SINK / TABLE FROM SOURCE (off-thread purification).Description
User-facing change: a staged CREATE statement (CREATE SECRET, CREATE CONNECTION, CREATE SOURCE, CREATE SINK, CREATE TABLE ... FROM SOURCE) that races with a concurrent
DROP DATABASE ... CASCADEorDROP SCHEMA ... CASCADEof its target now returns a cleandatabase 'uN' was dropped/schema 'uN' was droppederror instead of crashingenvironmentd.The fix is at the
transact_oplevel rather than inPlanValidity, so one check covers the whole statement class.Op::CreateItemnow revalidates that the item's target database and schema still exist and returnsAdapterError::ConcurrentDependencyDropotherwise, consistent with the errorPlanValidityraises for concurrently dropped dependencies. The check must sit beforeinsert_user_item: the durable catalog performs no referential validation, so without it a fallible name lookup would durably commit an item under a dangling schema id (previously only the panic prevented that).Also adds a
create_secret_between_ensure_and_finishfailpoint (mirroring the existing rotate-keys one) and a deterministic regression workflowcreate-secret-db-drop-raceintest/race-condition, wired into Nightly as a new steprace-condition-create-secret-db-drop.Verification
database 'u2' was dropped, the coordinator survives, and the sanity restart finds no panics.test_create_item_into_dropped_database_or_schemaexercises thetransact_opcheck directly for both the missing-database and missing-schema cases.catalog::transactandcoord::validityunit test suites, plustest/sqllogictest/secret.sltandschemas.slt, pass locally.🤖 Generated with Claude Code