Skip to content

Commit 89c7a98

Browse files
committed
adapter: hydrate migrated builtin MVs before 0dt cut-over
Problem: A builtin schema migration using the `Replacement` mechanism hands the new deployment a fresh persist shard. Nothing writes that shard while the deployment is read-only, because `ComputeController::allow_writes` no-ops in read-only mode, so the MV's write frontier never advances and the 0dt readiness gate has to drop it and everything downstream of it from the caught-up check. The deployment then promotes with those collections unhydrated, and they all hydrate at once at cut-over, spiking catalog-server CPU and degrading catalog queries. Solution: The replacement shard is exclusively owned by this deployment, so the MV can write it while we are still read-only. Do that, and keep the MV in the readiness gate, so it hydrates before cut-over instead of at it. Previously we didn't do this for builtins derived from mz_catalog because catalog shard frontier was not held. However, as of 26.17 it is, so gate the behaviour on the version of the old leader. The behaviour is gated behind the `enable_0dt_hydrate_migrated_builtin_mvs` feature flag, on by default, a break-glass revert to the prior exclude-from-gate behaviour without a redeploy. Testing: - New environmentd integration test `test_0dt_migrated_builtin_mv_hydrates_before_promotion` boots a read-only generation with a forced `replacement` migration and asserts the migrated builtin MVs `mz_databases` and `mz_clusters` are readable before it reports `ReadyToPromote`, proving they hydrate before cut-over rather than at it. - Extended the `0dt` mzcompose workflow `builtin-schema-migrations-replacement` to read those MVs from the read-only generation before promotion, guarding the same invariant through a real cross-version upgrade.
1 parent c3e4ddd commit 89c7a98

15 files changed

Lines changed: 310 additions & 32 deletions

File tree

doc/developer/design/20251015_builtin_schema_migration.md

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,8 @@ Doing so requires no writes to durable state, and therefore doesn't interfere wi
5858

5959
In the subsequent read-only bootstrap phase, the process creates persist read and write handles using the new schema.
6060
Read handles perform transparent migration of any data updates that flow through them, so dataflow hydration can proceed using the new schema.
61-
Write handles only require a matching registered shard schema when writing batches, which is something a read-only process doesn't do.
61+
Write handles only require a matching registered shard schema when writing batches, which a read-only process performing schema evolution doesn't do: the shard is the leader's live one, so it stays read-only until promotion.
62+
(Shard replacement is different: there the read-only process does write batches, but to a shard it created for itself. See below.)
6263

6364
Once the read-only process gets promoted to a leader, and runs the builtin schema migration mechanism again, it this time registers the new schema with the persist shard.
6465
Doing so fences out any processes that planned to evolve the schema to an earlier version.
@@ -76,6 +77,10 @@ Notably, it ignores any entries at different versions or deploy generations, to
7677
Depending on the existing migration shard entries, the process either decides to use the existing replacement shard, or to create the replacement shard and write its ID into the migration shard, at the current version.
7778
It sets the new shard ID as the migrated collection's shard in its in-memory catalog and commences bootstrapping using the replacement shard.
7879

80+
Because this environment exclusively owns the replacement shard, the read-only process force-writes it during bootstrap rather than leaving it read-only until promotion.
81+
This lets a migrated builtin materialized view and its dependents hydrate before cut-over instead of all at once at cut-over.
82+
It is safe only for the self-owned replacement shard, never a shard the leader is still serving, which is why it applies to shard replacement and not schema evolution.
83+
7984
A leader process performing shard replacement performs the same steps as in read-only mode.
8085
Additionally, it cleans up durable state written by earlier versions and/or deploy generations by:
8186
- arranging for the previous shards used by the migrated storage collections to be finalized

