adapter: revalidate OR REPLACE drop set in staged create finish#37652
Draft
ggevay wants to merge 1 commit into
Draft
adapter: revalidate OR REPLACE drop set in staged create finish#37652ggevay wants to merge 1 commit into
ggevay wants to merge 1 commit into
Conversation
CREATE OR REPLACE [MATERIALIZED] VIEW computes the set of items to drop (the item being replaced) at planning time, but sequencing is staged and commits that set only in the finish stage, after off-thread optimization. Most DDL cannot interleave because it queues on the coordinator DDL lock, but statements with off-thread purification (e.g. CREATE SINK) are exempt and only check that their dependencies still exist. A sink created on the replaced materialized view could therefore commit inside the window. The finish stage then dropped the old item anyway, leaving the sink with dependency edges to a missing item and panicking the coordinator catalog consistency check (SQL-521). The finish stages now recompute the replaced item dependent closure against the current catalog and abort with a retryable error if it changed since planning. This also covers the item being replaced disappearing entirely, e.g. through a concurrent ALTER MATERIALIZED VIEW APPLY REPLACEMENT, which is likewise exempt from the DDL lock. Add a create_materialized_view_optimize failpoint and a deterministic regression test in test/race-condition, wired into the Nightly pipeline. 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-521
Important
Statistical verification with the Parallel Workload cancel scenario (the
reproducer from SQL-521, on PR #37590's branch) was deliberately deferred
and remains TODO before merging. Verification so far is the deterministic
failpoint-based repro and the new regression workflow, validated in both
directions (fails before the fix, passes after).
Motivation
CREATE OR REPLACE [MATERIALIZED] VIEWcomputes the set of items to drop(the item being replaced) at planning time, but sequencing is staged: the
optimize stage runs off the coordinator thread and the drop set is only
committed later, in the finish stage. Most DDL cannot interleave because it
queues on the coordinator's DDL lock, but statements with off-thread
purification (e.g.
CREATE SINK, seemust_serialize_ddl) are exempt fromthat lock and only check that their own dependencies still exist. A sink
created on the materialized view being replaced could therefore commit inside
the window. The finish stage then dropped the old item anyway, leaving the
sink with
uses/referencesedges to a missing item. The catalog consistencycheck panics the coordinator on that very transact:
(The
MissingUsesentry appears twice because the sink'sreferences()anduses()both contain the dropped item id.)The same window also allowed the item being replaced to disappear entirely,
via a concurrent
ALTER MATERIALIZED VIEW ... APPLY REPLACEMENT(likewiseexempt from the DDL lock), in which case the finish stage would panic while
generating the drop ops.
Description
User-facing change: when a new dependent (e.g. a
CREATE SINK) commitsconcurrently with a
CREATE OR REPLACE MATERIALIZED VIEW/CREATE OR REPLACE VIEW, theCREATE OR REPLACEstatement now fails with a clean,retryable error instead of corrupting the catalog and panicking the
coordinator. If the item being replaced was concurrently dropped, the
statement now fails with a "was dropped" error instead of panicking.
Coordinator::validate_or_replace_drop_ids, called at the top ofcreate_materialized_view_finishandcreate_view_finish(in the samemessage handler that commits the ops, so no further DDL can interleave). It
recomputes the replaced item's dependent closure against the current
catalog and compares it with the plan-time drop set:
AdapterError::ChangedPlan("the set of objects that depend on <kind> <name> changed while the statement was executing")AdapterError::ConcurrentDependencyDropcreate_materialized_view_optimizefailpoint in the off-threadoptimize stage.
test/race-conditionworkflowcreate-or-replace-race, which parks aCREATE OR REPLACE MATERIALIZED VIEWat the failpoint, commits aCREATE SINKon the replaced MV insidethe window, and asserts the replacement aborts cleanly with the sink and
the old definition intact. Wired into Nightly as the
race-condition-create-or-replacestep.Verification
create-or-replace-raceworkflow passes with the fix and correctlyfails (detecting the coordinator panic) with the finish-side check
disabled.
main(CREATE SINK purificationcommitting during a slow off-thread optimize of the OR REPLACE) reproduced
the exact SQL-521 panic signature, and produces the new clean error with
the fix.
test/sqllogictest/materialized_views.slt,replacement-materialized-views.slt,rbac_views.sltpass (normalOR REPLACE and replacement paths unchanged).
--scenario=cancel --complexity=ddl --threads=8on the PR parallel-workload: broaden SQL-surface and action coverage #37590 branch, which adds theCreateOrReplaceViewaction) to statistically confirm the panic is gone.Note that PR parallel-workload: broaden SQL-surface and action coverage #37590 will then need "changed while the statement was
executing" and "was dropped" in
CreateOrReplaceViewAction. errors_to_ignore.🤖 Generated with Claude Code