Skip to content

Commit ed7294b

Browse files
authored
adapter: hydrate migrated builtin MVs before 0dt cut-over (#38399)
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. 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 d7b0d9e commit ed7294b

16 files changed

Lines changed: 482 additions & 46 deletions

File tree

doc/developer/design/20251015_builtin_schema_migration.md

Lines changed: 11 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,15 @@ 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+
84+
The force-write is conditional on two things.
85+
First, the leader must be at v26.17 or later, because every builtin materialized view reads the catalog shard and only leaders from that version on keep its frontier advancing with the current time; against an older leader the dataflow would sit at a stale frontier.
86+
Second, the `enable_0dt_hydrate_migrated_builtin_mvs` feature flag must be on; it exists as a break-glass revert.
87+
When either condition does not hold, the migrated materialized views and their dependents are instead excluded from the 0dt caught-up check, which is the older behaviour: promotion proceeds without them and they hydrate at cut-over.
88+
7989
A leader process performing shard replacement performs the same steps as in read-only mode.
8090
Additionally, it cleans up durable state written by earlier versions and/or deploy generations by:
8191
- 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
@@ -750,6 +750,7 @@ def get_default_system_parameters(
750750
"with_0dt_caught_up_check_cutoff",
751751
"enable_0dt_caught_up_replica_status_check",
752752
"enable_0dt_caught_up_stability_check",
753+
"enable_0dt_hydrate_migrated_builtin_mvs",
753754
"plan_insights_notice_fast_path_clusters_optimize_duration",
754755
"enable_expression_cache",
755756
"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
@@ -3284,6 +3284,7 @@ def __init__(
32843284
"with_0dt_caught_up_check_cutoff",
32853285
"with_0dt_caught_up_check_stability_period",
32863286
"enable_0dt_caught_up_stability_check",
3287+
"enable_0dt_hydrate_migrated_builtin_mvs",
32873288
"enable_statement_lifecycle_logging",
32883289
"enable_introspection_subscribes",
32893290
"plan_insights_notice_fast_path_clusters_optimize_duration",

src/adapter-types/src/dyncfgs.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,19 @@ pub const WITH_0DT_CAUGHT_UP_CHECK_STABILITY_PERIOD: Config<Duration> = Config::
9090
ParameterScope::Environment,
9191
);
9292

93+
pub const ENABLE_0DT_HYDRATE_MIGRATED_BUILTIN_MVS: Config<bool> = Config::new(
94+
"enable_0dt_hydrate_migrated_builtin_mvs",
95+
true,
96+
"Write-enable replacement-migrated builtin materialized views while read-only during a 0dt \
97+
deployment, so they hydrate before cut-over and keep gating promotion. Emergency break-glass \
98+
flag: disabling excludes migrated MVs (and their dependents) from the caught-up check again, \
99+
so promotion proceeds with them unhydrated. Not an exact revert: a collection with no live \
100+
leader frontier must be hydrated either way. Only takes effect when the leader is new enough \
101+
for the write to make progress, and is read once at startup, so changing it means setting it \
102+
on the leader and restarting the new deployment.",
103+
ParameterScope::Environment,
104+
);
105+
93106
/// Enable logging of statement lifecycle events in mz_internal.mz_statement_lifecycle_history.
94107
pub const ENABLE_STATEMENT_LIFECYCLE_LOGGING: Config<bool> = Config::new(
95108
"enable_statement_lifecycle_logging",
@@ -511,6 +524,7 @@ pub fn all_dyncfgs(configs: ConfigSet) -> ConfigSet {
511524
.add(&ENABLE_0DT_CAUGHT_UP_REPLICA_STATUS_CHECK)
512525
.add(&ENABLE_0DT_CAUGHT_UP_STABILITY_CHECK)
513526
.add(&WITH_0DT_CAUGHT_UP_CHECK_STABILITY_PERIOD)
527+
.add(&ENABLE_0DT_HYDRATE_MIGRATED_BUILTIN_MVS)
514528
.add(&ENABLE_STATEMENT_LIFECYCLE_LOGGING)
515529
.add(&ENABLE_INTROSPECTION_SUBSCRIBES)
516530
.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/catalog/open/builtin_schema_migration.rs

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,9 @@ use futures::future::BoxFuture;
3737
use mz_build_info::{BuildInfo, DUMMY_BUILD_INFO};
3838
use mz_catalog::builtin::{
3939
BUILTIN_LOOKUP, Builtin, Fingerprint, MZ_CATALOG_RAW, MZ_CATALOG_RAW_DESCRIPTION,
40-
MZ_OBJECT_ARRANGEMENT_SIZE_HISTORY_DESCRIPTION, MZ_STORAGE_USAGE_BY_SHARD,
41-
MZ_STORAGE_USAGE_BY_SHARD_DESCRIPTION, RUNTIME_ALTERABLE_FINGERPRINT_SENTINEL,
40+
MZ_CLUSTER_REPLICA_FRONTIERS_DESCRIPTION, MZ_OBJECT_ARRANGEMENT_SIZE_HISTORY_DESCRIPTION,
41+
MZ_STORAGE_USAGE_BY_SHARD, MZ_STORAGE_USAGE_BY_SHARD_DESCRIPTION,
42+
RUNTIME_ALTERABLE_FINGERPRINT_SENTINEL,
4243
};
4344
use mz_catalog::config::BuiltinItemMigrationConfig;
4445
use mz_catalog::durable::objects::SystemObjectUniqueIdentifier;
@@ -758,6 +759,15 @@ impl Migration {
758759
"mz_catalog_raw cannot be migrated"
759760
);
760761

762+
// The 0dt caught-up gate reads the leader's `mz_cluster_replica_frontiers` shard for
763+
// the live frontiers it checks every collection against. Migrating it via `Replacement`
764+
// hands us a fresh shard we write ourselves, so the gate would compare us against
765+
// ourselves instead of against the leader.
766+
assert_ne!(
767+
&*MZ_CLUSTER_REPLICA_FRONTIERS_DESCRIPTION, object,
768+
"mz_cluster_replica_frontiers cannot be migrated or else the 0dt caught-up gate loses its live-frontier reference"
769+
);
770+
761771
let Some(object_info) = self.system_objects.get(object) else {
762772
panic!("migration step for non-existent builtin: {object:?}");
763773
};

0 commit comments

Comments
 (0)