adapter: apply replacement: wait for cutover time #34719
Draft
+366
−86
Conversation
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
The previous apply update sorting logic assumed that ID order equals dependency order for "derived items". That's not correct anymore: An altered materialized view can depend on items with greater catalog IDs. To ensure Materialize doesn't panic during startup when trying to parse such altered materialized views, this commit changes the item update sorting to use a topological sort for derived items instead.
The topological sorting in apply was using an inefficient quadratic algorithm. That was fine when we were only sorting connection items but is not fine anymore now that all most catalog items need to be sorted topologically. This commit changes the topo sort implementation to use Kahn's algorithm (https://en.wikipedia.org/wiki/Topological_sorting#Kahn's_algorithm), which does the job in time linear in the number of items and dependency edges.
This commit implements a piece of missing sequencing logic for `ALTER MATERIALIZED VIEW ... APPLY REPLACEMENT ...`: Before cutting over to the new dataflow we need to make sure that the old one has caught up to the new one. Otherwise the new one would start writing by appending an empty batch to bridge the gap, which could cause consistency violations. The strategy chosen here is the same as what we already do for `ALTER SINK`. We split the sequencing into a prepare and a finish step. In the prepare step, we install a watch set that waits for the target MV's dataflow to catch up. When the watch set completes, we run the finish step which applies the replacement. To ensure the waiting doesn't block all concurrent DDL, we need to exempt the ALTER MV sequencing from the global DDL lock. In the finish step we have to make sure that the replacement plan we're going to apply is still valid, i.e., neither the target nor the replacement has been dropped in the meantime.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
This commit implements a piece of missing sequencing logic for
ALTER MATERIALIZED VIEW ... APPLY REPLACEMENT ...: Before cutting over to the new dataflow we need to make sure that the old one has caught up to the new one. Otherwise the new one would start writing by appending an empty batch to bridge the gap, which could cause consistency violations.The strategy chosen here is the same as what we already do for
ALTER SINK. We split the sequencing into a prepare and a finish step. In the prepare step, we install a watch set that waits for the target MV's dataflow to catch up. When the watch set completes, we run the finish step which applies the replacement.To ensure the waiting doesn't block all concurrent DDL, we need to exempt the ALTER MV sequencing from the global DDL lock. In the finish step we have to make sure that the replacement plan we're going to apply is still valid, i.e., neither the target nor the replacement has been dropped in the meantime.
Motivation
Fixes https://github.com/MaterializeInc/database-issues/issues/9980
(Kind of... It's expected behavior, so this provides a useful error instead of really fixing anything.)
Closes https://github.com/MaterializeInc/database-issues/issues/9944
Tips for reviewer
Checklist
$T ⇔ Proto$Tmapping (possibly in a backwards-incompatible way), then it is tagged with aT-protolabel.