misc/python/materialize/mzcompose/__init__.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -744,6 +744,7 @@ def get_default_system_parameters(
744744
"with_0dt_caught_up_check_cutoff",
745745
"enable_0dt_caught_up_replica_status_check",
746746
"enable_0dt_caught_up_stability_check",
747+
"enable_0dt_hydrate_migrated_builtin_mvs",
747748
"plan_insights_notice_fast_path_clusters_optimize_duration",
748749
"enable_expression_cache",
749750
"mz_metrics_lgalloc_map_refresh_interval",

misc/python/materialize/parallel_workload/action.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3289,6 +3289,7 @@ def __init__(
32893289
"with_0dt_caught_up_check_cutoff",
32903290
"with_0dt_caught_up_check_stability_period",
32913291
"enable_0dt_caught_up_stability_check",
3292+
"enable_0dt_hydrate_migrated_builtin_mvs",
32923293
"enable_statement_lifecycle_logging",
32933294
"enable_introspection_subscribes",
32943295
"plan_insights_notice_fast_path_clusters_optimize_duration",

misc/scratch/ember-scratch.json

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
{
2+
"name": "ember-scratch",
3+
"launch_script": "true",
4+
"instance_type": "c7a.12xlarge",
5+
"ami": "ami-0ecb62995f68bb549",
6+
"size_gb": 512,
7+
"tags": {}
8+
}

src/adapter-types/src/dyncfgs.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,16 @@ pub const WITH_0DT_CAUGHT_UP_CHECK_STABILITY_PERIOD: Config<Duration> = Config::
8080
"How long a cluster must continuously be caught-up and have all replicas healthy before it is considered ready to cut over during a 0dt deployment.",
8181
);
8282

83+
pub const ENABLE_0DT_HYDRATE_MIGRATED_BUILTIN_MVS: Config<bool> = Config::new(
84+
"enable_0dt_hydrate_migrated_builtin_mvs",
85+
true,
86+
"Write-enable replacement-migrated builtin materialized views while read-only during a 0dt \
87+
deployment, so they hydrate before cut-over and keep gating promotion. Emergency break-glass \
88+
flag: disabling reverts to excluding migrated MVs (and their dependents) from the caught-up \
89+
check, which lets promotion proceed with them unhydrated and hydrate at cut-over instead. \
90+
Only takes effect when the leader is new enough for the write to make progress.",
91+
);
92+
8393
/// Enable logging of statement lifecycle events in mz_internal.mz_statement_lifecycle_history.
8494
pub const ENABLE_STATEMENT_LIFECYCLE_LOGGING: Config<bool> = Config::new(
8595
"enable_statement_lifecycle_logging",
@@ -458,6 +468,7 @@ pub fn all_dyncfgs(configs: ConfigSet) -> ConfigSet {
458468
.add(&ENABLE_0DT_CAUGHT_UP_REPLICA_STATUS_CHECK)
459469
.add(&ENABLE_0DT_CAUGHT_UP_STABILITY_CHECK)
460470
.add(&WITH_0DT_CAUGHT_UP_CHECK_STABILITY_PERIOD)
471+
.add(&ENABLE_0DT_HYDRATE_MIGRATED_BUILTIN_MVS)
461472
.add(&ENABLE_STATEMENT_LIFECYCLE_LOGGING)
462473
.add(&ENABLE_INTROSPECTION_SUBSCRIBES)
463474
.add(&ENABLE_FRONTEND_SUBSCRIBES)

src/adapter/src/catalog.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -648,6 +648,7 @@ impl Catalog {
648648

649649
let OpenCatalogResult {
650650
catalog,
651+
last_seen_version: _,
651652
migrated_storage_collections_0dt: _,
652653
new_builtin_collections: _,
653654
builtin_table_updates: _,

src/adapter/src/catalog/open.rs

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ use mz_sql::session::user::{MZ_SYSTEM_ROLE_ID, SYSTEM_USER};
6161
use mz_sql::session::vars::{SessionVars, SystemVars, VarError, VarInput};
6262
use mz_storage_client::controller::{StorageMetadata, StorageTxn};
6363
use mz_storage_client::storage_collections::StorageCollections;
64+
use semver::Version;
6465
use tracing::{Instrument, info, warn};
6566
use uuid::Uuid;
6667

@@ -73,14 +74,26 @@ use crate::catalog::{BuiltinTableUpdate, Catalog, CatalogState, Config, is_reser
7374
pub struct InitializeStateResult {
7475
/// An initialized [`CatalogState`].
7576
pub state: CatalogState,
76-
/// A set of new shards that may need to be initialized (only used by 0dt migration).
77+
/// Items whose builtin schema migration allocated a fresh, self-owned persist shard. Only used
78+
/// by 0dt migration.
79+
///
80+
/// `Replacement`-migrated items only: derived by filtering to items with a global id in
81+
/// `MigrationRunResult::new_shards`, which only `migrate_replace` populates. `Evolution`
82+
/// migrates in place and reuses the leader's shard, so it never lands here. Read-only write
83+
/// paths depend on that: force-writing a shard while read-only is safe only because we
84+
/// exclusively own it (see `ComputeController::allow_writes_in_read_only`).
7785
pub migrated_storage_collections_0dt: BTreeSet<CatalogItemId>,
7886
/// A set of new builtin items.
7987
pub new_builtin_collections: BTreeSet<GlobalId>,
8088
/// A list of builtin table updates corresponding to the initialized state.
8189
pub builtin_table_updates: Vec<BuiltinTableUpdate>,
82-
/// The version of the catalog that existed before initializing the catalog.
83-
pub last_seen_version: String,
90+
/// The version of the binary that last committed catalog migrations, or `None` for a newly
91+
/// initialized catalog.
92+
///
93+
/// While this environment is read-only during a 0dt deployment, this is the version of the
94+
/// leader environment: the read-only catalog transaction is a savepoint, so our own bump of
95+
/// the setting never lands.
96+
pub last_seen_version: Option<Version>,
8497
/// A handle to the expression cache if it's enabled.
8598
pub expr_cache_handle: Option<ExpressionCacheHandle>,
8699
/// The global expressions that were cached in `expr_cache_handle`.
@@ -92,7 +105,10 @@ pub struct InitializeStateResult {
92105
pub struct OpenCatalogResult {
93106
/// An opened [`Catalog`].
94107
pub catalog: Catalog,
95-
/// A set of new shards that may need to be initialized.
108+
/// See [`InitializeStateResult::last_seen_version`].
109+
pub last_seen_version: Option<Version>,
110+
/// See [`InitializeStateResult::migrated_storage_collections_0dt`]; `Replacement`-migrated
111+
/// items only.
96112
pub migrated_storage_collections_0dt: BTreeSet<CatalogItemId>,
97113
/// A set of new builtin items.
98114
pub new_builtin_collections: BTreeSet<GlobalId>,
@@ -430,8 +446,7 @@ impl Catalog {
430446
.await;
431447
builtin_table_updates.extend(builtin_table_update);
432448

433-
let last_seen_version =
434-
get_migration_version(&txn).map_or_else(|| "new".into(), |v| v.to_string());
449+
let last_seen_version = get_migration_version(&txn);
435450

436451
let mz_authentication_mock_nonce =
437452
txn.get_authentication_mock_nonce().ok_or_else(|| {
@@ -453,7 +468,9 @@ impl Catalog {
453468
.await
454469
.map_err(|e| {
455470
Error::new(ErrorKind::FailedCatalogMigration {
456-
last_seen_version: last_seen_version.clone(),
471+
last_seen_version: last_seen_version
472+
.as_ref()
473+
.map_or_else(|| "new".to_string(), |v| v.to_string()),
457474
this_version: config.build_info.version,
458475
cause: e.to_string(),
459476
})
@@ -566,7 +583,7 @@ impl Catalog {
566583
migrated_storage_collections_0dt,
567584
new_builtin_collections,
568585
mut builtin_table_updates,
569-
last_seen_version: _,
586+
last_seen_version,
570587
expr_cache_handle,
571588
cached_global_exprs,
572589
uncached_local_exprs,
@@ -624,6 +641,7 @@ impl Catalog {
624641

625642
Ok(OpenCatalogResult {
626643
catalog,
644+
last_seen_version,
627645
migrated_storage_collections_0dt,
628646
new_builtin_collections,
629647
builtin_table_updates,

src/adapter/src/coord.rs

Lines changed: 62 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,8 @@ use mz_adapter_types::compaction::CompactionWindow;
9393
use mz_adapter_types::connection::ConnectionId;
9494
use mz_adapter_types::dyncfgs::FRONTEND_READ_THEN_WRITE;
9595
use mz_adapter_types::dyncfgs::{
96-
USER_ID_POOL_BATCH_SIZE, WITH_0DT_DEPLOYMENT_CAUGHT_UP_CHECK_INTERVAL,
96+
ENABLE_0DT_HYDRATE_MIGRATED_BUILTIN_MVS, USER_ID_POOL_BATCH_SIZE,
97+
WITH_0DT_DEPLOYMENT_CAUGHT_UP_CHECK_INTERVAL,
9798
};
9899
use mz_auth::password::Password;
99100
use mz_build_info::BuildInfo;
@@ -171,6 +172,7 @@ use mz_storage_types::sources::{IngestionDescription, SourceExport, Timeline};
171172
use mz_timestamp_oracle::{TimestampOracleConfig, WriteTimestamp};
172173
use mz_transform::dataflow::DataflowMetainfo;
173174
use opentelemetry::trace::TraceContextExt;
175+
use semver::Version;
174176
use serde::Serialize;
175177
use thiserror::Error;
176178
use timely::progress::{Antichain, Timestamp as _};
@@ -243,6 +245,17 @@ mod privatelink_status;
243245
mod sql;
244246
mod validity;
245247

248+
/// The oldest leader version against which a replacement-migrated builtin materialized view may
249+
/// write its new persist shard while this environment is still read-only.
250+
///
251+
/// Every builtin materialized view reads `mz_internal.mz_catalog_raw`, so its dataflow only makes
252+
/// progress up to the catalog shard's frontier. Holding that frontier at the current time is the
253+
/// leader's job, and leaders only started doing it in v26.17. Write-enable such an MV against an
254+
/// older leader and it sits at a stale frontier and never reports caught up, which blocks
255+
/// promotion outright instead of merely leaving the collection cold at cut-over. We still support
256+
/// upgrading from before v26.17, so that leader is a real case, not a hypothetical.
257+
const MIN_LEADER_VERSION_FOR_MIGRATED_MV_WRITES: Version = Version::new(26, 17, 0);
258+
246259
/// A pool of pre-allocated user IDs to avoid per-DDL persist writes.
247260
///
248261
/// IDs in the range `[next, upper)` are available for allocation.
@@ -2445,6 +2458,7 @@ impl Coordinator {
24452458
&mut self,
24462459
boot_ts: Timestamp,
24472460
migrated_storage_collections_0dt: BTreeSet<CatalogItemId>,
2461+
hydrate_migrated_mvs: bool,
24482462
mut builtin_table_updates: Vec<BuiltinTableUpdate>,
24492463
cached_global_exprs: BTreeMap<GlobalId, GlobalExpressions>,
24502464
uncached_local_exprs: BTreeMap<GlobalId, LocalExpressions>,
@@ -2773,7 +2787,22 @@ impl Coordinator {
27732787
// If this is a replacement MV, it must remain read-only until the replacement
27742788
// gets applied.
27752789
if mview.replacement_target.is_none() {
2776-
self.allow_writes(mview.cluster_id, mview.global_id_writes());
2790+
let gid = mview.global_id_writes();
2791+
if hydrate_migrated_mvs
2792+
&& migrated_storage_collections_0dt.contains(&entry.id())
2793+
{
2794+
// `migrated_storage_collections_0dt` is `Replacement`-migrated items
2795+
// only, so this is a fresh shard we own: nothing else writes it, and
2796+
// writing it while read-only hydrates the MV and its dependents before
2797+
// cut-over. An `Evolution`-migrated MV reuses the leader's live shard
2798+
// and must never reach here.
2799+
self.controller
2800+
.compute
2801+
.allow_writes_in_read_only(mview.cluster_id, gid)
2802+
.unwrap_or_terminate("allow_writes cannot fail");
2803+
} else {
2804+
self.allow_writes(mview.cluster_id, gid);
2805+
}
27772806
}
27782807
}
27792808
CatalogItem::MetricSink(metric_sink) => {
@@ -5021,6 +5050,7 @@ pub fn serve(
50215050
;
50225051
let OpenCatalogResult {
50235052
mut catalog,
5053+
last_seen_version,
50245054
migrated_storage_collections_0dt,
50255055
new_builtin_collections,
50265056
builtin_table_updates,
@@ -5076,6 +5106,22 @@ pub fn serve(
50765106
catalog_open_start.elapsed()
50775107
);
50785108

5109+
// Whether replacement-migrated builtin MVs may write their new shards before cut-over.
5110+
// Both `bootstrap` and the readiness gate below read this, and they have to agree.
5111+
// `MIN_LEADER_VERSION_FOR_MIGRATED_MV_WRITES` explains why the leader's version settles it.
5112+
//
5113+
// While we are read-only, `last_seen_version` is that leader's version: our catalog
5114+
// transaction is a savepoint, so our own bump of the setting never lands. `None` means a
5115+
// freshly initialized catalog, with nothing migrated and no leader to be compatible with.
5116+
//
5117+
// `ENABLE_0DT_HYDRATE_MIGRATED_BUILTIN_MVS` is the break-glass revert: off falls back to
5118+
// excluding migrated MVs from the caught-up gate, no redeploy needed.
5119+
let hydrate_migrated_mvs = ENABLE_0DT_HYDRATE_MIGRATED_BUILTIN_MVS
5120+
.get(catalog.system_config().dyncfgs())
5121+
&& last_seen_version
5122+
.as_ref()
5123+
.is_none_or(|version| *version >= MIN_LEADER_VERSION_FOR_MIGRATED_MV_WRITES);
5124+
50795125
let coord_thread_start = Instant::now();
50805126
info!("startup: coordinator init: coordinator thread start beginning");
50815127

@@ -5126,18 +5172,16 @@ pub fn serve(
51265172

51275173
// A collection that can't advance its write frontier in read-only mode
51285174
// stalls its transitive dependents too, so exclude those from the caught-up
5129-
// check as well. That's migrated MVs (their dataflows don't write in
5130-
// read-only mode) and new builtin MVs (their fresh shard has no writer until
5131-
// this deployment promotes). An excluded dependent may still be hydrating
5132-
// right after promotion, a brief blip we accept because these MVs are small
5133-
// and get a writer at cut-over.
5175+
// check as well. That's new builtin MVs, whose fresh shard has no writer until
5176+
// this deployment promotes, plus migrated MVs whenever the leader is too old for
5177+
// them to write. An excluded dependent may still be hydrating right after
5178+
// promotion, a brief blip we accept because these MVs are small and get a writer
5179+
// at cut-over.
51345180
//
5135-
// TODO: Consider sending `allow_writes` for the dataflows of migrated MVs, which
5136-
// would allow them to make progress even in read-only mode. This doesn't
5137-
// work for MVs based on `mz_catalog_raw`, if the leader's version is less
5138-
// than v26.17, since before that version the catalog shard's frontier wasn't
5139-
// kept up-to-date with the current time. So this workaround has to remain in
5140-
// place upgrades from a version less than v26.17 are no longer supported.
5181+
// A migrated builtin *table* needs no such treatment even though a builtin MV can
5182+
// read one (`mz_clusters` joins `mz_cluster_replica_size_internal`):
5183+
// `read_only_mode_table_worker` keeps advancing the uppers of migrated tables, so
5184+
// an MV over one still catches up.
51415185
let new_builtin_mvs = new_builtin_collections
51425186
.iter()
51435187
.map(|global_id| {
@@ -5148,12 +5192,12 @@ pub fn serve(
51485192
})
51495193
.filter(|entry| entry.is_materialized_view())
51505194
.map(|entry| entry.id());
5151-
let mut todo: Vec<_> = migrated_storage_collections_0dt
5195+
let frozen_migrated_mvs = migrated_storage_collections_0dt
51525196
.iter()
51535197
.copied()
5154-
.filter(|id| catalog.state().get_entry(id).is_materialized_view())
5155-
.chain(new_builtin_mvs)
5156-
.collect();
5198+
.filter(|_| !hydrate_migrated_mvs)
5199+
.filter(|id| catalog.state().get_entry(id).is_materialized_view());
5200+
let mut todo: Vec<_> = new_builtin_mvs.chain(frozen_migrated_mvs).collect();
51575201
while let Some(item_id) = todo.pop() {
51585202
let entry = catalog.state().get_entry(&item_id);
51595203
exclude_collections.extend(entry.global_ids());
@@ -5326,6 +5370,7 @@ pub fn serve(
53265370
.bootstrap(
53275371
boot_ts,
53285372
migrated_storage_collections_0dt,
5373+
hydrate_migrated_mvs,
53295374
builtin_table_updates,
53305375
cached_global_exprs,
53315376
uncached_local_exprs,

0 commit comments

Comments
 (0)