-
Notifications
You must be signed in to change notification settings - Fork 513
adapter: hydrate migrated builtin MVs before 0dt cut-over #38399
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -90,6 +90,19 @@ pub const WITH_0DT_CAUGHT_UP_CHECK_STABILITY_PERIOD: Config<Duration> = Config:: | |
| ParameterScope::Environment, | ||
| ); | ||
|
|
||
| pub const ENABLE_0DT_HYDRATE_MIGRATED_BUILTIN_MVS: Config<bool> = Config::new( | ||
| "enable_0dt_hydrate_migrated_builtin_mvs", | ||
| true, | ||
| "Write-enable replacement-migrated builtin materialized views while read-only during a 0dt \ | ||
| deployment, so they hydrate before cut-over and keep gating promotion. Emergency break-glass \ | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. On top of finding 3 of the QA LLM review (boot-time capture, restart required, worth
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I updated the description of the flag. I think we should defer LD?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok to defer LD, but then let's add it to Edit: Although, I just realized that Dennis has disabled |
||
| flag: disabling excludes migrated MVs (and their dependents) from the caught-up check again, \ | ||
| so promotion proceeds with them unhydrated. Not an exact revert: a collection with no live \ | ||
| leader frontier must be hydrated either way. Only takes effect when the leader is new enough \ | ||
| for the write to make progress, and is read once at startup, so changing it means setting it \ | ||
| on the leader and restarting the new deployment.", | ||
| ParameterScope::Environment, | ||
| ); | ||
|
|
||
| /// Enable logging of statement lifecycle events in mz_internal.mz_statement_lifecycle_history. | ||
| pub const ENABLE_STATEMENT_LIFECYCLE_LOGGING: Config<bool> = Config::new( | ||
| "enable_statement_lifecycle_logging", | ||
|
|
@@ -511,6 +524,7 @@ pub fn all_dyncfgs(configs: ConfigSet) -> ConfigSet { | |
| .add(&ENABLE_0DT_CAUGHT_UP_REPLICA_STATUS_CHECK) | ||
| .add(&ENABLE_0DT_CAUGHT_UP_STABILITY_CHECK) | ||
| .add(&WITH_0DT_CAUGHT_UP_CHECK_STABILITY_PERIOD) | ||
| .add(&ENABLE_0DT_HYDRATE_MIGRATED_BUILTIN_MVS) | ||
| .add(&ENABLE_STATEMENT_LIFECYCLE_LOGGING) | ||
| .add(&ENABLE_INTROSPECTION_SUBSCRIBES) | ||
| .add(&ENABLE_FRONTEND_SUBSCRIBES) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This paragraph states the force-write unconditionally. Worth mentioning the two
conditions the code has: the old leader must be at v26.17 or newer (it has to keep the
catalog shard's frontier advancing, see
MIN_LEADER_VERSION_FOR_MIGRATED_MV_WRITES),and the
enable_0dt_hydrate_migrated_builtin_mvsbreak-glass, with exclusion from thecaught-up gate as the fallback in both cases.