diff --git a/doc/user/content/reference/system-catalog/mz_internal.md b/doc/user/content/reference/system-catalog/mz_internal.md index bc2694d5c903f..e8d4a88ce2fc4 100644 --- a/doc/user/content/reference/system-catalog/mz_internal.md +++ b/doc/user/content/reference/system-catalog/mz_internal.md @@ -181,7 +181,7 @@ shape is in [`mz_clusters`](../mz_catalog/#mz_clusters). | `status` | [`text`] | The lifecycle status of the reconfiguration: `in-progress` while the controller converges on the target, then a terminal `finalized`, `timed-out`, `cancelled`, or `resource-exhausted`. The record is retained after it settles, so the latest outcome stays inspectable until a later reconfiguration overwrites it. | | `deadline` | [`mz_timestamp`] | The deadline by which the reconfiguration must complete. After it passes, the `on_timeout` action applies. | | `on_timeout` | [`text`] | The action applied if `deadline` passes before the target hydrates: `commit` (cut over to the not-yet-hydrated target) or `rollback` (revert to the pre-reconfiguration shape). | -| `target` | [`jsonb`] | The config shape the cluster is reconfiguring to, as JSON: `size`, `replication_factor`, `availability_zones`, and `logging`. The realized (current) shape is in `mz_clusters`. | +| `target` | [`jsonb`] | The config shape the cluster is reconfiguring to, as JSON: `size`, `replication_factor`, `availability_zones`, `logging`, and `arrangement_compression`. The realized (current) shape is in `mz_clusters`. | | `changes` | [`jsonb`] | The dimensions in which `target` differs from the cluster's realized configuration, as a JSON object holding the target value per changed dimension. Empty (`{}`) once a record settles with its target applied. A rolled-back record keeps the abandoned diff. | ## `mz_cluster_auto_scaling_strategies` diff --git a/doc/user/content/self-managed-deployments/troubleshooting.md b/doc/user/content/self-managed-deployments/troubleshooting.md index 5be696e1d8d7b..ca3e0b02136bc 100644 --- a/doc/user/content/self-managed-deployments/troubleshooting.md +++ b/doc/user/content/self-managed-deployments/troubleshooting.md @@ -113,13 +113,19 @@ To increase the cluster's size, you can follow the following steps: show clusters; ``` + Resizing a cluster is a graceful reconfiguration: Materialize brings up a + replica at the new size, waits for it to hydrate, and only then retires the + old one. Until that finishes, `SHOW CLUSTERS` reports the old size, and + briefly both. Re-run the statement until it settles. The replacement replica + also gets a fresh name, so the resized cluster reports `r2` rather than `r1`. + The output should include the `mz_catalog_server` cluster with a size of `50cc`: ```none name | replicas | comment -------------------+-----------+--------- mz_analytics | | - mz_catalog_server | r1 (50cc) | + mz_catalog_server | r2 (50cc) | mz_probe | | mz_support | | mz_system | | diff --git a/doc/user/content/sql/alter-cluster.md b/doc/user/content/sql/alter-cluster.md index e21673884d665..5e9f1f8d1bf8e 100644 --- a/doc/user/content/sql/alter-cluster.md +++ b/doc/user/content/sql/alter-cluster.md @@ -177,10 +177,6 @@ by default), Materialize rolls back the resize and the cluster keeps its current size. To customize the timeout behavior, use the `WAIT UNTIL READY` or `WAIT FOR` options. The resize still proceeds in the background. -{{< private-preview >}} -Customizing the resize timeout with `WAIT UNTIL READY` or `WAIT FOR` -{{< /private-preview >}} - - `WAIT UNTIL READY (TIMEOUT = ..., ON TIMEOUT = ...)` sets the timeout for the resize. On timeout, `ON TIMEOUT` selects whether to `COMMIT` (retire the old replicas and proceed with the not-yet-hydrated new ones, which can cause @@ -225,7 +221,6 @@ current size. Materialize drops the pending replicas and keeps the current configuration. #### Downtime considerations for v26.34 or before -{{< private-preview />}} You can use the `WAIT UNTIL READY` option to perform a zero-downtime resizing, which incurs **no downtime**. Instead of restarting the cluster, this approach diff --git a/doc/user/data/metrics.yml b/doc/user/data/metrics.yml index 276de9f8288df..1f086a0e000e9 100644 --- a/doc/user/data/metrics.yml +++ b/doc/user/data/metrics.yml @@ -453,28 +453,6 @@ metrics: help: Total number of started transactions. source: src/catalog/src/durable/metrics.rs visibility: internal -- name: mz_check_scheduling_policies_seconds_bucket - help: The time each policy in `check_scheduling_policies` takes. - labels: - - le - - policy - - thread - source: src/adapter/src/metrics.rs - visibility: internal -- name: mz_check_scheduling_policies_seconds_count - help: The time each policy in `check_scheduling_policies` takes. - labels: - - policy - - thread - source: src/adapter/src/metrics.rs - visibility: internal -- name: mz_check_scheduling_policies_seconds_sum - help: The time each policy in `check_scheduling_policies` takes. - labels: - - policy - - thread - source: src/adapter/src/metrics.rs - visibility: internal - name: mz_cluster_handle_command_duration_seconds_bucket help: Time spent in handling commands. labels: @@ -971,25 +949,6 @@ metrics: help: The time it takes to advance the catalog shard upper for a txns-shard write (group commits and table register/forget). source: src/adapter/src/metrics.rs visibility: internal -- name: mz_handle_scheduling_decisions_seconds_bucket - help: The time `handle_scheduling_decisions` takes. - labels: - - altered_a_cluster - - le - source: src/adapter/src/metrics.rs - visibility: internal -- name: mz_handle_scheduling_decisions_seconds_count - help: The time `handle_scheduling_decisions` takes. - labels: - - altered_a_cluster - source: src/adapter/src/metrics.rs - visibility: internal -- name: mz_handle_scheduling_decisions_seconds_sum - help: The time `handle_scheduling_decisions` takes. - labels: - - altered_a_cluster - source: src/adapter/src/metrics.rs - visibility: internal - name: mz_index_peek_cursor_setup_seconds_bucket help: Time setting up cursor and literal constraints. labels: diff --git a/misc/python/materialize/checks/all_checks/builtin_cluster_replication_factor.py b/misc/python/materialize/checks/all_checks/builtin_cluster_replication_factor.py index 95742b6b87482..8277efad9e5bd 100644 --- a/misc/python/materialize/checks/all_checks/builtin_cluster_replication_factor.py +++ b/misc/python/materialize/checks/all_checks/builtin_cluster_replication_factor.py @@ -21,6 +21,10 @@ class BuiltinClusterReplicationFactor(Check): the break-glass path a support engineer scales up by hand. A catalog open that does not read the cluster's replication factor tears that replica down, leaving the cluster reporting a factor it is not honoring. + + The replica arrives asynchronously: the cluster controller materializes it a + tick after the ALTER commits. Every assertion below is a retrying testdrive + query for that reason. """ def _can_run(self, e: Executor) -> bool: diff --git a/misc/python/materialize/checks/all_checks/cluster.py b/misc/python/materialize/checks/all_checks/cluster.py index 65a4bcb12ad1e..a969489da776a 100644 --- a/misc/python/materialize/checks/all_checks/cluster.py +++ b/misc/python/materialize/checks/all_checks/cluster.py @@ -152,9 +152,6 @@ def _can_run(self, e: Executor) -> bool: def initialize(self) -> Testdrive: return Testdrive(dedent(""" - $ postgres-execute connection=postgres://mz_system@${testdrive.materialize-internal-sql-addr} - ALTER SYSTEM SET enable_zero_downtime_cluster_reconfiguration = true - $ postgres-execute connection=postgres://postgres:postgres@postgres CREATE USER graceful_reconfig WITH SUPERUSER PASSWORD 'postgres'; ALTER USER graceful_reconfig WITH replication; diff --git a/misc/python/materialize/mzcompose/__init__.py b/misc/python/materialize/mzcompose/__init__.py index 9fb80b0a1b169..9a13c01f1f5ef 100644 --- a/misc/python/materialize/mzcompose/__init__.py +++ b/misc/python/materialize/mzcompose/__init__.py @@ -97,14 +97,9 @@ def get_minimal_system_parameters( "enable_refresh_every_mvs": "true", "enable_replacement_materialized_views": "true", "enable_cluster_schedule_refresh": "true", - # The cluster controller and background ALTER CLUSTER dyncfgs default on - # in current versions. Pin them explicitly so runs against older versions - # (which predate the flags or defaulted them off) exercise the legacy - # paths while current versions exercise the controller owning the - # managed-cluster replica set. - "enable_cluster_controller": ( - "true" if version >= MzVersion.parse_mz("v26.29.0-dev") else "false" - ), + # Pinned explicitly so runs against older versions (which predate the + # flag or defaulted it off) behave like current ones, where it defaults + # on. "enable_background_alter_cluster": ( "true" if version >= MzVersion.parse_mz("v26.29.0-dev") else "false" ), @@ -127,6 +122,23 @@ def get_minimal_system_parameters( if version < MzVersion.parse_mz("v0.163.0-dev"): config["enable_compute_active_dataflow_cancelation"] = "true" + # The cluster controller's break-glass gate. Removed in v26.38, where the + # controller runs unconditionally. Older binaries still read it, and + # defaulted it off before v26.29, so pin it on for them to keep mixed-version + # runs exercising the same path as current versions. + if version < MzVersion.parse_mz("v26.38.0-dev"): + config["enable_cluster_controller"] = ( + "true" if version >= MzVersion.parse_mz("v26.29.0-dev") else "false" + ) + + # The `WITH (WAIT ...)` graceful-reconfiguration surface. Always accepted + # from v26.38 on. Older binaries still gate it behind this feature flag, so + # pin it on for them: the tests that use the surface no longer enable it + # themselves, and in a mixed-version run some of their phases execute + # against the old binary. + if version < MzVersion.parse_mz("v26.38.0-dev"): + config["enable_zero_downtime_cluster_reconfiguration"] = "true" + return config diff --git a/misc/python/materialize/parallel_workload/action.py b/misc/python/materialize/parallel_workload/action.py index c585e9bccb09a..e33e80231b316 100644 --- a/misc/python/materialize/parallel_workload/action.py +++ b/misc/python/materialize/parallel_workload/action.py @@ -3270,7 +3270,6 @@ def __init__( "oidc_group_role_sync_strict", "console_oidc_client_id", "console_oidc_scopes", - "enable_cluster_controller", "cluster_controller_tick_interval", "enable_background_alter_cluster", "default_cluster_reconfiguration_timeout", diff --git a/misc/python/materialize/parallel_workload/parallel_workload.py b/misc/python/materialize/parallel_workload/parallel_workload.py index addeca00435fe..f4c9afdcb2ae9 100644 --- a/misc/python/materialize/parallel_workload/parallel_workload.py +++ b/misc/python/materialize/parallel_workload/parallel_workload.py @@ -168,10 +168,6 @@ def run( system_exe.execute("ALTER SYSTEM SET max_sql_server_connections = 1000000") system_exe.execute("ALTER SYSTEM SET max_kafka_connections = 1000000") system_exe.execute("ALTER SYSTEM SET idle_in_transaction_session_timeout = 0") - # Gates the WITH (WAIT ...) clause used by ReconfigureClusterAction. - system_exe.execute( - "ALTER SYSTEM SET enable_zero_downtime_cluster_reconfiguration = true" - ) # Most queries should not fail because of privileges for object_type in [ "TABLES", diff --git a/src/adapter-types/src/dyncfgs.rs b/src/adapter-types/src/dyncfgs.rs index 2a370901583f8..327af7a7c464e 100644 --- a/src/adapter-types/src/dyncfgs.rs +++ b/src/adapter-types/src/dyncfgs.rs @@ -353,23 +353,7 @@ pub const ENABLE_SCOPED_SYSTEM_PARAMETERS: Config = Config::new( "Whether per-cluster and per-replica scoped system parameters are evaluated and applied.", ); -/// Top-level gate for the cluster controller. When on, the controller owns the -/// managed-cluster replica set and the legacy paths (the graceful 3-stage -/// machine and `cluster_scheduling.rs`) are bypassed. The replica set cannot -/// have two writers, so this is a clean switch, not a per-strategy toggle. -/// -/// Defaults on. This is the break-glass switch to fall back to the legacy -/// paths if the controller misbehaves. -pub const ENABLE_CLUSTER_CONTROLLER: Config = Config::new( - "enable_cluster_controller", - true, - "Whether the cluster controller owns the managed-cluster replica set. When false, the legacy scheduling and graceful-reconfiguration paths run instead.", -); - /// Cadence of the cluster controller's reconcile tick. -/// -/// Replaces `cluster_check_scheduling_policies_interval` once the controller is -/// the sole owner; while the controller is dark both intervals exist. pub const CLUSTER_CONTROLLER_TICK_INTERVAL: Config = Config::new( "cluster_controller_tick_interval", Duration::from_secs(5), @@ -380,9 +364,6 @@ pub const CLUSTER_CONTROLLER_TICK_INTERVAL: Config = Config::new( /// controller converging in the background, or blocks the session on a /// wait-shim until the reconfiguration completes or its deadline passes. /// -/// Only consulted while [`ENABLE_CLUSTER_CONTROLLER`] is on, when the -/// controller owns the reconfiguration. -/// /// Defaults on. This is the break-glass switch back to the blocking wait-shim /// if returning immediately causes trouble. pub const ENABLE_BACKGROUND_ALTER_CLUSTER: Config = Config::new( @@ -404,9 +385,9 @@ pub const DEFAULT_CLUSTER_RECONFIGURATION_TIMEOUT: Config = Config::ne /// runs a burst replica; graceful reconfiguration and `ON REFRESH` scheduling /// are unaffected. /// -/// Only consulted while [`ENABLE_CLUSTER_CONTROLLER`] is on. A cluster can only -/// carry an `AUTO SCALING STRATEGY` while its SQL acceptance feature flag is -/// on, so this is the second of the two gates burst sits behind. +/// A cluster can only carry an `AUTO SCALING STRATEGY` while its SQL acceptance +/// feature flag is on, so this is the second of the two gates burst sits +/// behind. pub const ENABLE_HYDRATION_BURST: Config = Config::new( "enable_hydration_burst", true, @@ -426,7 +407,6 @@ pub const DEFAULT_HYDRATION_BURST_LINGER: Config = Config::new( pub fn all_dyncfgs(configs: ConfigSet) -> ConfigSet { configs .add(&ALLOW_USER_SESSIONS) - .add(&ENABLE_CLUSTER_CONTROLLER) .add(&CLUSTER_CONTROLLER_TICK_INTERVAL) .add(&ENABLE_BACKGROUND_ALTER_CLUSTER) .add(&DEFAULT_CLUSTER_RECONFIGURATION_TIMEOUT) diff --git a/src/adapter/src/catalog/open.rs b/src/adapter/src/catalog/open.rs index afe4f8f1729c7..43c696e2b8d1a 100644 --- a/src/adapter/src/catalog/open.rs +++ b/src/adapter/src/catalog/open.rs @@ -1155,9 +1155,14 @@ fn add_new_remove_old_builtin_roles_migration( /// recorded durably and runs before the cluster controller is spawned, and the /// controller does not run at all while a deployment is read-only. /// -/// The controller derives its target from the same cluster config, so it converges -/// on the same replica set rather than competing for it. It excludes system -/// clusters today, but nothing here depends on that staying true. +/// The cluster controller owns these replica sets at runtime and derives its +/// target from the same cluster config, so the two converge on the same set +/// rather than competing for it. The controller matches replicas by shape and +/// count, never by name, so the `r1..rN` this creates satisfy it. This converges +/// by name, so a boot after the controller reshaped a cluster renames or +/// re-creates replicas it had materialized under generator names. That is +/// harmless: every replica is a cold process at boot anyway, so the cost is +/// replica-id and audit-log noise. fn reconcile_builtin_cluster_replicas( txn: &mut Transaction<'_>, builtin_cluster_config_map: &BuiltinBootstrapClusterConfigMap, @@ -1231,9 +1236,9 @@ fn reconcile_builtin_cluster_replicas( } // Reading the cluster's factor is what makes this compose with the other - // writers of a replica set. The refresh scheduler parks a scheduled cluster - // by writing its factor to 0, so converging on the factor honors that - // instead of resurrecting a replica the scheduler just dropped. + // writers of a replica set. The controller's on-refresh strategy parks a + // scheduled cluster by writing its factor to 0, so converging on the + // factor honors that instead of resurrecting a replica it just dropped. let mut surplus = replicas_by_cluster.remove(&cluster.id).unwrap_or_default(); for index in 0..managed.replication_factor { let replica_name = managed_cluster_replica_name(index); @@ -1425,8 +1430,14 @@ fn remove_invalid_config_param_role_defaults_migration( Ok(()) } -/// Cluster Replicas may be created ephemerally during an alter statement, these replicas -/// are marked as pending and should be cleaned up on catalog open. +/// Drops replicas left durably marked `pending`. +/// +/// No runtime path creates one anymore. An upgrade can still come from a version +/// whose staged reconfiguration machine crashed between the pending-create commit +/// and the finalize, and those replicas are excluded from the cluster +/// controller's ownership test, so this catalog-open sweep is their only +/// remaining cleaner. It goes away together with the durable `pending` field, +/// once no supported upgrade source can still write one. fn remove_pending_cluster_replicas_migration( tx: &mut Transaction, boot_ts: mz_repr::Timestamp, diff --git a/src/adapter/src/catalog/open/builtin_schema_migration.rs b/src/adapter/src/catalog/open/builtin_schema_migration.rs index 58b4ec995a88a..549d036034b2b 100644 --- a/src/adapter/src/catalog/open/builtin_schema_migration.rs +++ b/src/adapter/src/catalog/open/builtin_schema_migration.rs @@ -355,6 +355,16 @@ static MIGRATIONS: LazyLock> = LazyLock::new(|| { MZ_CATALOG_SCHEMA, "mz_aws_privatelink_connections", ), + // The mz_cluster_reconfigurations MV definition changed (the `changes` + // diff now includes the `arrangement_compression` dimension). See the + // NOTE above: this version must stay at the workspace's current dev + // version until the change ships. + MigrationStep::replacement( + "26.38.0-dev.0", + CatalogItemType::MaterializedView, + MZ_INTERNAL_SCHEMA, + "mz_cluster_reconfigurations", + ), ] }); diff --git a/src/adapter/src/catalog/transact.rs b/src/adapter/src/catalog/transact.rs index c69eb846f998a..947ab6818c43c 100644 --- a/src/adapter/src/catalog/transact.rs +++ b/src/adapter/src/catalog/transact.rs @@ -86,7 +86,6 @@ use crate::catalog::{ use crate::config::{ScopedParameters, ScopedParametersScope}; use crate::coord::ConnMeta; use crate::coord::catalog_implications::parsed_state_updates::ParsedStateUpdate; -use crate::coord::cluster_scheduling::SchedulingDecision; use crate::util::ResultExt; /// A manually injected audit event. @@ -360,9 +359,6 @@ pub enum ReplicaCreateDropReason { /// - ALTERing various options on a managed cluster, /// - CREATE/DROP CLUSTER REPLICA on an unmanaged cluster. Manual, - /// The automated cluster scheduling initiated the replica create or drop, e.g., a - /// materialized view is needing a refresh on a SCHEDULE ON REFRESH cluster. - ClusterScheduling(Vec), /// The cluster controller's graceful-reconfiguration strategy created the replica while /// converging a cluster onto an in-flight `reconfiguration` target (a background /// `ALTER CLUSTER`). @@ -373,11 +369,7 @@ pub enum ReplicaCreateDropReason { /// The cluster controller's on-refresh strategy created the replica for a refresh window on /// a `SCHEDULE = ON REFRESH` cluster. Audited as the `schedule` reason, carrying the tick's /// window decision (which MVs needed a refresh or compaction time, and the hydration-time - /// estimate) as the `scheduling_policies` detail, the same detail the legacy scheduler's - /// [`ReplicaCreateDropReason::ClusterScheduling`] records. Deliberately not that variant - /// itself: its legacy shape carries a per-policy `Vec` and an on/off flag for auditing - /// off-decisions, neither of which the controller has (controller drops are uniformly - /// `Retired`), and it is removed together with the legacy scheduler. + /// estimate) as the `scheduling_policies` detail. OnRefresh(RefreshWindowDecision), /// The cluster controller dropped the replica because the cluster's configuration no longer /// calls for it. The uniform reason on every controller-emitted drop (e.g. a @@ -394,12 +386,6 @@ impl ReplicaCreateDropReason { ) { match self { ReplicaCreateDropReason::Manual => (CreateOrDropClusterReplicaReasonV1::Manual, None), - ReplicaCreateDropReason::ClusterScheduling(scheduling_decisions) => ( - CreateOrDropClusterReplicaReasonV1::Schedule, - Some(SchedulingDecision::reasons_to_audit_log_reasons( - &scheduling_decisions, - )), - ), ReplicaCreateDropReason::GracefulReconfiguration => { (CreateOrDropClusterReplicaReasonV1::Reconfiguration, None) } @@ -416,8 +402,8 @@ impl ReplicaCreateDropReason { } /// Convert the controller's on-refresh window decision into the audit log's -/// `scheduling_policies` detail, the same shape the legacy scheduler records: -/// ids as strings and the hydration-time estimate as an interval string. +/// `scheduling_policies` detail: ids as strings and the hydration-time estimate +/// as an interval string. fn refresh_window_decision_to_audit_log( decision: RefreshWindowDecision, ) -> SchedulingDecisionsWithReasonsV2 { @@ -514,8 +500,8 @@ impl Catalog { /// status change, a fresh record, or the drop of an in-progress record. /// /// Every such movement is an audit-log transition, so a write performing - /// one must declare the matching intent. Status-preserving copies (legacy - /// paths carrying a record forward, re-targets that stay in progress with a + /// one must declare the matching intent. Status-preserving copies (a write + /// carrying a record forward, re-targets that stay in progress with a /// declared `Started`) and drops of already-settled records move nothing. fn reconfiguration_lifecycle_moved( old_config: &ClusterConfig, @@ -3827,8 +3813,8 @@ mod tests { &unmanaged, )); - // Not movements: no record at all, a status-preserving copy (legacy - // paths carry the record forward), and dropping a settled record. + // Not movements: no record at all, a status-preserving copy (a write + // that carries the record forward), and dropping a settled record. assert!(!Catalog::reconfiguration_lifecycle_moved( &managed(None), &managed(None), @@ -3985,9 +3971,8 @@ mod tests { use crate::catalog::ReplicaCreateDropReason; - // `OnRefresh` shares the `schedule` audit word with the legacy - // `ClusterScheduling` variant and converts the controller's window - // decision into the same `scheduling_policies` detail blob: ids as + // `OnRefresh` audits the `schedule` word and converts the controller's + // window decision into the `scheduling_policies` detail blob: ids as // strings, the hydration-time estimate as an interval string, and the // decision hardcoded `on` (the controller produces a create, and so // this detail, only for an open window). diff --git a/src/adapter/src/coord.rs b/src/adapter/src/coord.rs index 66650756c64ef..1fd5deaaa2c6d 100644 --- a/src/adapter/src/coord.rs +++ b/src/adapter/src/coord.rs @@ -104,8 +104,8 @@ use mz_catalog::config::{AwsPrincipalContext, BuiltinItemMigrationConfig, Cluste use mz_catalog::durable::OpenableDurableCatalogState; use mz_catalog::expr_cache::{GlobalExpressions, LocalExpressions}; use mz_catalog::memory::objects::{ - CatalogEntry, CatalogItem, ClusterReplicaProcessStatus, ClusterVariantManaged, Connection, - DataSourceDesc, ReconfigurationTarget, Table, TableDataSource, + CatalogEntry, CatalogItem, ClusterReplicaProcessStatus, Connection, DataSourceDesc, + ReconfigurationTarget, Table, TableDataSource, }; use mz_cloud_resources::{CloudResourceController, VpcEndpointConfig, VpcEndpointEvent}; use mz_compute_client::as_of_selection; @@ -153,7 +153,7 @@ use mz_sql::names::{QualifiedItemName, ResolvedIds, SchemaSpecifier}; use mz_sql::optimizer_metrics::OptimizerMetrics; use mz_sql::plan::{ self, AlterSinkPlan, ConnectionDetails, CreateConnectionPlan, HirRelationExpr, - NetworkPolicyRule, OnTimeoutAction, Params, QueryWhen, + NetworkPolicyRule, Params, QueryWhen, }; use mz_sql::session::user::User; use mz_sql::session::vars::{MAX_CREDIT_CONSUMPTION_RATE, SystemVars, Var}; @@ -195,7 +195,6 @@ use crate::coord::appends::{ PendingWriteTxn, }; use crate::coord::caught_up::CaughtUpCheckContext; -use crate::coord::cluster_scheduling::SchedulingDecision; use crate::coord::id_bundle::CollectionIdBundle; use crate::coord::introspection::IntrospectionSubscribe; use crate::coord::peek::PendingPeek; @@ -220,7 +219,6 @@ use crate::{AdapterNotice, ReadHolds, flags}; pub(crate) mod appends; pub(crate) mod catalog_serving; pub(crate) mod cluster_controller; -pub(crate) mod cluster_scheduling; pub(crate) mod consistency; pub(crate) mod id_bundle; pub(crate) mod in_memory_oracle; @@ -446,13 +444,6 @@ pub enum Message { }, DrainStatementLog, PrivateLinkVpcEndpointEvents(Vec), - CheckSchedulingPolicies, - - /// Scheduling policy decisions about turning clusters On/Off. - /// `Vec<(policy name, Vec of decisions by the policy)>` - /// A cluster will be On if and only if there is at least one On decision for it. - /// Scheduling decisions for clusters that have `SCHEDULE = MANUAL` are ignored. - SchedulingDecisions(Vec<(&'static str, Vec<(ClusterId, SchedulingDecision)>)>), /// One pull/apply call from the cluster controller task, answered on the main /// coordinator message loop from the catalog and live controller signals. @@ -560,8 +551,6 @@ impl Message { Message::DrainStatementLog => "drain_statement_log", Message::AlterConnectionValidationReady(..) => "alter_connection_validation_ready", Message::PrivateLinkVpcEndpointEvents(_) => "private_link_vpc_endpoint_events", - Message::CheckSchedulingPolicies => "check_scheduling_policies", - Message::SchedulingDecisions { .. } => "scheduling_decision", Message::ClusterControllerRequest(_) => "cluster_controller_request", Message::DeferredStatementReady => "deferred_statement_ready", } @@ -886,8 +875,6 @@ pub struct ExplainTimestampFinish { #[derive(Debug)] pub enum ClusterStage { Alter(AlterCluster), - WaitForHydrated(AlterClusterWaitForHydrated), - Finalize(AlterClusterFinalize), /// The foreground wait-shim over a controller-driven background /// reconfiguration: poll the durable `reconfiguration` record until it /// clears, then report success or timeout depending on whether the realized @@ -901,24 +888,6 @@ pub struct AlterCluster { plan: plan::AlterClusterPlan, } -#[derive(Debug)] -pub struct AlterClusterWaitForHydrated { - validity: PlanValidity, - plan: plan::AlterClusterPlan, - new_config: ClusterVariantManaged, - workload_class: Option, - timeout_time: Instant, - on_timeout: OnTimeoutAction, -} - -#[derive(Debug)] -pub struct AlterClusterFinalize { - validity: PlanValidity, - plan: plan::AlterClusterPlan, - new_config: ClusterVariantManaged, - workload_class: Option, -} - #[derive(Debug)] pub struct AlterClusterAwaitReconfiguration { validity: PlanValidity, @@ -1323,10 +1292,6 @@ pub struct ConnMeta { #[serde(skip)] deferred_lock: Option>, - /// Cluster reconfigurations that will need to be - /// cleaned up when the current transaction is cleared - pending_cluster_alters: BTreeSet, - /// Channel on which to send notices to a session. #[serde(skip)] notice_tx: mpsc::UnboundedSender, @@ -2134,14 +2099,6 @@ pub struct Coordinator { /// a timestamp oracle backend is configured. timestamp_oracle_config: Option, - /// Periodically asks cluster scheduling policies to make their decisions. - check_cluster_scheduling_policies_interval: Interval, - - /// This keeps the last On/Off decision for each cluster and each scheduling policy. - /// (Clusters that have been dropped or are otherwise out of scope for automatic scheduling are - /// periodically cleaned up from this Map.) - cluster_scheduling_decisions: BTreeMap>, - /// When doing 0dt upgrades/in read-only mode, periodically ask all known /// clusters/collections whether they are caught up. caught_up_check_interval: Interval, @@ -4041,13 +3998,6 @@ impl Coordinator { linearize_reads_notified.set(linearize_reads_notify.notified()); messages.push(Message::LinearizeReads); } - // `tick()` on `Interval` is cancel-safe: - // https://docs.rs/tokio/1.19.2/tokio/time/struct.Interval.html#cancel-safety - // Receive a single command. - _ = self.check_cluster_scheduling_policies_interval.tick() => { - messages.push(Message::CheckSchedulingPolicies); - }, - // `tick()` on `Interval` is cancel-safe: // https://docs.rs/tokio/1.19.2/tokio/time/struct.Interval.html#cancel-safety // Receive a single command. @@ -4928,12 +4878,6 @@ pub fn serve( let coord_now = now.clone(); let advance_timelines_interval = tokio::time::interval(catalog.system_config().default_timestamp_interval()); - let mut check_scheduling_policies_interval = tokio::time::interval( - catalog - .system_config() - .cluster_check_scheduling_policies_interval(), - ); - check_scheduling_policies_interval.set_missed_tick_behavior(MissedTickBehavior::Delay); let clusters_caught_up_check_interval = if read_only_controllers { let dyncfgs = catalog.system_config().dyncfgs(); @@ -5122,8 +5066,6 @@ pub fn serve( statement_logging: StatementLogging::new(coord_now.clone()), webhook_concurrency_limit, timestamp_oracle_config, - check_cluster_scheduling_policies_interval: check_scheduling_policies_interval, - cluster_scheduling_decisions: BTreeMap::new(), caught_up_check_interval: clusters_caught_up_check_interval, caught_up_check: clusters_caught_up_check, installed_watch_sets: BTreeMap::new(), diff --git a/src/adapter/src/coord/cluster_controller.rs b/src/adapter/src/coord/cluster_controller.rs index be53782f19608..f958edf378252 100644 --- a/src/adapter/src/coord/cluster_controller.rs +++ b/src/adapter/src/coord/cluster_controller.rs @@ -18,19 +18,17 @@ //! signals are pulled on demand, so a tick's round-trips scale with the number of //! managed clusters that need a live signal, not with a constant. //! -//! Everything here is gated by [`ENABLE_CLUSTER_CONTROLLER`] (default on). With -//! the gate off the task does not tick, so the legacy scheduling and graceful -//! paths remain the sole writers of the replica set. With the gate on the -//! controller owns the *user* managed-cluster replica set; the legacy entry -//! points no-op. (System/builtin clusters are excluded here. Their config-implied -//! replicas are materialized by `reconcile_builtin_cluster_replicas` at catalog -//! open, which derives the same target from the same config.) +//! The controller owns the replica set of every managed cluster, user and +//! system alike. A builtin cluster's config-implied replicas are additionally +//! materialized by `reconcile_builtin_cluster_replicas` at catalog open, which +//! derives the same target from the same config, so the two converge rather +//! than compete. use std::collections::BTreeSet; use std::sync::Arc; use std::time::Duration; -use mz_adapter_types::dyncfgs::{CLUSTER_CONTROLLER_TICK_INTERVAL, ENABLE_CLUSTER_CONTROLLER}; +use mz_adapter_types::dyncfgs::CLUSTER_CONTROLLER_TICK_INTERVAL; use mz_catalog::memory::objects::{ClusterConfig, ClusterVariant}; use mz_cluster_controller::ClusterController; use mz_cluster_controller::ctx::{ @@ -58,9 +56,7 @@ use crate::error::AdapterError; /// per-cluster live signal a strategy pulls on demand. #[derive(Debug)] pub enum ClusterControllerRequest { - /// The ids of all *user* managed clusters the controller owns this tick. - /// System/builtin clusters are excluded. `reconcile_builtin_cluster_replicas` - /// has already materialized their config-implied replicas at catalog open. + /// The ids of all managed clusters the controller owns this tick. ManagedClusterIds { tx: oneshot::Sender> }, /// A consistent durable view of the given clusters and their replicas, plus /// the current time. @@ -203,13 +199,10 @@ impl ClusterControllerCtx for CoordCtx { impl Coordinator { /// Spawn the cluster controller task. /// - /// The task ticks at [`CLUSTER_CONTROLLER_TICK_INTERVAL`] and reconciles when - /// [`ENABLE_CLUSTER_CONTROLLER`] is on; while the gate is off it ticks but - /// each tick is an early no-op. Both the gate and the interval are re-read - /// each tick (the interval via a [`ClusterControllerRequest::TickInterval`] - /// round-trip), so a runtime change to either takes effect without a restart. - /// It owns the controller and a [`CoordCtx`] that marshals back to this - /// Coordinator. + /// The task ticks at [`CLUSTER_CONTROLLER_TICK_INTERVAL`], re-read each tick + /// via a [`ClusterControllerRequest::TickInterval`] round-trip so a runtime + /// change takes effect without a restart. It owns the controller and a + /// [`CoordCtx`] that marshals back to this Coordinator. /// /// The interval is the fallback cadence: `reconcile_now` cuts the /// sleep short after a catalog transaction changes durable cluster state. @@ -255,37 +248,27 @@ impl Coordinator { /// Handle one [`ClusterControllerRequest`] on the coordinator loop. /// - /// The controller is inactive when the gate is off, or while the deployment - /// is in read-only mode (a 0dt upgrade, where it must not write the catalog). - /// When inactive, reads report no managed clusters (so the controller finds - /// nothing to reconcile) and applies are rejected: the task still wakes each - /// tick and sends one `ManagedClusterIds` request, but that request - /// early-returns here and no catalog state is read or written, so the legacy - /// paths remain the sole writers of the replica set. The task keeps ticking, - /// so the controller reactivates on its own once the deployment promotes out - /// of read-only mode. + /// The controller is inactive while the deployment is in read-only mode (a + /// 0dt upgrade, where it must not write the catalog). When inactive, reads + /// report no managed clusters (so the controller finds nothing to + /// reconcile) and applies are rejected: the task still wakes each tick and + /// sends one `ManagedClusterIds` request, but that request early-returns + /// here and no catalog state is read or written. The task keeps ticking, so + /// the controller reactivates on its own once the deployment promotes out of + /// read-only mode. #[mz_ore::instrument(level = "debug")] pub(crate) async fn handle_cluster_controller_request( &mut self, request: ClusterControllerRequest, ) { - let active = ENABLE_CLUSTER_CONTROLLER.get(self.catalog().system_config().dyncfgs()) - && !self.controller.read_only(); + let active = !self.controller.read_only(); match request { ClusterControllerRequest::ManagedClusterIds { tx } => { let ids = if active { self.catalog() .clusters() - // Only *user* managed clusters. System/builtin clusters - // (mz_system, mz_catalog_server, …) are also managed, and - // `reconcile_builtin_cluster_replicas` materializes their - // config-implied replica set at catalog open, so those - // replicas exist before the controller could ever tick. - // Both derive the target from the cluster's - // `replication_factor`, so extending ownership here would - // converge rather than conflict. - .filter(|c| c.is_managed() && c.id.is_user()) + .filter(|c| c.is_managed()) .map(|c| c.id) .collect() } else { @@ -331,8 +314,7 @@ impl Coordinator { // then complete the reply from a spawned task: the oracle // read is a network round-trip (to the Postgres/CRDB-backed // timestamp oracle) and must never run on the serial - // coordinator loop. The legacy `check_refresh_policy` makes - // the same split. + // coordinator loop. match self.refresh_window_catalog_inputs(cluster_id) { None => { let _ = tx.send(None); @@ -375,7 +357,11 @@ impl Coordinator { /// Build the controller's view of one managed cluster from the catalog. /// Returns `None` for a missing or unmanaged cluster. - fn observe_cluster_state(&self, cluster_id: ClusterId) -> Option { + /// + /// Also used by the ALTER sequencer's synchronous cut-over, which runs the + /// controller's reconcile kernel against this same view so both paths + /// converge on the same replica set. + pub(crate) fn observe_cluster_state(&self, cluster_id: ClusterId) -> Option { let cluster = self.catalog().try_get_cluster(cluster_id)?; let ClusterVariant::Managed(managed) = &cluster.config.variant else { return None; @@ -511,8 +497,7 @@ impl Coordinator { /// cluster (the system compaction estimate and each bound REFRESH /// materialized view's storage write frontier and refresh schedule), or /// `None` if the cluster is missing, unmanaged, or not scheduled `ON - /// REFRESH`. These are the same signals the legacy `check_refresh_policy` - /// reads. + /// REFRESH`. /// /// The oracle read timestamp completing [`RefreshWindowInputs`] is /// deliberately not fetched here: this runs on the coordinator loop, and @@ -520,8 +505,8 @@ impl Coordinator { /// a spawned task instead. /// /// The MV write frontier is carried through with full fidelity as the - /// `Antichain` the storage controller reports, matching the legacy refresh - /// policy; the on-refresh strategy compares against it directly. + /// `Antichain` the storage controller reports. The on-refresh strategy + /// compares against it directly. fn refresh_window_catalog_inputs( &self, cluster_id: ClusterId, diff --git a/src/adapter/src/coord/cluster_scheduling.rs b/src/adapter/src/coord/cluster_scheduling.rs deleted file mode 100644 index 88c5c9f6872e5..0000000000000 --- a/src/adapter/src/coord/cluster_scheduling.rs +++ /dev/null @@ -1,566 +0,0 @@ -// Copyright Materialize, Inc. and contributors. All rights reserved. -// -// Use of this software is governed by the Business Source License -// included in the LICENSE file. -// -// As of the Change Date specified in that file, in accordance with -// the Business Source License, use of this software will be governed -// by the Apache License, Version 2.0. - -use itertools::Itertools; -use mz_adapter_types::dyncfgs::ENABLE_CLUSTER_CONTROLLER; -use mz_audit_log::SchedulingDecisionsWithReasonsV2; -use mz_catalog::memory::objects::{CatalogItem, ClusterVariant, ClusterVariantManaged}; -use mz_controller_types::ClusterId; -use mz_ore::collections::CollectionExt; -use mz_ore::{soft_assert_or_log, soft_panic_or_log}; -use mz_repr::adt::interval::Interval; -use mz_repr::{GlobalId, TimestampManipulation}; -use mz_sql::catalog::CatalogCluster; -use mz_sql::plan::{AlterClusterPlanStrategy, ClusterSchedule}; -use std::time::{Duration, Instant}; -use tracing::{debug, warn}; - -use crate::AdapterError; -use crate::coord::sequencer::cancel_carried_reconfiguration; -use crate::coord::{Coordinator, Message}; - -const POLICIES: &[&str] = &[REFRESH_POLICY_NAME]; - -const REFRESH_POLICY_NAME: &str = "refresh"; - -/// A policy's decision for whether it wants a certain cluster to be On, along with its reason. -/// (Among the reasons there can be settings of the policy as well as other information about the -/// state of the system.) -#[derive(Clone, Debug)] -pub enum SchedulingDecision { - /// The reason for the refresh policy for wanting to turn a cluster On or Off. - Refresh(RefreshDecision), -} - -impl SchedulingDecision { - /// Extract the On/Off decision from the policy-specific structs. - pub fn cluster_on(&self) -> bool { - match &self { - SchedulingDecision::Refresh(RefreshDecision { cluster_on, .. }) => cluster_on.clone(), - } - } -} - -#[derive(Clone, Debug)] -pub struct RefreshDecision { - /// Whether the ON REFRESH policy wants a certain cluster to be On. - cluster_on: bool, - /// Objects that currently need a refresh on the cluster (taking into account the rehydration - /// time estimate), and therefore should keep the cluster On. - objects_needing_refresh: Vec, - /// Objects for which we estimate that they currently need Persist compaction, and therefore - /// should keep the cluster On. - objects_needing_compaction: Vec, - /// The HYDRATION TIME ESTIMATE setting of the cluster. - hydration_time_estimate: Duration, -} - -impl SchedulingDecision { - pub fn reasons_to_audit_log_reasons<'a, I>(reasons: I) -> SchedulingDecisionsWithReasonsV2 - where - I: IntoIterator, - { - SchedulingDecisionsWithReasonsV2 { - on_refresh: reasons - .into_iter() - .filter_map(|r| match r { - SchedulingDecision::Refresh(RefreshDecision { - cluster_on, - objects_needing_refresh, - objects_needing_compaction, - hydration_time_estimate, - }) => { - soft_assert_or_log!( - !cluster_on - || !objects_needing_refresh.is_empty() - || !objects_needing_compaction.is_empty(), - "`cluster_on = true` should have an explanation" - ); - let mut hydration_time_estimate_str = String::new(); - mz_repr::strconv::format_interval( - &mut hydration_time_estimate_str, - Interval::from_duration(hydration_time_estimate).expect( - "planning ensured that this is convertible back to Interval", - ), - ); - Some(mz_audit_log::RefreshDecisionWithReasonV2 { - decision: (*cluster_on).into(), - objects_needing_refresh: objects_needing_refresh - .iter() - .map(|id| id.to_string()) - .collect(), - objects_needing_compaction: objects_needing_compaction - .iter() - .map(|id| id.to_string()) - .collect(), - hydration_time_estimate: hydration_time_estimate_str, - }) - } - }) - .into_element(), // Each policy should have exactly one opinion on each cluster. - } - } -} - -impl Coordinator { - #[mz_ore::instrument(level = "debug")] - /// Call each scheduling policy. - /// - /// No-ops when the cluster controller owns the replica set - /// ([`ENABLE_CLUSTER_CONTROLLER`]): the controller's `OnRefreshStrategy` is - /// then the sole authority over scheduled clusters, so the legacy policy must - /// not also toggle their replication factor (two writers of the replica set is - /// not allowed). The legacy path remains in place to drive scheduling while the - /// gate is off. - pub(crate) async fn check_scheduling_policies(&self) { - if ENABLE_CLUSTER_CONTROLLER.get(self.catalog().system_config().dyncfgs()) { - return; - } - // (So far, we have only this one policy.) - self.check_refresh_policy(); - } - - /// Runs the `SCHEDULE = ON REFRESH` cluster scheduling policy, which makes cluster On/Off - /// decisions based on REFRESH materialized view write frontiers and the current time (the local - /// oracle read ts), and sends `Message::SchedulingDecisions` with these decisions. - /// (Queries the timestamp oracle on a background task.) - fn check_refresh_policy(&self) { - let start_time = Instant::now(); - - // Collect information about REFRESH MVs: - // - cluster - // - hydration_time_estimate of the cluster - // - MV's id - // - MV's write frontier - // - MV's refresh schedule - let mut refresh_mv_infos = Vec::new(); - for cluster in self.catalog().clusters() { - if let ClusterVariant::Managed(ref config) = cluster.config.variant { - match config.schedule { - ClusterSchedule::Manual => { - // Nothing to do, user manages this cluster manually. - } - ClusterSchedule::Refresh { - hydration_time_estimate, - } => { - let mvs = cluster - .bound_objects() - .iter() - .filter_map(|id| { - if let CatalogItem::MaterializedView(mv) = - self.catalog().get_entry(id).item() - { - mv.refresh_schedule.clone().map(|refresh_schedule| { - let (_since, write_frontier) = self - .controller - .storage - .collection_frontiers(mv.global_id_writes()) - .expect("the storage controller should know about MVs that exist in the catalog"); - (mv.global_id_writes(), write_frontier, refresh_schedule) - }) - } else { - None - } - }) - .collect_vec(); - debug!(%cluster.id, ?refresh_mv_infos, "check_refresh_policy"); - refresh_mv_infos.push((cluster.id, hydration_time_estimate, mvs)); - } - } - } - } - - // Spawn a background task that queries the timestamp oracle for the current read timestamp, - // compares this ts with the REFRESH MV write frontiers, thus making On/Off decisions per - // cluster, and sends a `Message::SchedulingDecisions` with these decisions. - let ts_oracle = self.get_local_timestamp_oracle(); - let internal_cmd_tx = self.internal_cmd_tx.clone(); - let check_scheduling_policies_seconds_cloned = - self.metrics.check_scheduling_policies_seconds.clone(); - let compaction_estimate = self - .catalog() - .system_config() - .cluster_refresh_mv_compaction_estimate() - .try_into() - .expect("should be configured to a reasonable value"); - mz_ore::task::spawn(|| "refresh policy get ts and make decisions", async move { - let task_start_time = Instant::now(); - let local_read_ts = ts_oracle.read_ts().await; - debug!(%local_read_ts, ?refresh_mv_infos, "check_refresh_policy background task"); - let decisions = refresh_mv_infos - .into_iter() - .map(|(cluster_id, hydration_time_estimate, refresh_mv_info)| { - // 1. check that - // write_frontier < local_read_ts + hydration_time_estimate - let hydration_estimate = &hydration_time_estimate - .try_into() - .expect("checked during planning"); - let local_read_ts_adjusted = local_read_ts.step_forward_by(hydration_estimate); - let mvs_needing_refresh = refresh_mv_info - .iter() - .cloned() - .filter_map(|(id, frontier, _refresh_schedule)| { - if frontier.less_than(&local_read_ts_adjusted) { - Some(id) - } else { - None - } - }) - .collect_vec(); - - // 2. check that - // prev_refresh + compaction_estimate > local_read_ts - let mvs_needing_compaction = refresh_mv_info - .into_iter() - .filter_map(|(id, frontier, refresh_schedule)| { - let frontier = frontier.as_option(); - // `prev_refresh` will be None in two cases: - // 1. When there is no previous refresh, because we haven't yet had - // the first refresh. In this case, there is no need to schedule - // time now for compaction. - // 2. In the niche case where a `REFRESH EVERY` MV's write frontier - // is empty. In this case, it's not impossible that there would be a - // need for compaction. But I can't see any easy way to correctly - // handle this case, because we don't have any info handy about when - // the last refresh happened in wall clock time, because the - // frontiers have no relation to wall clock time. So, we'll not - // schedule any compaction time. - // (Note that `REFRESH AT` MVs with empty frontiers, which is a more - // common case, are fine, because `last_refresh` will return - // Some(...) for them.) - let prev_refresh = match frontier { - Some(frontier) => frontier.round_down_minus_1(&refresh_schedule), - None => refresh_schedule.last_refresh(), - }; - prev_refresh - .map(|prev_refresh| { - if prev_refresh.step_forward_by(&compaction_estimate) - > local_read_ts - { - Some(id) - } else { - None - } - }) - .flatten() - }) - .collect_vec(); - - let cluster_on = - !mvs_needing_refresh.is_empty() || !mvs_needing_compaction.is_empty(); - ( - cluster_id, - SchedulingDecision::Refresh(RefreshDecision { - cluster_on, - objects_needing_refresh: mvs_needing_refresh, - objects_needing_compaction: mvs_needing_compaction, - hydration_time_estimate, - }), - ) - }) - .collect(); - if let Err(e) = internal_cmd_tx.send(Message::SchedulingDecisions(vec![( - REFRESH_POLICY_NAME, - decisions, - )])) { - // It is not an error for this task to be running after `internal_cmd_rx` is dropped. - warn!("internal_cmd_rx dropped before we could send: {:?}", e); - } - check_scheduling_policies_seconds_cloned - .with_label_values(&[REFRESH_POLICY_NAME, "background"]) - .observe((Instant::now() - task_start_time).as_secs_f64()); - }); - - self.metrics - .check_scheduling_policies_seconds - .with_label_values(&[REFRESH_POLICY_NAME, "main"]) - .observe((Instant::now() - start_time).as_secs_f64()); - } - - /// Handles `SchedulingDecisions`: - /// 1. Adds the newly made decisions to `cluster_scheduling_decisions`. - /// 2. Cleans up old decisions that are for clusters no longer in scope of automated scheduling - /// decisions. - /// 3. For each cluster, it sums up `cluster_scheduling_decisions`, checks the summed up decision - /// against the cluster state, and turns cluster On/Off if needed. - #[mz_ore::instrument(level = "debug")] - pub(crate) async fn handle_scheduling_decisions( - &mut self, - decisions: Vec<(&'static str, Vec<(ClusterId, SchedulingDecision)>)>, - ) { - // When the cluster controller owns the replica set it is the sole writer - // for scheduled clusters. Drop any legacy decisions still in flight from a - // background task spawned before the gate flipped on, so the two never - // contend. (`check_scheduling_policies` already stops spawning new ones.) - if ENABLE_CLUSTER_CONTROLLER.get(self.catalog().system_config().dyncfgs()) { - return; - } - - let start_time = Instant::now(); - - // 1. Add the received decisions to `cluster_scheduling_decisions`. - for (policy_name, decisions) in decisions.iter() { - for (cluster_id, decision) in decisions { - self.cluster_scheduling_decisions - .entry(*cluster_id) - .or_insert_with(Default::default) - .insert(policy_name, decision.clone()); - } - } - - // 2. Clean up those clusters from `scheduling_decisions` that - // - have been dropped, or - // - were switched to unmanaged, or - // - were switched to `SCHEDULE = MANUAL`. - for cluster_id in self - .cluster_scheduling_decisions - .keys() - .cloned() - .collect_vec() - { - match self.get_managed_cluster_config(cluster_id) { - None => { - // Cluster have been dropped or switched to unmanaged. - debug!( - "handle_scheduling_decisions: \ - Removing cluster {} from cluster_scheduling_decisions, \ - because get_managed_cluster_config returned None", - cluster_id - ); - self.cluster_scheduling_decisions.remove(&cluster_id); - } - Some(managed_config) => { - if matches!(managed_config.schedule, ClusterSchedule::Manual) { - debug!( - "handle_scheduling_decisions: \ - Removing cluster {} from cluster_scheduling_decisions, \ - because schedule is Manual", - cluster_id - ); - self.cluster_scheduling_decisions.remove(&cluster_id); - } - } - } - } - - // 3. Act on `scheduling_decisions` where needed. - let mut altered_a_cluster = false; - for (cluster_id, decisions) in self.cluster_scheduling_decisions.clone() { - // We touch a cluster only when all policies have made a decision about it. This is - // to ensure that after an envd restart all policies have a chance to run at least once - // before we turn off a cluster, to avoid spuriously turning off a cluster and possibly - // losing a hydrated state. - if POLICIES.iter().all(|policy| decisions.contains_key(policy)) { - // Check whether the cluster's state matches the needed state. - // If any policy says On, then we need a replica. - let needs_replica = decisions - .values() - .map(|decision| decision.cluster_on()) - .contains(&true); - let cluster = self.catalog().get_cluster(cluster_id); - let cluster_name = cluster.name().to_string(); - let cluster_config = cluster.config.clone(); - // NOTE: the durable replication factor is not a reliable - // on/off signal here. The cluster controller runs a scheduled - // cluster's replica while holding the factor at 0, so after a - // controller gate-off the factor can disagree with the replica - // set that actually exists. Decide from the physical replicas, - // with the same exclusions as the controller's ownership test - // (`ObservedReplica::owned_shape`): internal and billed-as - // replicas are manually managed, pending ones belong to an - // in-flight reconfiguration. In a pure legacy world the factor - // and the replica set always agree, so both signals give the - // same answer there. - let owned_replicas: Vec<_> = cluster - .replicas() - .filter(|r| { - !r.config.location.internal() - && r.config.location.billed_as().is_none() - && !r.config.location.pending() - }) - .map(|r| r.replica_id) - .collect(); - let has_pending_replica = cluster.replicas().any(|r| r.config.location.pending()); - let mut new_config = cluster_config.clone(); - let ClusterVariant::Managed(managed_config) = &mut new_config.variant else { - panic!("cleaned up unmanaged clusters above"); - }; - let replication_factor = managed_config.replication_factor; - let has_replica = !owned_replicas.is_empty(); // Is it On? - let reason = crate::catalog::ReplicaCreateDropReason::ClusterScheduling( - decisions.values().cloned().collect(), - ); - if has_pending_replica { - // A graceful reconfiguration owns the replica set until it - // finalizes. The turn-on alter below would reject this - // case itself, the direct drop and adopt paths must not - // race the finalization either. This covers only legacy - // `-pending` replicas: controller-created overlap replicas - // are not pending and are handled by the adopt and - // turn-off branches instead. - debug!( - "handle_scheduling_decisions skipped cluster {} because it is \ - undergoing a graceful reconfiguration", - cluster_id - ); - } else if needs_replica && !has_replica { - // Turn the cluster On. - altered_a_cluster = true; - managed_config.replication_factor = 1; - if let Err(e) = self - .sequence_alter_cluster_managed_to_managed( - None, - cluster_id, - new_config.clone(), - reason, - AlterClusterPlanStrategy::None, - ) - .await - { - if let AdapterError::AlterClusterWhilePendingReplicas = e { - debug!( - "handle_scheduling_decisions tried to alter a cluster that is undergoing a graceful reconfiguration" - ); - } else { - soft_panic_or_log!( - "handle_scheduling_decisions couldn't alter cluster {}. \ - Old config: {:?}, \ - New config: {:?}, \ - Error: {}", - cluster_id, - cluster_config, - new_config, - e - ); - } - } - } else if !needs_replica && has_replica { - // Turn the cluster Off. Drop the replicas by id rather - // than altering the factor down: a replica handed over by - // the controller exists while the factor is already 0 (an - // alter to 0 would be a no-op there), and it may not sit - // at the canonical `r` name a factor-derived drop - // would look for. - altered_a_cluster = true; - let drops = owned_replicas - .into_iter() - .map(|replica_id| { - crate::catalog::DropObjectInfo::ClusterReplica(( - cluster_id, - replica_id, - reason.clone(), - )) - }) - .collect(); - managed_config.replication_factor = 0; - let reconfiguration_audit = cancel_carried_reconfiguration(&mut new_config); - let mut ops = vec![crate::catalog::Op::DropObjects(drops)]; - // After a controller handoff the factor is already 0 and - // there is usually no record to retire, so the config - // write would be a no-op. Push it only when something - // actually changed. - if new_config != cluster_config || reconfiguration_audit.is_some() { - ops.push(crate::catalog::Op::UpdateClusterConfig { - id: cluster_id, - name: cluster_name, - config: new_config.clone(), - reconfiguration_audit, - burst_audit: None, - }); - } - if let Err(e) = self.catalog_transact(None, ops).await { - soft_panic_or_log!( - "handle_scheduling_decisions couldn't turn off cluster {}. \ - Old config: {:?}, \ - New config: {:?}, \ - Error: {}", - cluster_id, - cluster_config, - new_config, - e - ); - } - } else if needs_replica && replication_factor == 0 { - // The controller left in-window replicas behind on - // gate-off (`has_replica` is true here). Adopt exactly - // one: the scheduled-cluster invariant caps the factor at - // 1 (the planner refuses higher, and `unplan` asserts it), - // so the lowest-id replica is kept, the factor is aligned - // with it so later decisions and user `ALTER`s see a - // consistent on-state, and any surplus is retired in the - // same transaction. Surplus replicas are possible when a - // pre-schedule reconfiguration's overlap replica was live - // at gate-off, those are not marked pending. Nothing is - // created. - altered_a_cluster = true; - let mut owned_replicas = owned_replicas; - owned_replicas.sort_unstable(); - let surplus = owned_replicas.split_off(1); - managed_config.replication_factor = 1; - let reconfiguration_audit = cancel_carried_reconfiguration(&mut new_config); - let mut ops = Vec::new(); - if !surplus.is_empty() { - let drops = surplus - .into_iter() - .map(|replica_id| { - crate::catalog::DropObjectInfo::ClusterReplica(( - cluster_id, - replica_id, - reason.clone(), - )) - }) - .collect(); - ops.push(crate::catalog::Op::DropObjects(drops)); - } - ops.push(crate::catalog::Op::UpdateClusterConfig { - id: cluster_id, - name: cluster_name, - config: new_config.clone(), - reconfiguration_audit, - burst_audit: None, - }); - if let Err(e) = self.catalog_transact(None, ops).await { - soft_panic_or_log!( - "handle_scheduling_decisions couldn't adopt replicas of cluster {}. \ - Old config: {:?}, \ - New config: {:?}, \ - Error: {}", - cluster_id, - cluster_config, - new_config, - e - ); - } - } - } else { - debug!( - "handle_scheduling_decisions: \ - Not all policies have made a decision about cluster {}. decisions: {:?}", - cluster_id, decisions, - ); - } - } - - self.metrics - .handle_scheduling_decisions_seconds - .with_label_values(&[altered_a_cluster.to_string().as_str()]) - .observe((Instant::now() - start_time).as_secs_f64()); - } - - /// Returns the managed config for a cluster. Returns None if the cluster doesn't exist or if - /// it's an unmanaged cluster. - fn get_managed_cluster_config(&self, cluster_id: ClusterId) -> Option { - let cluster = self.catalog().try_get_cluster(cluster_id)?; - if let ClusterVariant::Managed(managed_config) = cluster.config.variant.clone() { - Some(managed_config) - } else { - None - } - } -} diff --git a/src/adapter/src/coord/command_handler.rs b/src/adapter/src/coord/command_handler.rs index 45f582cf42cee..e17c438125a6e 100644 --- a/src/adapter/src/coord/command_handler.rs +++ b/src/adapter/src/coord/command_handler.rs @@ -839,7 +839,6 @@ impl Coordinator { secret_key, notice_tx, drop_sinks: BTreeSet::new(), - pending_cluster_alters: BTreeSet::new(), connected_at: self.now(), user, application_name, @@ -1984,8 +1983,6 @@ impl Coordinator { // SQL cancellation has no success response to delay. Each subscribe // still waits for its own retraction before it observes retirement. drop(retire_notify); - self.cancel_cluster_reconfigurations_for_conn(&conn_id) - .await; self.cancel_pending_copy(&conn_id); if let Some((tx, _rx)) = self.connection_cancel_watches.get_mut(&conn_id) { let _ = tx.send(true); diff --git a/src/adapter/src/coord/ddl.rs b/src/adapter/src/coord/ddl.rs index a9df4730f3cc4..d671f96ed6391 100644 --- a/src/adapter/src/coord/ddl.rs +++ b/src/adapter/src/coord/ddl.rs @@ -51,7 +51,7 @@ use serde_json::json; use tracing::{Instrument, Level, event, info_span, warn}; use crate::active_compute_sink::{ActiveComputeSink, ActiveComputeSinkRetireReason}; -use crate::catalog::{DropObjectInfo, Op, ReplicaCreateDropReason, TransactionResult}; +use crate::catalog::{DropObjectInfo, Op, TransactionResult}; use crate::coord::Coordinator; use crate::coord::appends::{BuiltinTableAppendCompletion, BuiltinTableAppendNotify}; use crate::coord::catalog_implications::parsed_state_updates::ParsedStateUpdate; @@ -859,43 +859,6 @@ impl Coordinator { })) } - /// Drops all pending replicas for a set of clusters - /// that are undergoing reconfiguration. - pub async fn drop_reconfiguration_replicas( - &mut self, - cluster_ids: BTreeSet, - ) -> Result<(), AdapterError> { - let pending_cluster_ops: Vec = cluster_ids - .iter() - .map(|c| { - self.catalog() - .get_cluster(c.clone()) - .replicas() - .filter_map(|r| match r.config.location { - ReplicaLocation::Managed(ref l) if l.pending => { - Some(DropObjectInfo::ClusterReplica(( - c.clone(), - r.replica_id, - ReplicaCreateDropReason::Manual, - ))) - } - _ => None, - }) - .collect::>() - }) - .filter_map(|pending_replica_drop_ops_by_cluster| { - match pending_replica_drop_ops_by_cluster.len() { - 0 => None, - _ => Some(Op::DropObjects(pending_replica_drop_ops_by_cluster)), - } - }) - .collect(); - if !pending_cluster_ops.is_empty() { - self.catalog_transact(None, pending_cluster_ops).await?; - } - Ok(()) - } - /// Cancels all active compute sinks for the identified connection. #[mz_ore::instrument(level = "debug")] pub(crate) async fn cancel_compute_sinks_for_conn( @@ -906,15 +869,6 @@ impl Coordinator { .await } - /// Cancels all active cluster reconfigurations sinks for the identified connection. - #[mz_ore::instrument(level = "debug")] - pub(crate) async fn cancel_cluster_reconfigurations_for_conn( - &mut self, - conn_id: &ConnectionId, - ) { - self.retire_cluster_reconfigurations_for_conn(conn_id).await - } - /// Retires all active compute sinks for the identified connection with the /// specified reason. #[mz_ore::instrument(level = "debug")] @@ -934,30 +888,6 @@ impl Coordinator { self.retire_compute_sinks(drop_sinks).await } - /// Cleans pending cluster reconfiguraiotns for the identified connection - #[mz_ore::instrument(level = "debug")] - pub(crate) async fn retire_cluster_reconfigurations_for_conn( - &mut self, - conn_id: &ConnectionId, - ) { - let reconfiguring_clusters = self - .active_conns - .get(conn_id) - .expect("must exist for active session") - .pending_cluster_alters - .clone(); - // try to drop reconfig replicas - self.drop_reconfiguration_replicas(reconfiguring_clusters) - .await - .unwrap_or_terminate("cannot fail to drop reconfiguration replicas"); - - self.active_conns - .get_mut(conn_id) - .expect("must exist for active session") - .pending_cluster_alters - .clear(); - } - pub(crate) fn drop_storage_sinks(&mut self, sink_gids: Vec) { let storage_metadata = self.catalog.state().storage_metadata(); self.controller diff --git a/src/adapter/src/coord/message_handler.rs b/src/adapter/src/coord/message_handler.rs index 988000c1abfb4..71c81667b1055 100644 --- a/src/adapter/src/coord/message_handler.rs +++ b/src/adapter/src/coord/message_handler.rs @@ -245,14 +245,6 @@ impl Coordinator { ); } } - Message::CheckSchedulingPolicies => { - self.check_scheduling_policies().boxed_local().await; - } - Message::SchedulingDecisions(decisions) => { - self.handle_scheduling_decisions(decisions) - .boxed_local() - .await; - } Message::ClusterControllerRequest(request) => { self.handle_cluster_controller_request(request) .boxed_local() diff --git a/src/adapter/src/coord/sequencer.rs b/src/adapter/src/coord/sequencer.rs index c2c22fc996c83..f6f56f204796b 100644 --- a/src/adapter/src/coord/sequencer.rs +++ b/src/adapter/src/coord/sequencer.rs @@ -94,7 +94,6 @@ use crate::util::ClientTransmitter; // big refactoring after the old peek sequencing is removed. mod inner; -pub(crate) use inner::cancel_carried_reconfiguration; impl Coordinator { /// BOXED FUTURE: As of Nov 2023 the returned Future from this function was 34KB. This would diff --git a/src/adapter/src/coord/sequencer/inner.rs b/src/adapter/src/coord/sequencer/inner.rs index 4b62df13d3edb..f979d56383ef1 100644 --- a/src/adapter/src/coord/sequencer/inner.rs +++ b/src/adapter/src/coord/sequencer/inner.rs @@ -126,7 +126,6 @@ use crate::{PeekResponseUnary, ReadHolds}; type RtrTimestampFuture = BoxFuture<'static, Result>; mod cluster; -pub(crate) use cluster::cancel_carried_reconfiguration; mod copy_from; mod create_index; mod create_materialized_view; diff --git a/src/adapter/src/coord/sequencer/inner/cluster.rs b/src/adapter/src/coord/sequencer/inner/cluster.rs index 4fd7b174d467a..b6fdcdd6b686b 100644 --- a/src/adapter/src/coord/sequencer/inner/cluster.rs +++ b/src/adapter/src/coord/sequencer/inner/cluster.rs @@ -7,22 +7,24 @@ // the Business Source License, use of this software will be governed // by the Apache License, Version 2.0. -use std::collections::{BTreeMap, BTreeSet}; -use std::time::{Duration, Instant}; +use std::collections::BTreeSet; +use std::time::Duration; use itertools::Itertools; -use maplit::btreeset; use mz_adapter_types::cluster_state::ReconfigurationAudit; use mz_catalog::builtin::BUILTINS; use mz_catalog::durable::managed_cluster_replica_name; use mz_catalog::memory::error::ErrorKind; use mz_catalog::memory::objects::{ - ClusterConfig, ClusterReplica, ClusterVariant, ClusterVariantManaged, - ManagedReplicaConfigShape, ReconfigurationState, ReconfigurationStatus, ReconfigurationTarget, + ClusterConfig, ClusterVariant, ClusterVariantManaged, ManagedReplicaConfigShape, + ReconfigurationState, ReconfigurationStatus, ReconfigurationTarget, }; +use mz_cluster_controller::ctx::{AvailabilityZones, CreateReason, Decision, ReplicaShape}; +use mz_cluster_controller::reconcile_replicas; +use mz_cluster_controller::strategy::DesiredReplica; use mz_compute_types::config::ComputeReplicaConfig; use mz_controller::clusters::{ - ClusterStatus, ManagedReplicaLocation, ReplicaConfig, ReplicaLocation, ReplicaLogging, + ManagedReplicaLocation, ReplicaConfig, ReplicaLocation, ReplicaLogging, }; use mz_controller_types::{ClusterId, DEFAULT_REPLICA_LOGGING_INTERVAL, ReplicaId}; use mz_ore::cast::CastFrom; @@ -31,49 +33,41 @@ use mz_ore::instrument; use mz_repr::Timestamp; use mz_repr::adt::numeric::Numeric; use mz_repr::role_id::RoleId; -use mz_sql::ast::{Ident, QualifiedReplica}; use mz_sql::catalog::{CatalogCluster, CatalogError, ObjectType}; use mz_sql::plan::{ self, AlterClusterPlanStrategy, AlterClusterRenamePlan, AlterClusterReplicaRenamePlan, - AlterClusterSwapPlan, AlterOptionParameter, AlterSetClusterPlan, - ComputeReplicaIntrospectionConfig, CreateClusterManagedPlan, CreateClusterPlan, - CreateClusterReplicaPlan, CreateClusterUnmanagedPlan, CreateClusterVariant, PlanClusterOption, + AlterClusterSwapPlan, AlterOptionParameter, AlterSetClusterPlan, CreateClusterManagedPlan, + CreateClusterPlan, CreateClusterReplicaPlan, CreateClusterUnmanagedPlan, CreateClusterVariant, + PlanClusterOption, }; use mz_sql::plan::{AlterClusterPlan, OnTimeoutAction}; use mz_sql::session::metadata::SessionMetadata; use mz_sql::session::vars::{ MAX_CREDIT_CONSUMPTION_RATE, MAX_REPLICAS_PER_CLUSTER, SystemVars, Var, }; -use tracing::{Instrument, Span, debug}; +use tracing::{Instrument, Span}; use mz_adapter_types::dyncfgs::{ DEFAULT_CLUSTER_RECONFIGURATION_TIMEOUT, ENABLE_BACKGROUND_ALTER_CLUSTER, - ENABLE_CLUSTER_CONTROLLER, }; use super::return_if_err; -use crate::AdapterError::AlterClusterWhilePendingReplicas; use crate::catalog::{self, Op, ReplicaCreateDropReason}; use crate::config::{ ClusterEvalContext, ClusterScopeContext, ReplicaEvalContext, ReplicaScopeContext, }; use crate::coord::{ - AlterCluster, AlterClusterAwaitReconfiguration, AlterClusterFinalize, - AlterClusterWaitForHydrated, ClusterReplicaStatuses, ClusterStage, Coordinator, Message, + AlterCluster, AlterClusterAwaitReconfiguration, ClusterStage, Coordinator, Message, PlanValidity, StageResult, Staged, }; use crate::{AdapterError, AdapterNotice, ExecuteContext, ExecuteResponse, session::Session}; -const PENDING_REPLICA_SUFFIX: &str = "-pending"; - impl Staged for ClusterStage { type Ctx = ExecuteContext; fn validity(&mut self) -> &mut PlanValidity { match self { Self::Alter(stage) => &mut stage.validity, - Self::WaitForHydrated(stage) => &mut stage.validity, - Self::Finalize(stage) => &mut stage.validity, Self::AwaitReconfiguration(stage) => &mut stage.validity, } } @@ -89,37 +83,6 @@ impl Staged for ClusterStage { .sequence_alter_cluster_stage(ctx.session(), stage.plan.clone(), stage.validity) .await } - Self::WaitForHydrated(stage) => { - let AlterClusterWaitForHydrated { - validity, - plan, - new_config, - workload_class, - timeout_time, - on_timeout, - } = stage; - coord - .check_if_pending_replicas_hydrated_stage( - ctx.session(), - plan, - new_config, - workload_class, - timeout_time, - on_timeout, - validity, - ) - .await - } - Self::Finalize(stage) => { - coord - .finalize_alter_cluster_stage( - ctx.session(), - stage.plan.clone(), - stage.new_config.clone(), - stage.workload_class.clone(), - ) - .await - } Self::AwaitReconfiguration(stage) => { coord.await_reconfiguration_stage(stage.validity, stage.cluster_id, stage.target) } @@ -311,15 +274,6 @@ impl Coordinator { Unchanged => {} } - // The controller owns only *user* managed clusters (see `ManagedClusterIds` - // in cluster_controller.rs and `controller_owns` in the managed-to-managed - // path below). A system/builtin cluster is never converged by the - // controller, so it must not be reshaped into a durable reconfiguration - // record nobody would cut over. It takes the direct realized-config path - // below, exactly as it does with the controller off. - let cluster_controller_owns = ENABLE_CLUSTER_CONTROLLER - .get(self.catalog().system_config().dyncfgs()) - && cluster_id.is_user(); let reconfiguration_in_flight = matches!( &config.variant, Managed(managed) if managed @@ -333,23 +287,17 @@ impl Coordinator { // never writes a reconfiguration record for a scheduled cluster (see // the routing below). Refuse flipping the schedule under an in-flight // record rather than let the two ownership regimes overlap mid-flight. - if cluster_controller_owns - && reconfiguration_in_flight - && !matches!(options.schedule, Unchanged) - { + if reconfiguration_in_flight && !matches!(options.schedule, Unchanged) { return Err(AdapterError::AlterClusterScheduleWhileReconfiguring); } - // Replication factor is one of the four dimensions the cut-over sets + // Replication factor is one of the dimensions the cut-over sets // atomically from the record's target (`fold_reconfiguration_target`), // so a change applied independently while a reconfiguration is in // flight would be silently clobbered at cut-over. Refused even when the // same statement also re-targets the shape, so a record's target // replication factor is always the one it started with. - if cluster_controller_owns - && reconfiguration_in_flight - && !matches!(options.replication_factor, Unchanged) - { + if reconfiguration_in_flight && !matches!(options.replication_factor, Unchanged) { return Err(AdapterError::AlterClusterReplicationFactorWhileReconfiguring); } @@ -359,58 +307,75 @@ impl Coordinator { // the reshape path below to cancel the record. let cancels_or_retargets = reconfiguration_in_flight && alter_changes_replica_shape(options); - if new_config == config && !(cluster_controller_owns && cancels_or_retargets) { + if new_config == config && !cancels_or_retargets { return Ok(StageResult::Response(ExecuteResponse::AlteredObject( ObjectType::Cluster, ))); } - // When the controller owns the replica set, a shape-changing `ALTER` - // reshapes into a durable `reconfiguration` record (starting, - // retargeting, or cancelling one) instead of going through the legacy - // 3-stage machine. Everything else falls through to the realized-config - // update below without touching the record, in flight or not. + // A shape-changing `ALTER` reshapes into a durable `reconfiguration` + // record (starting, retargeting, or cancelling one) that the controller + // converges on, unless the statement asks for the cut-over to happen + // right now, which routes to the direct path below instead. Everything + // else falls through to the realized-config update below without + // touching the record, in flight or not. // // With a record in flight the statement decides: an `ALTER` back to the // realized shape is value-identical yet must reach the reshape path to // cancel. With nothing in flight the values decide: a shape option set // to its current value reconfigures nothing, and reshaping it anyway // would write a spurious pre-cancelled record. - if cluster_controller_owns { - if let (Managed(old_managed), Managed(new_managed)) = - (&config.variant, &new_config.variant) - { - let needs_record = if reconfiguration_in_flight { - alter_changes_replica_shape(options) + // + // The target the cut-over must materialize: the shape and factor the + // statement asks for, folded onto any in-flight record's target, + // exactly as the reshape path computes it. Applied to `new_config` + // below, once the borrow taken here is released. With nothing in + // flight the fold returns the statement's shape unchanged, so applying + // it is the identity. + let mut cut_over_target = None; + if let (Managed(old_managed), Managed(new_managed)) = (&config.variant, &new_config.variant) + { + let needs_record = if reconfiguration_in_flight { + alter_changes_replica_shape(options) + } else { + new_managed.replica_config_shape() != old_managed.replica_config_shape() + }; + // A scheduled (non-MANUAL) cluster holds its replication factor + // at 0 and the on-refresh strategy owns its replica set, so a + // graceful hydrate-overlap has nothing meaningful to wait for. + // A config-shape `ALTER` on such a cluster takes the direct + // path below instead of writing a record: that path only updates + // the realized config, and the controller reconciles any in-window + // replica to the new shape on its next tick. The schedule guard + // above keeps a schedule change from reaching here mid-record, so a + // record on a scheduled cluster can only pre-date the schedule + // (written on an older version). For that case the reshape + // path stays reachable, so the record can still be retargeted + // or cancelled until it settles. + let scheduled_direct = + !matches!(new_managed.schedule, mz_sql::plan::ClusterSchedule::Manual) + && !reconfiguration_in_flight; + // A `WAIT` option would be silently vacuous on the direct + // path: there may be no replica at all (window closed), and + // an in-window replica is bounced to the new shape without a + // hydrate-overlap to wait on. Reject it rather than return an + // instant success that waited for nothing, mirroring the + // planner's rejection of a `WAIT` without a shape change. + if scheduled_direct && !matches!(strategy, AlterClusterPlanStrategy::None) { + return Err(AdapterError::AlterClusterWaitOnScheduledCluster); + } + if needs_record && !scheduled_direct { + if requests_immediate_cut_over(strategy) { + let in_flight = old_managed + .reconfiguration + .as_ref() + .filter(|record| record.is_in_progress()); + cut_over_target = Some(alter_reconfiguration_target( + new_managed, + options, + in_flight.map(|record| &record.target), + )); } else { - new_managed.replica_config_shape() != old_managed.replica_config_shape() - }; - // A scheduled (non-MANUAL) cluster holds its replication factor - // at 0 and the on-refresh strategy owns its replica set, so a - // graceful hydrate-overlap has nothing meaningful to wait for. - // A config-shape `ALTER` on such a cluster takes the direct - // path below instead of writing a record: with the controller - // owning the cluster the direct path only updates the realized - // config, and the controller reconciles any in-window replica - // to the new shape on its next tick. The schedule guard above - // keeps a schedule change from reaching here mid-record, so a - // record on a scheduled cluster can only pre-date the schedule - // (written on an older version). For that case the reshape - // path stays reachable, so the record can still be retargeted - // or cancelled until it settles. - let scheduled_direct = - !matches!(new_managed.schedule, mz_sql::plan::ClusterSchedule::Manual) - && !reconfiguration_in_flight; - // A `WAIT` option would be silently vacuous on the direct - // path: there may be no replica at all (window closed), and - // an in-window replica is bounced to the new shape without a - // hydrate-overlap to wait on. Reject it rather than return an - // instant success that waited for nothing, mirroring the - // planner's rejection of a `WAIT` without a shape change. - if scheduled_direct && !matches!(strategy, AlterClusterPlanStrategy::None) { - return Err(AdapterError::AlterClusterWaitOnScheduledCluster); - } - if needs_record && !scheduled_direct { return self .reshape_alter_cluster_managed( session, @@ -424,70 +389,23 @@ impl Coordinator { } } } + let cut_over = cut_over_target.is_some(); + if let Some(target) = cut_over_target { + let Managed(target_managed) = &mut new_config.variant else { + unreachable!("a cut-over target is produced only for a managed config"); + }; + target_managed.apply_reconfiguration_target(target); + } match (&config.variant, &new_config.variant) { - (Managed(_), Managed(new_config_managed)) => { - let alter_followup = self - .sequence_alter_cluster_managed_to_managed( - Some(session), - cluster_id, - new_config.clone(), - ReplicaCreateDropReason::Manual, - strategy.clone(), - ) - .await?; - if alter_followup == NeedsFinalization::Yes { - // For non backgrounded zero-downtime alters, store the - // cluster_id in the ConnMeta to allow for cancellation. - self.active_conns - .get_mut(session.conn_id()) - .expect("There must be an active connection") - .pending_cluster_alters - .insert(cluster_id.clone()); - let new_config_managed = new_config_managed.clone(); - return match &strategy { - AlterClusterPlanStrategy::None => Err(AdapterError::Internal( - "AlterClusterPlanStrategy must not be None if NeedsFinalization is Yes" - .into(), - )), - AlterClusterPlanStrategy::For(duration) => { - let span = Span::current(); - let plan = plan.clone(); - let duration = duration.clone().to_owned(); - let workload_class = new_config.workload_class.clone(); - Ok(StageResult::Handle(mz_ore::task::spawn( - || "Finalize Alter Cluster", - async move { - tokio::time::sleep(duration).await; - let stage = ClusterStage::Finalize(AlterClusterFinalize { - validity, - plan, - new_config: new_config_managed, - workload_class, - }); - Ok(Box::new(stage)) - } - .instrument(span), - ))) - } - AlterClusterPlanStrategy::UntilReady { - timeout, - on_timeout, - } => Ok(StageResult::Immediate(Box::new( - ClusterStage::WaitForHydrated(AlterClusterWaitForHydrated { - validity, - plan: plan.clone(), - new_config: new_config_managed.clone(), - workload_class: new_config.workload_class.clone(), - timeout_time: Instant::now() + timeout.to_owned(), - // The legacy foreground wait uses COMMIT as - // its implicit default. The controller-owned - // paths default to ROLLBACK. - on_timeout: on_timeout.unwrap_or(OnTimeoutAction::Commit), - }), - ))), - }; - } + (Managed(_), Managed(_)) => { + self.sequence_alter_cluster_managed_to_managed( + session, + cluster_id, + new_config.clone(), + cut_over, + ) + .await?; } (Unmanaged, Managed(new_managed)) => { // The conversion path creates no overlap replicas to wait on, @@ -533,8 +451,10 @@ impl Coordinator { cluster_id: ClusterId, target: &ReconfigurationTarget, ) -> Result<(), AdapterError> { - // Only user clusters are converged by the controller and counted against - // these limits. A system cluster never reshapes into a record. + // System clusters are exempt from `max_replicas_per_cluster` and from + // credit accounting everywhere else (see the `is_user` guards in + // `catalog_transact`'s validation), so a reconfiguration of one has no + // budget to fit either. if !cluster_id.is_user() { return Ok(()); } @@ -556,12 +476,10 @@ impl Coordinator { // realized and target sets side by side until cut-over, so this cluster's // peak contribution is both shapes at once, computed from config as // realized plus target. That slightly over-counts a same-shape overlap, - // where existing replicas double as target replicas, but it matches the - // legacy wait path, which creates the full target set as pending replicas - // at `ALTER` time and therefore enforces both limits on the overlap. - // Rejecting here is also strictly better than the asynchronous abort the - // controller falls back to when a limit shrinks or the environment grows - // after the record is written. + // where existing replicas double as target replicas. We accept the + // over-count: rejecting here is strictly better than the asynchronous + // abort the controller falls back to when a limit shrinks or the + // environment grows after the record is written. // Per-cluster replica count: the peak is `realized_rf + target_rf`, // deterministic from the cluster's own config. `validate_resource_limit` @@ -631,15 +549,8 @@ impl Coordinator { /// to the realized config immediately. The controller converges the replica /// set onto the target and cuts the realized shape over at hydration. /// - /// **Fold semantics.** When a record is already in flight, the target is an - /// overlay on the *in-flight target*, not the realized config: a dimension the - /// `ALTER` set (`options.*` is `Set`/`Reset`) takes the new value, a dimension - /// left `Unchanged` keeps the in-flight target's value. `new_config` was built - /// against the realized config, which still holds the pre-reconfiguration shape - /// (the realized config is advanced only at cut-over), so seeding `Unchanged` - /// dimensions from it would silently revert the in-flight transition along any - /// dimension this `ALTER` did not mention. With no record in flight there is - /// nothing to fold and the target is exactly `new_config`'s shape. + /// The target is folded onto any in-flight one by + /// [`alter_reconfiguration_target`]. /// /// **Timeout action.** The record carries an `on_timeout` action (resolved /// from `WITH (WAIT ...)`, defaulting to `ROLLBACK`), which the controller @@ -666,19 +577,12 @@ impl Coordinator { strategy: &AlterClusterPlanStrategy, validity: PlanValidity, ) -> Result>, AdapterError> { - use mz_sql::plan::AlterOptionParameter::Unchanged; - let ClusterVariant::Managed(new_managed) = &new_config.variant else { return Err(AdapterError::Internal( "reshape_alter_cluster_managed requires a managed target config".into(), )); }; - // Fold onto the in-flight target when one exists: `new_config` carries the - // realized value for any dimension the `ALTER` left `Unchanged`, but the - // realized config is the pre-reconfiguration shape, so we instead carry the - // in-flight target's value for those dimensions. Only dimensions the `ALTER` - // explicitly set diverge from the in-flight target. let cluster = self.catalog.get_cluster(cluster_id); let in_flight = match &cluster.config.variant { ClusterVariant::Managed(managed) => managed @@ -688,27 +592,10 @@ impl Coordinator { .cloned(), ClusterVariant::Unmanaged => None, }; - let new_target = ReconfigurationTarget { - size: new_managed.size.clone(), - replication_factor: new_managed.replication_factor, - availability_zones: new_managed.availability_zones.clone(), - logging: new_managed.logging.clone(), - arrangement_compression: new_managed.arrangement_compression, - }; - let unchanged = ReconfigurationDimensionsUnchanged { - size: matches!(options.size, Unchanged), - replication_factor: matches!(options.replication_factor, Unchanged), - availability_zones: matches!(options.availability_zones, Unchanged), - // The two logging options fold independently, so a debugging-only - // `ALTER` cannot revert an in-flight interval change (or vice versa). - log_logging: matches!(options.introspection_debugging, Unchanged), - interval: matches!(options.introspection_interval, Unchanged), - arrangement_compression: matches!(options.arrangement_compression, Unchanged), - }; - let target = fold_reconfiguration_target( + let target = alter_reconfiguration_target( + new_managed, + options, in_flight.as_ref().map(|r| &r.target), - new_target, - unchanged, ); // Validate the target up front, so a bad reshape errors at `ALTER` time @@ -752,7 +639,7 @@ impl Coordinator { // its target may already have. `ROLLBACK` (the default) reverts an // un-hydrated reconfiguration to its pre-reconfiguration shape rather // than cutting over to a not-yet-hydrated target, which could induce - // downtime. The legacy foreground path uses implicit `COMMIT`. + // downtime. let now = self.now(); let deadline_from = |timeout: Duration| -> Timestamp { now.saturating_add(u64::try_from(timeout.as_millis()).unwrap_or(u64::MAX)) @@ -782,13 +669,12 @@ impl Coordinator { }; // Build the durable write from `new_config`, which carries every field the - // `ALTER` changed, then reset the config *shape* (size, replication factor, - // availability zones, logging) back to the realized values: that transition - // is deferred to the `reconfiguration` record and applied at cut-over. This - // applies non-shape changes (`workload_class`, `schedule`, - // `auto_scaling_strategy`, ...) immediately, matching the legacy path, - // rather than silently dropping them. Any existing record is folded over by - // the `record` we just built. + // `ALTER` changed, then reset the config *shape* (every + // `ReconfigurationTarget` dimension) back to the realized values: that + // transition is deferred to the `reconfiguration` record and applied at + // cut-over. This applies non-shape changes (`workload_class`, `schedule`, + // `auto_scaling_strategy`, ...) immediately rather than silently dropping + // them. Any existing record is folded over by the `record` we just built. let cluster = self.catalog.get_cluster(cluster_id); let cluster_name = cluster.name().to_string(); let ClusterVariant::Managed(realized_now) = &cluster.config.variant else { @@ -796,10 +682,7 @@ impl Coordinator { "reshape_alter_cluster_managed requires a managed realized config".into(), )); }; - let realized_size = realized_now.size.clone(); - let realized_replication_factor = realized_now.replication_factor; - let realized_availability_zones = realized_now.availability_zones.clone(); - let realized_logging = realized_now.logging.clone(); + let realized_target = realized_now.realized_reconfiguration_target(); // The status and the audit intent are two views of the same decision, // made together here: an ALTER back to the realized shape is a cancel, // anything else starts (or re-targets) a reconfiguration. @@ -827,10 +710,7 @@ impl Coordinator { "reshape_alter_cluster_managed requires a managed target config".into(), )); }; - realized_managed.size = realized_size; - realized_managed.replication_factor = realized_replication_factor; - realized_managed.availability_zones = realized_availability_zones; - realized_managed.logging = realized_logging; + realized_managed.apply_reconfiguration_target(realized_target); realized_managed.reconfiguration = Some(record); self.catalog_transact( @@ -951,258 +831,6 @@ impl Coordinator { } } - async fn finalize_alter_cluster_stage( - &mut self, - session: &Session, - AlterClusterPlan { - id: cluster_id, - name: cluster_name, - .. - }: AlterClusterPlan, - new_config: ClusterVariantManaged, - workload_class: Option, - ) -> Result>, AdapterError> { - let cluster = self.catalog.get_cluster(cluster_id); - let mut ops = vec![]; - - // Gather the ops to remove the non pending replicas - // Also skip any billed_as free replicas - let remove_replicas = cluster - .replicas() - .filter_map(|r| { - if !r.config.location.pending() && !r.config.location.internal() { - Some(catalog::DropObjectInfo::ClusterReplica(( - cluster_id.clone(), - r.replica_id, - ReplicaCreateDropReason::Manual, - ))) - } else { - None - } - }) - .collect(); - ops.push(catalog::Op::DropObjects(remove_replicas)); - - // Gather the Ops to remove the "-pending" suffix from the name and set - // pending to false - let finalize_replicas: Vec = cluster - .replicas() - .filter_map(|r| { - if r.config.location.pending() { - let cluster_ident = match Ident::new(cluster.name.clone()) { - Ok(id) => id, - Err(err) => { - return Some(Err(AdapterError::internal( - "Unexpected error parsing cluster name", - err, - ))); - } - }; - let replica_ident = match Ident::new(r.name.clone()) { - Ok(id) => id, - Err(err) => { - return Some(Err(AdapterError::internal( - "Unexpected error parsing replica name", - err, - ))); - } - }; - Some(Ok((cluster_ident, replica_ident, r))) - } else { - None - } - }) - // Early collection is to handle errors from generating of the - // Idents - .collect::, _>>()? - .into_iter() - .map(|(cluster_ident, replica_ident, replica)| { - let mut new_replica_config = replica.config.clone(); - debug!("Promoting replica: {}", replica.name); - match new_replica_config.location { - mz_controller::clusters::ReplicaLocation::Managed(ManagedReplicaLocation { - ref mut pending, - .. - }) => { - *pending = false; - } - mz_controller::clusters::ReplicaLocation::Unmanaged(_) => {} - } - - let mut replica_ops = vec![]; - let to_name = replica.name.strip_suffix(PENDING_REPLICA_SUFFIX); - if let Some(to_name) = to_name { - replica_ops.push(catalog::Op::RenameClusterReplica { - cluster_id: cluster_id.clone(), - replica_id: replica.replica_id.to_owned(), - name: QualifiedReplica { - cluster: cluster_ident, - replica: replica_ident, - }, - to_name: to_name.to_owned(), - }); - } - replica_ops.push(catalog::Op::UpdateClusterReplicaConfig { - cluster_id, - replica_id: replica.replica_id.to_owned(), - config: new_replica_config, - }); - replica_ops - }) - .flatten() - .collect(); - - ops.extend(finalize_replicas); - - // Add the Op to update the cluster state. A stale in-progress - // reconfiguration record carried by this legacy write is retained as - // cancelled, with the matching audit intent declared. - let mut final_config = ClusterConfig { - variant: ClusterVariant::Managed(new_config), - workload_class: workload_class.clone(), - }; - let reconfiguration_audit = cancel_carried_reconfiguration(&mut final_config); - ops.push(Op::UpdateClusterConfig { - id: cluster_id, - name: cluster_name, - config: final_config, - reconfiguration_audit, - burst_audit: None, - }); - self.catalog_transact(Some(session), ops).await?; - // Remove the cluster being altered from the ConnMeta - // pending_cluster_alters BTreeSet - self.active_conns - .get_mut(session.conn_id()) - .expect("There must be an active connection") - .pending_cluster_alters - .remove(&cluster_id); - - Ok(StageResult::Response(ExecuteResponse::AlteredObject( - ObjectType::Cluster, - ))) - } - - async fn check_if_pending_replicas_hydrated_stage( - &mut self, - session: &Session, - plan: AlterClusterPlan, - new_config: ClusterVariantManaged, - workload_class: Option, - timeout_time: Instant, - on_timeout: OnTimeoutAction, - validity: PlanValidity, - ) -> Result>, AdapterError> { - // wait and re-signal wait for hydrated if not hydrated - let cluster = self.catalog.get_cluster(plan.id); - let pending_replicas = cluster - .replicas() - .filter_map(|r| { - if r.config.location.pending() { - Some(r.replica_id.clone()) - } else { - None - } - }) - .collect_vec(); - // Check For timeout - if Instant::now() > timeout_time { - // Timed out handle timeout action - match on_timeout { - OnTimeoutAction::Rollback => { - self.active_conns - .get_mut(session.conn_id()) - .expect("There must be an active connection") - .pending_cluster_alters - .remove(&cluster.id); - self.drop_reconfiguration_replicas(btreeset!(cluster.id)) - .await?; - return Err(AdapterError::AlterClusterTimeout); - } - OnTimeoutAction::Commit => { - let span = Span::current(); - let poll_duration = self - .catalog - .system_config() - .cluster_alter_check_ready_interval() - .clone(); - return Ok(StageResult::Handle(mz_ore::task::spawn( - || "Finalize Alter Cluster", - async move { - tokio::time::sleep(poll_duration).await; - let stage = ClusterStage::Finalize(AlterClusterFinalize { - validity, - plan, - new_config, - workload_class, - }); - Ok(Box::new(stage)) - } - .instrument(span), - ))); - } - } - } - let compute_hydrated_fut = self - .controller - .compute - .collections_hydrated_for_replicas(cluster.id, pending_replicas.clone(), [].into()) - .map_err(|e| AdapterError::internal("Failed to check hydration", e))?; - - let storage_hydrated = self - .controller - .storage - .collections_hydrated_on_replicas( - Some(pending_replicas.clone()), - &cluster.id, - &[].into(), - ) - .map_err(|e| AdapterError::internal("Failed to check hydration", e))?; - - // Also require every pending replica to be online, in case it has no - // objects that need hydration on it (e.g. a single-replica source). - let replicas_online = pending_replicas.iter().all(|replica_id| { - let status = self - .cluster_replica_statuses - .try_get_cluster_replica_statuses(cluster.id, *replica_id) - .map(ClusterReplicaStatuses::cluster_replica_status); - matches!(status, Some(ClusterStatus::Online)) - }); - - let span = Span::current(); - Ok(StageResult::Handle(mz_ore::task::spawn( - || "Alter Cluster: wait for hydrated", - async move { - let compute_hydrated = compute_hydrated_fut - .await - .map_err(|e| AdapterError::internal("Failed to check hydration", e))?; - - if compute_hydrated && storage_hydrated && replicas_online { - // We're done - Ok(Box::new(ClusterStage::Finalize(AlterClusterFinalize { - validity, - plan, - new_config: new_config.clone(), - workload_class: workload_class.clone(), - }))) - } else { - // Check later - tokio::time::sleep(Duration::from_secs(1)).await; - let stage = ClusterStage::WaitForHydrated(AlterClusterWaitForHydrated { - validity, - plan, - new_config, - workload_class, - timeout_time, - on_timeout, - }); - Ok(Box::new(stage)) - } - } - .instrument(span), - ))) - } - #[mz_ore::instrument(level = "debug")] pub(crate) async fn sequence_create_cluster( &mut self, @@ -1305,6 +933,22 @@ impl Coordinator { self.ensure_valid_azs(availability_zones.iter())?; + // The shape every replica below is created at, matching the cluster's + // own config (see `sequence_create_cluster`) so the controller + // reconciles the replicas as already conforming. + let replica_shape = ReplicaShape { + size: size.clone(), + availability_zones: AvailabilityZones(availability_zones.clone()), + logging: match compute.introspection { + Some(config) => ReplicaLogging { + log_logging: config.debugging, + interval: Some(config.interval), + }, + None => ReplicaLogging::default(), + }, + arrangement_compression: compute.arrangement_compression, + }; + let role_id = session.role_metadata().current_role; self.catalog.ensure_valid_replica_size( &self @@ -1371,15 +1015,8 @@ impl Coordinator { cluster_id, replica_id, replica_name.clone(), - &compute, - &size, + &replica_shape, &mut ops, - if availability_zones.is_empty() { - None - } else { - Some(availability_zones.as_ref()) - }, - false, *session.current_role_id(), ReplicaCreateDropReason::Manual, )?; @@ -1418,16 +1055,21 @@ impl Coordinator { Ok(ExecuteResponse::CreatedCluster) } + /// Pushes an [`Op::CreateClusterReplica`] for a managed replica of `shape`, + /// returning its size family for the caller's scoped eval context. + /// + /// Takes the [`ReplicaShape`] the controller reconciles against rather than + /// a planned `ComputeReplicaConfig`, so the replica the op creates and the + /// cluster config that called for it cannot disagree. The planned form + /// cannot represent `INTROSPECTION DEBUGGING` without an interval, which + /// `ALTER CLUSTER` can durably write. fn create_managed_cluster_replica_op( &self, cluster_id: ClusterId, replica_id: ReplicaId, name: String, - compute: &mz_sql::plan::ComputeReplicaConfig, - size: &String, + shape: &ReplicaShape, ops: &mut Vec, - azs: Option<&[String]>, - pending: bool, owner_id: RoleId, reason: ReplicaCreateDropReason, ) -> Result { @@ -1437,19 +1079,16 @@ impl Coordinator { availability_zones: Vec::new(), billed_as: None, internal: false, - size: size.clone(), - pending, + size: shape.size.clone(), + pending: false, }; - let logging = if let Some(config) = compute.introspection { - ReplicaLogging { - log_logging: config.debugging, - interval: Some(config.interval), - } + // An empty pool is "no restriction", not "restricted to nothing". + let azs: Option<&[String]> = if shape.availability_zones.0.is_empty() { + None } else { - ReplicaLogging::default() + Some(&shape.availability_zones.0) }; - let config = ReplicaConfig { location: self.catalog().concretize_replica_location( location, @@ -1460,8 +1099,8 @@ impl Coordinator { false, )?, compute: ComputeReplicaConfig { - logging, - arrangement_compression: compute.arrangement_compression, + logging: shape.logging.clone(), + arrangement_compression: shape.arrangement_compression, }, }; @@ -1855,9 +1494,28 @@ impl Coordinator { } } - /// When this is called by the automated cluster scheduling, `scheduling_decision_reason` should - /// contain information on why is a cluster being turned On/Off. It will be forwarded to the - /// `details` field of the audit log event that records creating or dropping replicas. + /// Applies a managed→managed `ALTER CLUSTER`. + /// + /// By default this is a config-only write: the cluster controller owns the + /// replica set and reconciles it to the new realized config on its next + /// tick. Emitting creates and drops here as well would fight it, since it + /// derives replica names from the observed set, so an adapter create by + /// canonical `rN` can collide with a controller-chosen name and an adapter + /// drop by canonical `rN` can miss a churned one. + /// + /// `cut_over` is the direct path an explicitly zero-timeout commit `WAIT` + /// requests (see [`requests_immediate_cut_over`]): the observed owned + /// replica set is converged onto the target shape and factor (replicas + /// that already match are kept, up to the factor), and any carried + /// reconfiguration record is settled to a terminal status (see + /// [`retire_carried_reconfiguration`]), all in this one catalog + /// transaction with no controller involvement. It + /// is the one reshape that still works when the controller itself is the + /// problem, and it simultaneously unsticks a reconfiguration nothing else + /// would retire. Requesting it under a live controller stays safe: the + /// config write invalidates any in-flight tick's compare-and-append + /// witness, so a stale controller batch is rejected like it would be for + /// any user DDL landing mid-tick. /// /// # Panics /// @@ -1865,18 +1523,16 @@ impl Coordinator { /// Panics if `new_config` is not a configuration for a managed cluster. pub(crate) async fn sequence_alter_cluster_managed_to_managed( &mut self, - session: Option<&Session>, + session: &Session, cluster_id: ClusterId, new_config: ClusterConfig, - reason: ReplicaCreateDropReason, - strategy: AlterClusterPlanStrategy, - ) -> Result { + cut_over: bool, + ) -> Result<(), AdapterError> { let cluster = self.catalog.get_cluster(cluster_id); let name = cluster.name().to_string(); let owner_id = cluster.owner_id(); let mut ops = vec![]; - let mut finalization_needed = NeedsFinalization::No; let ClusterVariant::Managed(ClusterVariantManaged { size, @@ -1917,7 +1573,7 @@ impl Coordinator { burst: _, } = new_managed; - let role_id = session.map(|s| s.role_metadata().current_role); + let role_id = Some(session.role_metadata().current_role); self.catalog.ensure_valid_replica_size( &self.catalog().get_role_allowed_cluster_sizes(&role_id), new_size, @@ -1956,41 +1612,6 @@ impl Coordinator { } } - // check for active updates - if cluster.replicas().any(|r| r.config.location.pending()) { - return Err(AlterClusterWhilePendingReplicas); - } - - // Resolve existing replica ids by name before releasing the catalog - // borrow, so the drop branches below can build their ops without it. - let replica_id_by_name: BTreeMap = cluster - .replicas() - .map(|r| (r.name.clone(), r.replica_id)) - .collect(); - // The cluster's observed owned replica set, with the same exclusions - // as the controller's ownership test: internal and billed-as replicas - // are manually managed, pending ones belong to an in-flight - // reconfiguration (rejected above, so none exist here). - let owned_replica_ids: Vec = cluster - .replicas() - .filter(|r| { - !r.config.location.internal() - && r.config.location.billed_as().is_none() - && !r.config.location.pending() - }) - .map(|r| r.replica_id) - .collect(); - - let compute = mz_sql::plan::ComputeReplicaConfig { - introspection: new_logging - .interval - .map(|interval| ComputeReplicaIntrospectionConfig { - debugging: new_logging.log_logging, - interval, - }), - arrangement_compression: *new_arrangement_compression, - }; - // Eagerly validate the `max_replicas_per_cluster` limit. // `catalog_transact` will do this validation too, but allocating // replica IDs is expensive enough that we need to do this validation @@ -2007,28 +1628,6 @@ impl Coordinator { } } - // When the controller owns the managed replica set (master gate on, user - // cluster), a non-record change reaching this path is replication-factor - // only. Config-shape changes (size/logging/AZ) are reshaped into a durable - // reconfiguration record before they get here. The controller reconciles - // the replica set to the realized config's new count on its next tick, so - // we update only the realized config and emit no create/drop here. Doing - // both fights the controller. It derives replica names from the observed - // set, so an adapter create by canonical `rN` can collide with a - // controller-chosen name, and an adapter drop by canonical `rN` can miss a - // churned one. With the gate off (or a system cluster, which the - // controller never owns) the legacy path below still does the create/drop - // directly. - let controller_owns = ENABLE_CLUSTER_CONTROLLER - .get(self.catalog().system_config().dyncfgs()) - && cluster_id.is_user(); - - // Count exactly as many replica ids as the branches below consume. The - // config-changed branches recreate all replicas. A pure scale-up creates - // only the delta. Scale-down and no-op create none. A controller-owned - // alter emits no create/drop at all, so it must not allocate. Allocating - // there burns those ids durably and throws them away. The controller - // allocates its own when it materializes the change. let config_changed = new_managed.replica_config_shape() != ManagedReplicaConfigShape::new( &size, @@ -2036,41 +1635,67 @@ impl Coordinator { &logging, arrangement_compression, ); - let needed_replica_ids = if controller_owns { - 0 - } else if config_changed { - *new_replication_factor - } else if *new_replication_factor > replication_factor { - *new_replication_factor - replication_factor + // The controller creates replicas from the realized config without + // re-validating availability zones, so an invalid pool written here + // would produce an unplaceable replica. + if config_changed { + self.ensure_valid_azs(new_availability_zones.iter())?; + } + + // Decide the cut-over's creates and drops with the controller's own + // reconcile kernel, against the cluster as it is observed right now. + // Running it here rather than a tick later is the whole point of the + // cut-over, but *what* it converges on must not differ, or the same + // reshape would churn the replica set differently depending on which + // path ran it. In particular a replica that already has the target shape + // is kept rather than bounced. + let decisions = if cut_over { + let target_shape = ReplicaShape { + size: new_size.clone(), + availability_zones: AvailabilityZones(new_availability_zones.clone()), + logging: new_logging.clone(), + arrangement_compression: *new_arrangement_compression, + }; + let desired = vec![ + DesiredReplica { + shape: target_shape, + reason: CreateReason::Baseline, + }; + usize::cast_from(*new_replication_factor) + ]; + let state = self + .observe_cluster_state(cluster_id) + .expect("managed cluster observed above"); + reconcile_replicas(&state, &[desired]) } else { - 0 + Vec::new() }; - // Allocate the replica ids out-of-band via the durable allocator, only - // after the eager limit validation above so a rejected alter allocates - // nothing. Pick the id type from the target cluster, which may be a - // system cluster. This mirrors how cluster and item ids are allocated, - // so nothing allocates a replica id in-apply. Fetch the catalog write - // timestamp lazily here, since it needs mutable access to self (the - // cluster borrow above is already released) and scale-down, no-op, and - // automated scheduling turn-off alters must not pay an oracle - // round-trip just to allocate nothing. - let mut new_replica_ids = if needed_replica_ids > 0 { + + // One id per create, allocated out-of-band before the transaction like + // every other create path. A config-only alter creates nothing, so it + // allocates nothing: allocating would burn the ids durably and throw + // them away, and it would pay an oracle round-trip for no reason. + let creates = decisions + .iter() + .filter(|d| matches!(d, Decision::CreateReplica { .. })) + .count(); + let mut new_replica_ids = if creates > 0 { let id_ts = self.get_catalog_write_ts().await; let ids = self .catalog() - .allocate_replica_ids(cluster_id, u64::from(needed_replica_ids), id_ts) + .allocate_replica_ids(cluster_id, u64::cast_from(creates), id_ts) .await?; ids.into_iter() } else { Vec::::new().into_iter() }; - // Collect an eval context for each replica recreated below, so the alter - // transaction folds the replicas' replica-scoped overrides the same way - // the create paths do. ALTER CLUSTER SET (SIZE ...) to a different size - // family flips size-family-keyed render-frozen flags, so the override - // must reach the controller before the recreated replica renders. Only - // the replica scope is folded. The cluster already exists and its + // Collect an eval context for each replica the cut-over creates, so the + // alter transaction folds the replicas' replica-scoped overrides the same + // way the create paths do. ALTER CLUSTER SET (SIZE ...) to a different + // size family flips size-family-keyed render-frozen flags, so the + // override must reach the controller before the created replica renders. + // Only the replica scope is folded. The cluster already exists and its // cluster-scoped overrides are unaffected by this alter. let cluster_ctx = ClusterScopeContext { id: cluster_id.to_string(), @@ -2078,212 +1703,93 @@ impl Coordinator { is_builtin: cluster_id.is_system(), }; let mut replica_ctxs = Vec::new(); + let mut drops = Vec::new(); - if controller_owns { - // Defer all replica create/drop to the controller. Only the realized - // config update below is applied here. The target must still be - // valid: the controller creates replicas from the realized config - // without re-validating availability zones, so an invalid pool - // written here would produce an unplaceable replica. - if config_changed { - self.ensure_valid_azs(new_availability_zones.iter())?; - } - } else if config_changed { - self.ensure_valid_azs(new_availability_zones.iter())?; - // If we're not doing a zero-downtime reconfig tear down all - // replicas, create new ones else create the pending replicas and - // return early asking for finalization - match strategy { - AlterClusterPlanStrategy::None => { - // Names can drift from the canonical `r1..rN` while the - // controller owns the set (its name generator avoids - // observed names), so a factor-derived name list can miss - // replicas after a break-glass handoff. This branch - // recreates the entire replica set anyway, so dropping the - // observed owned set by id closes that. In the pure - // canonical world the two sets are identical. - let replica_ids_and_reasons = owned_replica_ids - .iter() - .map(|replica_id| { - catalog::DropObjectInfo::ClusterReplica(( - cluster_id, - *replica_id, - reason.clone(), - )) - }) - .collect(); - ops.push(catalog::Op::DropObjects(replica_ids_and_reasons)); - for replica_name in - (0..*new_replication_factor).map(managed_cluster_replica_name) - { - // The replica id is pre-allocated above like the create - // paths so its scoped overrides can be folded below. - let replica_id = new_replica_ids - .next() - .expect("pre-allocated enough replica ids"); - let size_family = self.create_managed_cluster_replica_op( - cluster_id, - replica_id, - replica_name.clone(), - &compute, - new_size, - &mut ops, - Some(new_availability_zones.as_ref()), - false, - owner_id, - reason.clone(), - )?; - replica_ctxs.push(ReplicaEvalContext { - cluster_id, - replica_id, - cluster: cluster_ctx.clone(), - replica: ReplicaScopeContext { - id: replica_id.to_string(), - name: replica_name, - is_builtin: cluster_id.is_system(), - size: new_size.clone(), - size_family, - cluster_id: cluster_id.to_string(), - cluster_name: cluster_ctx.name.clone(), - }, - }); - } - } - AlterClusterPlanStrategy::For(_) | AlterClusterPlanStrategy::UntilReady { .. } => { - for replica_name in - (0..*new_replication_factor).map(managed_cluster_replica_name) - { - let replica_name = format!("{replica_name}{PENDING_REPLICA_SUFFIX}"); - let replica_id = new_replica_ids - .next() - .expect("pre-allocated enough replica ids"); - let size_family = self.create_managed_cluster_replica_op( - cluster_id, - replica_id, - replica_name.clone(), - &compute, - new_size, - &mut ops, - Some(new_availability_zones.as_ref()), - true, - owner_id, - reason.clone(), - )?; - replica_ctxs.push(ReplicaEvalContext { - cluster_id, - replica_id, - cluster: cluster_ctx.clone(), - replica: ReplicaScopeContext { - id: replica_id.to_string(), - name: replica_name, - is_builtin: cluster_id.is_system(), - size: new_size.clone(), - size_family, - cluster_id: cluster_id.to_string(), - cluster_name: cluster_ctx.name.clone(), - }, - }); - } - finalization_needed = NeedsFinalization::Yes; + for decision in decisions { + match decision { + Decision::CreateReplica { + name: replica_name, + shape, + .. + } => { + let replica_id = new_replica_ids + .next() + .expect("one pre-allocated id per create"); + let size_family = self.create_managed_cluster_replica_op( + cluster_id, + replica_id, + replica_name.clone(), + &shape, + &mut ops, + owner_id, + ReplicaCreateDropReason::Manual, + )?; + replica_ctxs.push(ReplicaEvalContext { + cluster_id, + replica_id, + cluster: cluster_ctx.clone(), + replica: ReplicaScopeContext { + id: replica_id.to_string(), + name: replica_name, + is_builtin: cluster_id.is_system(), + size: shape.size.clone(), + size_family, + cluster_id: cluster_id.to_string(), + cluster_name: cluster_ctx.name.clone(), + }, + }); } - } - } else if *new_replication_factor < replication_factor { - // Adjust replica count down - let replica_ids = (*new_replication_factor..replication_factor) - .map(managed_cluster_replica_name) - .filter_map(|name| replica_id_by_name.get(&name).copied()) - .map(|replica_id| { - catalog::DropObjectInfo::ClusterReplica(( + Decision::DropReplica { replica_id, .. } => { + drops.push(catalog::DropObjectInfo::ClusterReplica(( cluster_id, replica_id, - reason.clone(), - )) - }) - .collect(); - ops.push(catalog::Op::DropObjects(replica_ids)); - } else if *new_replication_factor > replication_factor { - // Adjust replica count up - for replica_name in - (replication_factor..*new_replication_factor).map(managed_cluster_replica_name) - { - let replica_id = new_replica_ids - .next() - .expect("pre-allocated enough replica ids"); - let size_family = self.create_managed_cluster_replica_op( - cluster_id, - replica_id, - replica_name.clone(), - &compute, - new_size, - &mut ops, - // AVAILABILITY ZONES hasn't changed, so existing replicas don't need to be - // rescheduled. - Some(new_availability_zones.as_ref()), - false, - owner_id, - reason.clone(), - )?; - replica_ctxs.push(ReplicaEvalContext { - cluster_id, - replica_id, - cluster: cluster_ctx.clone(), - replica: ReplicaScopeContext { - id: replica_id.to_string(), - name: replica_name, - is_builtin: cluster_id.is_system(), - size: new_size.clone(), - size_family, - cluster_id: cluster_id.to_string(), - cluster_name: cluster_ctx.name.clone(), - }, - }); + ReplicaCreateDropReason::Manual, + ))); + } + // The kernel's replica diff emits creates and drops only. The + // durable state write is this function's own business. + Decision::UpdateClusterState { .. } => { + return Err(AdapterError::Internal( + "the replica reconcile kernel does not write cluster state".into(), + )); + } } } - - // If finalization is needed, finalization should update the cluster - // config. Otherwise the config write happens here. With the controller - // owning the cluster, a record still in progress belongs to a live, - // converging reconfiguration this write didn't touch: carry it through - // untouched. Without (gate off, or a system cluster), such a record is - // orphaned, so retain it as cancelled with the matching audit intent - // rather than risk a bogus revival if the gate comes back on. - // - // NOTE: `handle_scheduling_decisions` also calls this function and - // bypasses the sequencer's guards. It runs only while the controller - // gate is off, where the cancel-carried write below retires any - // in-progress record instead of leaving it behind for a controller - // that is not running. - match finalization_needed { - NeedsFinalization::No => { - let mut new_config = new_config; - let reconfiguration_audit = if controller_owns { - None - } else { - cancel_carried_reconfiguration(&mut new_config) - }; - ops.push(catalog::Op::UpdateClusterConfig { - id: cluster_id, - name: name.clone(), - config: new_config, - reconfiguration_audit, - burst_audit: None, - }); - } - NeedsFinalization::Yes => {} + if !drops.is_empty() { + ops.push(catalog::Op::DropObjects(drops)); } + // A record still in progress belongs to a live, converging + // reconfiguration a config-only write did not touch, so carry it through + // untouched. The cut-over, in contrast, has just transacted the reshape + // itself, so it retires the record: leaving it in progress would have + // the controller keep converging on a target this write superseded. + let mut new_config = new_config; + let reconfiguration_audit = if cut_over { + retire_carried_reconfiguration(&mut new_config) + } else { + None + }; + ops.push(catalog::Op::UpdateClusterConfig { + id: cluster_id, + name: name.clone(), + config: new_config, + reconfiguration_audit, + burst_audit: None, + }); + // Fold the recreated replicas' replica-scoped overrides into the same // transaction, so the committed diff drives the replica-scoped controller // push before create_replica. Render-frozen flags (chosen at // arrangement-build time) require the override to land before the replica - // renders. Scale-down and no-op alters recreate no replicas, so this is - // empty and folds nothing. + // renders. A config-only alter recreates no replicas, so this is empty + // and folds nothing. if let Some(scoped_op) = self.scoped_overrides_create_op(&[], &replica_ctxs) { ops.push(scoped_op); } - self.catalog_transact(session, ops).await?; - Ok(finalization_needed) + self.catalog_transact(Some(session), ops).await?; + Ok(()) } /// # Panics @@ -2608,34 +2114,67 @@ struct ReconfigurationDimensionsUnchanged { arrangement_compression: bool, } -/// Retains a stale in-progress reconfiguration record carried by a legacy-path -/// config write as cancelled, returning the audit intent to declare with the +/// Drives an in-progress reconfiguration record carried by a synchronous +/// cut-over to a terminal status, returning the audit intent to declare with the /// write. /// -/// The legacy write paths (controller gate off), the ALTER sequencer and the -/// legacy scheduler, change the realized config directly and know nothing -/// about reconfiguration records. Nothing on those -/// paths ever settles a record, and carrying an in-progress one forward invites -/// a bogus revival, up to a forced cut-over to an obsolete target, if the gate -/// is turned back on later. A record can only be in progress here if it was -/// written while the gate was on. -pub(crate) fn cancel_carried_reconfiguration( - config: &mut ClusterConfig, -) -> Option { +/// The cut-over reshapes the replica set itself, so it settles the record rather +/// than leaving the controller converging on a target this write superseded. +/// Which terminal status is the honest one depends on where the cut-over landed: +/// on the record's own target it *is* the finalization the record was waiting +/// for, and it is `forced` because it did not wait for hydration. Anywhere else +/// the record's target was abandoned, which is a cancel. +fn retire_carried_reconfiguration(config: &mut ClusterConfig) -> Option { let ClusterVariant::Managed(managed) = &mut config.variant else { return None; }; + // `matches_realized_config` compares against the config being written, which + // for a cut-over is the shape it just materialized. + let reached_target = managed + .reconfiguration + .as_ref() + .is_some_and(|record| record.target.matches_realized_config(managed)); let record = managed.reconfiguration.as_mut()?; if !record.is_in_progress() { return None; } - record.status = ReconfigurationStatus::Cancelled; - Some(ReconfigurationAudit::Cancelled) + if reached_target { + record.status = ReconfigurationStatus::Finalized; + Some(ReconfigurationAudit::Finalized { forced: true }) + } else { + record.status = ReconfigurationStatus::Cancelled; + Some(ReconfigurationAudit::Cancelled) + } +} + +/// Whether a `WITH (WAIT ...)` clause asks for the cut-over to happen now and to +/// be committed rather than rolled back: `WAIT FOR '0s'` (which is sugar for +/// `ON TIMEOUT COMMIT`), or an explicit +/// `WAIT UNTIL READY (TIMEOUT '0s', ON TIMEOUT 'COMMIT')`. +/// +/// Such a statement has already said "cut over now, hydrated or not", so the +/// reshape takes the direct path +/// ([`Coordinator::sequence_alter_cluster_managed_to_managed`]'s `cut_over`) +/// instead of writing a record whose deadline has already passed. Same outcome, +/// one controller tick sooner, and it works even when the controller does not. +/// +/// A zero timeout that rolls back is *not* this: it asks for the reconfiguration +/// to be abandoned at once, which is the record path's job. +fn requests_immediate_cut_over(strategy: &AlterClusterPlanStrategy) -> bool { + match strategy { + AlterClusterPlanStrategy::None => false, + AlterClusterPlanStrategy::For(timeout) => timeout.is_zero(), + AlterClusterPlanStrategy::UntilReady { + timeout, + on_timeout, + } => timeout.is_zero() && matches!(on_timeout, Some(OnTimeoutAction::Commit)), + } } /// Whether an `ALTER` statement sets a replica config shape dimension (`SIZE`, -/// `AVAILABILITY ZONES`, or either `INTROSPECTION` option), the changes that -/// need a durable `reconfiguration` record and a hydrate-overlap. +/// `AVAILABILITY ZONES`, either `INTROSPECTION` option, or `EXPERIMENTAL +/// ARRANGEMENT COMPRESSION`), the changes that need a durable +/// `reconfiguration` record and a hydrate-overlap. /// /// A statement-level check, used while a reconfiguration is in flight: an /// `ALTER` back to the realized shape sets a shape option without changing its @@ -2664,6 +2203,81 @@ fn alter_changes_replica_shape(options: &PlanClusterOption) -> bool { || !matches!(arrangement_compression, Unchanged) } +/// The reconfiguration target an `ALTER` establishes: the shape it asks for, +/// folded onto the in-flight target when a reconfiguration is in progress. +/// +/// `new_managed` was built against the *realized* config, which still holds the +/// pre-reconfiguration shape (the realized config advances only at cut-over), so +/// a dimension the statement left `Unchanged` carries the realized value there. +/// Folding replaces those with the in-flight target's values, leaving only the +/// dimensions the statement explicitly set to diverge. Without it, an `ALTER` +/// that mentions one dimension would silently revert the transition along every +/// dimension it did not mention. +/// +/// Both paths that act on a shape-changing `ALTER` share this: the reshape that +/// writes the record, and the synchronous cut-over that transacts the reshape +/// itself. They must agree on what "the target" means for a given statement. +fn alter_reconfiguration_target( + new_managed: &ClusterVariantManaged, + options: &PlanClusterOption, + in_flight: Option<&ReconfigurationTarget>, +) -> ReconfigurationTarget { + use mz_sql::plan::AlterOptionParameter::Unchanged; + + // Both structs are destructured exhaustively: a new shape dimension on the + // cluster config, or a new `ALTER` option that names one, then fails to + // compile until it is either folded or explicitly ruled out here. + let ClusterVariantManaged { + size, + replication_factor, + availability_zones, + logging, + arrangement_compression, + // Cluster-level settings, not per-replica shape. A reconfiguration does + // not transition them, so they never enter a target. + optimizer_feature_overrides: _, + schedule: _, + auto_scaling_strategy: _, + reconfiguration: _, + burst: _, + } = new_managed; + let PlanClusterOption { + size: size_opt, + replication_factor: replication_factor_opt, + availability_zones: availability_zones_opt, + arrangement_compression: arrangement_compression_opt, + introspection_debugging, + introspection_interval, + // Not shape dimensions: `managed`/`replicas` change the variant rather + // than reshape it, and the rest are cluster-level settings that apply + // immediately instead of transitioning through a reconfiguration. + managed: _, + replicas: _, + schedule: _, + workload_class: _, + auto_scaling_strategy: _, + } = options; + + let new_target = ReconfigurationTarget { + size: size.clone(), + replication_factor: *replication_factor, + availability_zones: availability_zones.clone(), + logging: logging.clone(), + arrangement_compression: *arrangement_compression, + }; + let unchanged = ReconfigurationDimensionsUnchanged { + size: matches!(size_opt, Unchanged), + replication_factor: matches!(replication_factor_opt, Unchanged), + availability_zones: matches!(availability_zones_opt, Unchanged), + // The two logging options fold independently, so a debugging-only + // `ALTER` cannot revert an in-flight interval change (or vice versa). + log_logging: matches!(introspection_debugging, Unchanged), + interval: matches!(introspection_interval, Unchanged), + arrangement_compression: matches!(arrangement_compression_opt, Unchanged), + }; + fold_reconfiguration_target(in_flight, new_target, unchanged) +} + /// Fold a new `ALTER` onto an in-flight reconfiguration target. /// /// `new_target` was built against the *realized* config, so any dimension the @@ -2725,15 +2339,6 @@ fn fold_reconfiguration_target( } } -/// The type of finalization needed after an -/// operation such as alter_cluster_managed_to_managed. -#[derive(PartialEq)] -pub(crate) enum NeedsFinalization { - /// Wait for the provided duration before finalizing - Yes, - No, -} - #[cfg(test)] mod tests { use mz_controller::clusters::ReplicaLogging; diff --git a/src/adapter/src/coord/sql.rs b/src/adapter/src/coord/sql.rs index 3be074ff30507..84ad3de62c743 100644 --- a/src/adapter/src/coord/sql.rs +++ b/src/adapter/src/coord/sql.rs @@ -233,7 +233,6 @@ impl Coordinator { let retire_notify = self .retire_compute_sinks_for_conn(conn_id, ActiveComputeSinkRetireReason::Finished) .await; - self.retire_cluster_reconfigurations_for_conn(conn_id).await; // Release this transaction's compaction hold on collections. if let Some(txn_reads) = self.txn_read_holds.remove(conn_id) { diff --git a/src/adapter/src/error.rs b/src/adapter/src/error.rs index ac348a8dd26d3..27e8b5f77944f 100644 --- a/src/adapter/src/error.rs +++ b/src/adapter/src/error.rs @@ -276,7 +276,6 @@ pub enum AdapterError { /// read-only mode. ReadOnly, AlterClusterTimeout, - AlterClusterWhilePendingReplicas, /// Attempt to convert a cluster to unmanaged while a graceful /// reconfiguration is in progress. AlterClusterUnmanagedWhileReconfiguring, @@ -997,7 +996,6 @@ impl AdapterError { // transactions. AdapterError::ReadOnly => SqlState::READ_ONLY_SQL_TRANSACTION, AdapterError::AlterClusterTimeout => SqlState::QUERY_CANCELED, - AdapterError::AlterClusterWhilePendingReplicas => SqlState::OBJECT_IN_USE, AdapterError::AlterClusterUnmanagedWhileReconfiguring => SqlState::OBJECT_IN_USE, AdapterError::AlterClusterUnmanagedWhileBursting => SqlState::OBJECT_IN_USE, AdapterError::AlterClusterReplicationFactorWhileReconfiguring => { @@ -1447,9 +1445,6 @@ impl fmt::Display for AdapterError { } Ok(()) } - AdapterError::AlterClusterWhilePendingReplicas => { - write!(f, "cannot alter clusters with pending updates") - } AdapterError::AlterClusterUnmanagedWhileReconfiguring => { write!( f, diff --git a/src/adapter/src/metrics.rs b/src/adapter/src/metrics.rs index b5168a4916b98..d6c31194d6e4c 100644 --- a/src/adapter/src/metrics.rs +++ b/src/adapter/src/metrics.rs @@ -43,8 +43,6 @@ pub struct Metrics { pub append_table_duration_seconds: Histogram, pub webhook_validation_reduce_failures: IntCounterVec, pub webhook_get_appender: IntCounter, - pub check_scheduling_policies_seconds: HistogramVec, - pub handle_scheduling_decisions_seconds: HistogramVec, pub row_set_finishing_seconds: Histogram, pub session_startup_table_writes_seconds: Histogram, pub parse_seconds: Histogram, @@ -198,18 +196,6 @@ impl Metrics { name: "mz_webhook_get_appender_count", help: "Count of getting a webhook appender from the Coordinator.", )), - check_scheduling_policies_seconds: registry.register(metric!( - name: "mz_check_scheduling_policies_seconds", - help: "The time each policy in `check_scheduling_policies` takes.", - var_labels: ["policy", "thread"], - buckets: histogram_seconds_buckets(0.000_128, 8.0), - )), - handle_scheduling_decisions_seconds: registry.register(metric!( - name: "mz_handle_scheduling_decisions_seconds", - help: "The time `handle_scheduling_decisions` takes.", - var_labels: ["altered_a_cluster"], - buckets: histogram_seconds_buckets(0.000_128, 8.0), - )), row_set_finishing_seconds: registry.register(metric!( name: "mz_row_set_finishing_seconds", help: "The time it takes to run RowSetFinishing::finish.", diff --git a/src/catalog/src/builtin/mz_internal.rs b/src/catalog/src/builtin/mz_internal.rs index c84850ec10ed5..f8adc09251b81 100644 --- a/src/catalog/src/builtin/mz_internal.rs +++ b/src/catalog/src/builtin/mz_internal.rs @@ -876,7 +876,7 @@ pub static MZ_CLUSTER_RECONFIGURATIONS: LazyLock = Lazy ), ( "target", - "The config shape the cluster is reconfiguring to, as JSON: `size`, `replication_factor`, `availability_zones`, and `logging`. The realized (current) shape is in `mz_clusters`.", + "The config shape the cluster is reconfiguring to, as JSON: `size`, `replication_factor`, `availability_zones`, `logging`, and `arrangement_compression`. The realized (current) shape is in `mz_clusters`.", ), ( "changes", @@ -953,7 +953,9 @@ SELECT CASE WHEN r.target->'availability_zones' != r.config->'availability_zones' THEN jsonb_build_object('availability_zones', r.target->'availability_zones') ELSE '{}'::jsonb END || CASE WHEN r.target->'logging' != r.config->'logging' - THEN jsonb_build_object('logging', r.target->'logging') ELSE '{}'::jsonb END + THEN jsonb_build_object('logging', r.target->'logging') ELSE '{}'::jsonb END || + CASE WHEN r.target->'arrangement_compression' != r.config->'arrangement_compression' + THEN jsonb_build_object('arrangement_compression', r.target->'arrangement_compression') ELSE '{}'::jsonb END AS changes FROM records r", is_retained_metrics_object: false, @@ -5651,7 +5653,8 @@ pub static MZ_SHOW_CLUSTERS: LazyLock = LazyLock::new(|| { 'size to ' || (changes->>'size'), 'replication factor to ' || (changes->>'replication_factor'), CASE WHEN changes->'availability_zones' IS NOT NULL THEN 'availability zones' END, - CASE WHEN changes->'logging' IS NOT NULL THEN 'introspection settings' END + CASE WHEN changes->'logging' IS NOT NULL THEN 'introspection settings' END, + CASE WHEN changes->'arrangement_compression' IS NOT NULL THEN 'arrangement compression' END ], ', '), '') AS summary FROM mz_internal.mz_cluster_reconfigurations WHERE status = 'in-progress' diff --git a/src/catalog/src/durable/objects.rs b/src/catalog/src/durable/objects.rs index f7f981a43fccb..3e95f150162f2 100644 --- a/src/catalog/src/durable/objects.rs +++ b/src/catalog/src/durable/objects.rs @@ -369,12 +369,12 @@ pub struct ClusterVariantManaged { /// A managed cluster's replicas are derived from its `replication_factor`: for a /// factor of N they are named `r1` through `rN`. /// -/// `ALTER CLUSTER` computes the replicas it creates and drops by this rule, and -/// so does the catalog-open reconciler that materializes builtin replicas. The -/// two have to share it, or `ALTER` cannot find the replicas it means to change. -/// The cluster controller deliberately does not: its `ReplicaNameGen` picks names -/// that avoid the observed set, which is why `ALTER` against a controller-owned -/// cluster drops by observed id rather than by derived name. +/// The catalog-open reconciler that materializes builtin replicas converges on +/// this rule, and so does the one `ALTER CLUSTER` path that still creates +/// replicas itself (the synchronous cut-over). The cluster controller +/// deliberately does not: its `ReplicaNameGen` picks names that avoid the +/// observed set, which is why the cut-over drops by observed id rather than by +/// derived name. pub fn managed_cluster_replica_name(index: u32) -> String { format!("r{}", index + 1) } diff --git a/src/catalog/src/memory/objects.rs b/src/catalog/src/memory/objects.rs index c422cd79e0148..68f7cd38e635c 100644 --- a/src/catalog/src/memory/objects.rs +++ b/src/catalog/src/memory/objects.rs @@ -3440,6 +3440,26 @@ impl ClusterVariantManaged { } } + /// Advances this config's realized shape to `target`, the write a + /// reconfiguration cut-over performs. The inverse of + /// [`ClusterVariantManaged::realized_reconfiguration_target`]. + pub fn apply_reconfiguration_target(&mut self, target: ReconfigurationTarget) { + // Destructured so a new target dimension fails to compile until it is + // applied here too. + let ReconfigurationTarget { + size, + replication_factor, + availability_zones, + logging, + arrangement_compression, + } = target; + self.size = size; + self.replication_factor = replication_factor; + self.availability_zones = availability_zones; + self.logging = logging; + self.arrangement_compression = arrangement_compression; + } + /// Whether the in-flight `burst` record is no longer warranted by this /// config: the `ON HYDRATION` policy was removed or re-sized away from the /// record's size, or the cluster was turned off (`replication_factor` 0). diff --git a/src/cluster-controller/src/ctx.rs b/src/cluster-controller/src/ctx.rs index beca4fd3d9215..d41af1d99d30b 100644 --- a/src/cluster-controller/src/ctx.rs +++ b/src/cluster-controller/src/ctx.rs @@ -58,7 +58,8 @@ pub struct ObservedReplica { pub internal: bool, /// Carries a `BILLED AS` override. pub billed_as: bool, - /// The `-pending` target of an in-flight graceful reconfiguration. + /// Durably marked `pending`. Vestigial: no path creates one anymore, but a + /// crash on an older version could have left one behind. pub pending: bool, } @@ -67,11 +68,11 @@ impl ObservedReplica { /// /// INTERNAL / BILLED AS replicas are manually managed: a user can attach /// one to any managed cluster, outside the replication-factor domain. A - /// pending replica is owned by the reconfiguration sequencer path until - /// finalize (retiring it would defeat the zero-downtime resize creating - /// it). The controller must neither count such a replica toward a desired - /// shape nor drop it as excess, but their names still block the name - /// generator, since every replica observed here occupies a name. + /// durably `pending` replica is stranded state from an older version, reaped + /// by the catalog-open migration rather than here. The controller must + /// neither count such a replica toward a desired shape nor drop it as + /// excess, but their names still block the name generator, since every + /// replica observed here occupies a name. pub fn owned_shape(&self) -> Option<&ReplicaShape> { if self.internal || self.billed_as || self.pending { return None; @@ -88,9 +89,8 @@ impl ObservedReplica { /// compares it against the read timestamp (`less_than`) to decide whether the MV /// still needs a refresh. For the compaction window it reads the frontier's lone /// element via `as_option` to find the previous refresh time, falling back to the -/// schedule's last refresh on the empty/sealed frontier `[]`, mirroring the -/// legacy refresh policy. The frontier of a single-input total-order MV holds at -/// most one element. +/// schedule's last refresh on the empty/sealed frontier `[]`. The frontier of a +/// single-input total-order MV holds at most one element. #[derive(Clone, Debug, PartialEq, Eq)] pub struct RefreshMvInfo { /// The MV's writes-`GlobalId`: the identity the window decision records in @@ -200,9 +200,10 @@ pub struct RefreshWindowInputs { /// This is the input every strategy reads. Unmanaged clusters are not /// controller-owned and are not represented here. /// -/// The `size`, `replication_factor`, `availability_zones`, and `logging` fields -/// together are the realized config the cluster is currently serving. The -/// implicit baseline desires `replication_factor` replicas at that shape. +/// The `size`, `replication_factor`, `availability_zones`, `logging`, and +/// `arrangement_compression` fields together are the realized config the +/// cluster is currently serving. The implicit baseline desires +/// `replication_factor` replicas at that shape. #[derive(Clone, Debug)] pub struct ClusterState { pub cluster_id: ClusterId, diff --git a/src/cluster-controller/src/lib.rs b/src/cluster-controller/src/lib.rs index 7b86b58eade7a..42a30aa557bfa 100644 --- a/src/cluster-controller/src/lib.rs +++ b/src/cluster-controller/src/lib.rs @@ -416,6 +416,10 @@ fn join( /// desired replica slots and the actual replicas, match slots to replicas by /// shape and emit the creates and drops that close the gap. /// +/// Public because the environment also runs it outside a tick, for the +/// synchronous cut-over an `ALTER CLUSTER` can request. A reshape must converge +/// on the same replica set whichever path performs it. +/// /// Semantics: /// - The desired set is the multiset **union** of every strategy's slots: a /// given shape is desired `max` over strategies (not the sum), since a replica @@ -427,7 +431,7 @@ fn join( /// - Creates carry the winning [`CreateReason`] among the slots that /// desired the shape (see [`CreateReason::outranks`]). Drops carry no /// attribution. A drop happens exactly when no strategy desires the replica. -fn reconcile_replicas( +pub fn reconcile_replicas( state: &ClusterState, contributions: &[Vec], ) -> Vec { diff --git a/src/cluster-controller/src/strategy.rs b/src/cluster-controller/src/strategy.rs index 59cbd9e3744f5..c8584c713bef7 100644 --- a/src/cluster-controller/src/strategy.rs +++ b/src/cluster-controller/src/strategy.rs @@ -384,8 +384,8 @@ impl Strategy for GracefulReconfigurationStrategy { /// replica at the cluster's realized shape while the cluster is inside a refresh /// window, and nothing otherwise. The window decision keys on the bound REFRESH /// materialized views' write frontiers, their refresh schedules, the configured -/// hydration-time estimate, and the current read timestamp (the same signals the -/// legacy scheduler reads), all carried in [`RefreshWindowInputs`]. +/// hydration-time estimate, and the current read timestamp, all carried in +/// [`RefreshWindowInputs`]. /// /// The controller (not the user's `replication_factor`) owns a scheduled /// cluster's replica set, so [`Strategy::update_state`] normalizes the realized @@ -394,12 +394,10 @@ impl Strategy for GracefulReconfigurationStrategy { /// a scheduled cluster, with `mz_cluster_replicas` authoritative for what is /// actually running. /// -/// NB: the decision is re-derived purely from the live signals each tick; there -/// is no "all policies have decided" latch like `cluster_scheduling.rs` needs. -/// That scheduler collects policy decisions asynchronously and across ticks, so -/// turning a cluster off is only safe once every policy has reported. We pull a -/// complete decision from durable + storage state on every tick, so the first -/// tick after a restart already decides from the same inputs as a steady tick. +/// NB: the decision is re-derived purely from the live signals each tick, with +/// no cross-tick latch. We pull a complete decision from durable and storage +/// state on every tick, so the first tick after a restart already decides from +/// the same inputs as a steady tick. #[derive(Clone, Copy, Debug, Default)] pub struct OnRefreshStrategy; @@ -485,9 +483,10 @@ impl Strategy for OnRefreshStrategy { _now: Timestamp, ) -> StateWrite { // The controller owns a scheduled cluster's replica set, so hold the - // realized `replication_factor` at `0`. A stale non-zero value (e.g. left - // by the legacy scheduler toggling 0↔1) would otherwise have the implicit - // baseline desire a replica the on-refresh strategy does not, a flap. + // realized `replication_factor` at `0`. A stale non-zero value (e.g. + // carried over from a cluster that was just given a schedule) would + // otherwise have the implicit baseline desire a replica the on-refresh + // strategy does not, a flap. // Only write when it is actually non-zero, to keep steady ticks no-ops. if matches!(state.schedule, ClusterSchedule::Manual) || state.replication_factor == 0 { return StateWrite::default(); @@ -540,9 +539,8 @@ impl Strategy for OnRefreshStrategy { return Vec::new(); } // One replica at the realized shape (`cluster.size` plus the cluster's AZ - // pool and logging), matching what the legacy scheduler brings up. The - // window decision rides inside the reason so the create it may produce - // can carry the audit detail. + // pool and logging). The window decision rides inside the reason so the + // create it may produce can carry the audit detail. vec![DesiredReplica { shape: state.realized_shape(), reason: CreateReason::OnRefresh(decision), diff --git a/src/cluster-controller/src/tests.rs b/src/cluster-controller/src/tests.rs index 5c8db3dd1f7bd..19e51834eac15 100644 --- a/src/cluster-controller/src/tests.rs +++ b/src/cluster-controller/src/tests.rs @@ -2278,7 +2278,7 @@ fn on_refresh_window_decision_lists_due_mvs() { fn on_refresh_caught_up_at_read_ts_is_off() { // Frontier exactly at the read ts (and no hydration lead, no compaction // window): the MV is caught up, so the cluster is Off. The needs-refresh check - // is strict (`frontier < read_ts + estimate`), matching the legacy scheduler. + // is strict (`frontier < read_ts + estimate`). let c = cluster(1); let inputs = window_inputs(100, 0, Some(100), refresh_at(50)); let (state, signals) = scheduled_state(c, "100cc", 0, 0, Vec::new(), Some(inputs)); @@ -2294,8 +2294,7 @@ fn on_refresh_caught_up_at_read_ts_is_off() { fn on_refresh_empty_frontier_needs_no_refresh() { // An empty (sealed) write frontier `[]` is the "complete past every timestamp" // state: `Antichain::less_than` is `false` for every timestamp, so the MV never - // reads as needing a refresh on that count, exactly as the legacy refresh - // policy decides it with `Antichain::less_than`. The compaction window is also + // reads as needing a refresh on that count. The compaction window is also // closed here (read ts 1000 is well past the last `AT 200` plus the compaction // estimate), so the cluster is Off. // diff --git a/src/controller/src/clusters.rs b/src/controller/src/clusters.rs index 01ad1c3c78a1f..d7b97f230ffbe 100644 --- a/src/controller/src/clusters.rs +++ b/src/controller/src/clusters.rs @@ -312,10 +312,11 @@ impl ReplicaLocation { } } - /// A pending replica is created as part of an alter cluster of an managed - /// cluster. the configuration of a pending replica will not match that of - /// the clusters until the alter has been finalized promoting the pending - /// replicas and setting this value to false. + /// Whether the replica is durably marked `pending`. + /// + /// Vestigial: no path creates one anymore. A crash on a version that still + /// staged reconfigurations through overlap replicas could have left one + /// behind, and the catalog-open migration reaps those. pub fn pending(&self) -> bool { match self { ReplicaLocation::Managed(ManagedReplicaLocation { pending, .. }) => *pending, @@ -374,7 +375,7 @@ pub struct ManagedReplicaLocation { /// concretization, not read back from a durable record. #[serde(skip)] pub availability_zones: Vec, - /// Whether the replica is pending reconfiguration + /// See [`ReplicaLocation::pending`]. pub pending: bool, } diff --git a/src/environmentd/tests/bootstrap_builtin_clusters.rs b/src/environmentd/tests/bootstrap_builtin_clusters.rs index 27689048514d3..e0132003c7245 100644 --- a/src/environmentd/tests/bootstrap_builtin_clusters.rs +++ b/src/environmentd/tests/bootstrap_builtin_clusters.rs @@ -13,8 +13,17 @@ //! single source of truth for its replica set. These tests pin that down from //! both directions: the factor a deployment bootstraps with is realized, and a //! factor an operator sets with `ALTER CLUSTER` survives a restart. +//! +//! Two writers realize that factor, at different times. At catalog open the +//! builtin-replica migration converges the set synchronously, so a bootstrap or +//! post-restart assertion can be exact. At runtime the cluster controller owns +//! it and converges a tick after the `ALTER` commits, so an assertion right +//! after an `ALTER` has to poll. + +use std::time::Duration; use mz_environmentd::test_util::{self, TestHarness, TestServerWithRuntime}; +use mz_ore::retry::Retry; /// A builtin cluster's declared replication factor and how many replicas it /// actually has. @@ -35,6 +44,24 @@ fn declared_and_actual(server: &TestServerWithRuntime, cluster: &str) -> (i32, i (row.get(0), row.get(1)) } +/// Polls [`declared_and_actual`] until it reads `expected`. +/// +/// For assertions that follow a runtime `ALTER CLUSTER`, whose replica +/// create/drop the cluster controller applies on a later tick. +fn await_declared_and_actual(server: &TestServerWithRuntime, cluster: &str, expected: (i32, i32)) { + Retry::default() + .max_duration(Duration::from_secs(60)) + .retry(|_| { + let actual = declared_and_actual(server, cluster); + if actual == expected { + Ok(()) + } else { + Err(format!("{cluster}: got {actual:?}, want {expected:?}")) + } + }) + .unwrap(); +} + /// Runs `stmt` as `user` over the internal port, which is where the system roles /// that own the builtin clusters can connect. fn execute_as(server: &TestServerWithRuntime, user: &str, stmt: &str) { @@ -104,7 +131,7 @@ fn test_alter_to_zero_replicas_survives_restart() { "mz_system", "ALTER CLUSTER mz_system SET (REPLICATION FACTOR 0)", ); - assert_eq!(declared_and_actual(&server, "mz_system"), (0, 0)); + await_declared_and_actual(&server, "mz_system", (0, 0)); } let server = harness.start_blocking(); @@ -124,7 +151,7 @@ fn test_alter_above_one_replica_survives_restart() { "mz_system", "ALTER CLUSTER mz_system SET (REPLICATION FACTOR 2)", ); - assert_eq!(declared_and_actual(&server, "mz_system"), (2, 2)); + await_declared_and_actual(&server, "mz_system", (2, 2)); } let server = harness.start_blocking(); @@ -182,7 +209,7 @@ fn test_support_cluster_replica_survives_restart() { "mz_support", "ALTER CLUSTER mz_support SET (REPLICATION FACTOR 1)", ); - assert_eq!(declared_and_actual(&server, "mz_support"), (1, 1)); + await_declared_and_actual(&server, "mz_support", (1, 1)); } let server = harness.start_blocking(); diff --git a/src/sql/src/plan/statement/ddl.rs b/src/sql/src/plan/statement/ddl.rs index cbfbd9de9c976..13eb6575739bc 100644 --- a/src/sql/src/plan/statement/ddl.rs +++ b/src/sql/src/plan/statement/ddl.rs @@ -6550,22 +6550,14 @@ pub fn plan_alter_cluster( && availability_zones.is_none() && introspection_debugging.is_none() && introspection_interval.is_none() + && experimental_arrangement_compression.is_none() { sql_bail!( "WAIT can only be used together with a SIZE, AVAILABILITY ZONES, \ - or INTROSPECTION change" + INTROSPECTION, or EXPERIMENTAL ARRANGEMENT COMPRESSION change" ); } - match alter_strategy { - AlterClusterPlanStrategy::None => {} - _ => { - scx.require_feature_flag( - &crate::session::vars::ENABLE_ZERO_DOWNTIME_CLUSTER_RECONFIGURATION, - )?; - } - } - if replica_defs.is_some() { sql_bail!("REPLICAS not supported for managed clusters"); } diff --git a/src/sql/src/session/vars.rs b/src/sql/src/session/vars.rs index 1cdc5dd702056..1922337e0e2d8 100644 --- a/src/sql/src/session/vars.rs +++ b/src/sql/src/session/vars.rs @@ -1309,7 +1309,6 @@ impl SystemVars { &cluster_scheduling::CLUSTER_SOFTEN_AZ_AFFINITY, &cluster_scheduling::CLUSTER_SOFTEN_AZ_AFFINITY_WEIGHT, &cluster_scheduling::CLUSTER_ALTER_CHECK_READY_INTERVAL, - &cluster_scheduling::CLUSTER_CHECK_SCHEDULING_POLICIES_INTERVAL, &cluster_scheduling::CLUSTER_SECURITY_CONTEXT_ENABLED, &cluster_scheduling::CLUSTER_REFRESH_MV_COMPACTION_ESTIMATE, &grpc_client::HTTP2_KEEP_ALIVE_TIMEOUT, @@ -2258,10 +2257,6 @@ impl SystemVars { *self.expect_value(&cluster_scheduling::CLUSTER_ALTER_CHECK_READY_INTERVAL) } - pub fn cluster_check_scheduling_policies_interval(&self) -> Duration { - *self.expect_value(&cluster_scheduling::CLUSTER_CHECK_SCHEDULING_POLICIES_INTERVAL) - } - pub fn cluster_security_context_enabled(&self) -> bool { *self.expect_value(&cluster_scheduling::CLUSTER_SECURITY_CONTEXT_ENABLED) } diff --git a/src/sql/src/session/vars/definitions.rs b/src/sql/src/session/vars/definitions.rs index 09e49d767be7e..021aaf69d8c91 100644 --- a/src/sql/src/session/vars/definitions.rs +++ b/src/sql/src/session/vars/definitions.rs @@ -1681,17 +1681,6 @@ pub mod cluster_scheduling { false, ); - const DEFAULT_CHECK_SCHEDULING_POLICIES_INTERVAL: Duration = Duration::from_secs(3); - - pub static CLUSTER_CHECK_SCHEDULING_POLICIES_INTERVAL: VarDefinition = VarDefinition::new( - "cluster_check_scheduling_policies_interval", - value!(Duration; DEFAULT_CHECK_SCHEDULING_POLICIES_INTERVAL), - "How often policies are invoked to automatically start/stop clusters, e.g., \ - for REFRESH EVERY materialized views.", - false, - ) - .with_constraint(&NON_ZERO_DURATION); - pub static CLUSTER_SECURITY_CONTEXT_ENABLED: VarDefinition = VarDefinition::new( "cluster_security_context_enabled", value!(bool; DEFAULT_SECURITY_CONTEXT_ENABLED), @@ -2238,12 +2227,6 @@ feature_flags!( default: false, enable_for_item_parsing: false, }, - { - name: enable_zero_downtime_cluster_reconfiguration, - desc: "Enable zero-downtime reconfiguration for alter cluster", - default: false, - enable_for_item_parsing: false, - }, { name: enable_network_policies, desc: "ENABLE NETWORK POLICIES", diff --git a/src/sqllogictest/src/bin/sqllogictest.rs b/src/sqllogictest/src/bin/sqllogictest.rs index 8e4c7643d0d03..ec6aecd00d419 100644 --- a/src/sqllogictest/src/bin/sqllogictest.rs +++ b/src/sqllogictest/src/bin/sqllogictest.rs @@ -18,7 +18,7 @@ use std::process::ExitCode; use chrono::Utc; use clap::ArgAction; -use mz_adapter_types::dyncfgs::{ENABLE_BACKGROUND_ALTER_CLUSTER, ENABLE_CLUSTER_CONTROLLER}; +use mz_adapter_types::dyncfgs::ENABLE_BACKGROUND_ALTER_CLUSTER; use mz_orchestrator_tracing::{StaticTracingConfig, TracingCliArgs}; use mz_ore::cli::{self, CliConfig, KeyValueArg}; use mz_ore::metrics::MetricsRegistry; @@ -176,19 +176,13 @@ async fn main() -> ExitCode { } } - // The cluster controller and background ALTER CLUSTER land dark in - // production (the dyncfg defaults stay false); force them on for - // sqllogictest so the suite exercises the controller owning the - // managed-cluster replica set. These are dyncfgs (set by name), and a - // caller-provided value wins. - for name in [ - ENABLE_CLUSTER_CONTROLLER.name(), - ENABLE_BACKGROUND_ALTER_CLUSTER.name(), - ] { - system_parameter_defaults - .entry(name.to_string()) - .or_insert_with(|| "true".to_string()); - } + // Pin background ALTER CLUSTER on for the suite so a config-shape + // `ALTER CLUSTER` returns immediately rather than blocking on the + // wait-shim. This is a dyncfg (set by name), and a caller-provided value + // wins. + system_parameter_defaults + .entry(ENABLE_BACKGROUND_ALTER_CLUSTER.name().to_string()) + .or_insert_with(|| "true".to_string()); let config = RunConfig { stdout: &OutputStream::new(io::stdout(), args.timestamps), diff --git a/test/cloudtest/test_managed_cluster.py b/test/cloudtest/test_managed_cluster.py index 46836c605d61a..3f702ae4611ba 100644 --- a/test/cloudtest/test_managed_cluster.py +++ b/test/cloudtest/test_managed_cluster.py @@ -146,7 +146,6 @@ def test_zero_downtime_reconfiguration(mz: MaterializeApplication) -> None: # within the short poll loops below. mz.environmentd.sql( """ - ALTER SYSTEM SET enable_zero_downtime_cluster_reconfiguration = true; ALTER SYSTEM SET cluster_controller_tick_interval = '5ms'; """, port="internal", diff --git a/test/cluster/mzcompose.py b/test/cluster/mzcompose.py index 68bd562e5a380..864e93db4929c 100644 --- a/test/cluster/mzcompose.py +++ b/test/cluster/mzcompose.py @@ -5829,7 +5829,6 @@ def workflow_test_zero_downtime_reconfigure( key${kafka-ingest.iteration}:value${kafka-ingest.iteration} $ postgres-execute connection=postgres://mz_system:materialize@${testdrive.materialize-internal-sql-addr} - ALTER SYSTEM SET enable_zero_downtime_cluster_reconfiguration = true; CREATE CLUSTER cluster1 ( SIZE = 'scale=1,workers=1'); GRANT ALL ON CLUSTER cluster1 TO materialize; @@ -5946,13 +5945,6 @@ def workflow_test_zero_downtime_reconfigure( > SELECT count(*) FROM kafka_tbl 1000 """)) - c.sql( - """ - ALTER SYSTEM RESET enable_zero_downtime_cluster_reconfiguration; - """, - port=6877, - user="mz_system", - ) def workflow_test_pending_replica_audit_events( @@ -5968,11 +5960,10 @@ def workflow_test_pending_replica_audit_events( """ c.up("materialized") - # Enable the WAIT surface and drive the controller tick down so the (empty) - # cluster's reconfiguration converges quickly. + # Drive the controller tick down so the (empty) cluster's reconfiguration + # converges quickly. c.sql( """ - ALTER SYSTEM SET enable_zero_downtime_cluster_reconfiguration = true; ALTER SYSTEM SET cluster_controller_tick_interval = '5ms'; CREATE CLUSTER test_audit (SIZE = 'scale=1,workers=1'); GRANT ALL ON CLUSTER test_audit TO materialize; @@ -6067,7 +6058,6 @@ def workflow_test_pending_replica_audit_events( c.sql( """ DROP CLUSTER test_audit CASCADE; - ALTER SYSTEM RESET enable_zero_downtime_cluster_reconfiguration; """, port=6877, user="mz_system", diff --git a/test/launchdarkly-flag-consistency/mzcompose.py b/test/launchdarkly-flag-consistency/mzcompose.py index 27ab80604948e..e198107e8e59e 100644 --- a/test/launchdarkly-flag-consistency/mzcompose.py +++ b/test/launchdarkly-flag-consistency/mzcompose.py @@ -195,7 +195,6 @@ aws_prefetch_sts_connect_timeout catalog_info_metrics_reconcile_interval cluster_alter_check_ready_interval - cluster_check_scheduling_policies_interval cluster_controller_tick_interval cluster_enable_topology_spread cluster_multi_process_replica_az_affinity_weight @@ -240,9 +239,7 @@ enable_statement_arrival_logging enable_binary_date_bin enable_bounded_staleness_isolation - enable_cluster_controller enable_coalesce_case_transform - enable_cluster_controller enable_compute_half_join2 enable_compute_render_fueled_as_specific_collection enable_date_bin_hopping @@ -468,6 +465,7 @@ enable_repr_typecheck enable_unified_cluster_arrangment enable_yugabyte_connection + enable_zero_downtime_cluster_reconfiguration kafka_default_metadata_fetch_interval mysql_offset_known_interval persist_enable_arrow_lgalloc_noncc_sizes @@ -505,7 +503,6 @@ "enable_scoped_system_parameters", "enable_timely_zero_copy_lgalloc", "enable_upsert_paged_spill", - "enable_zero_downtime_cluster_reconfiguration", "kafka_client_id_enrichment_rules", "kafka_progress_record_fetch_timeout", "kafka_socket_timeout", diff --git a/test/pg-cdc/cluster-graceful-reconfiguration.td b/test/pg-cdc/cluster-graceful-reconfiguration.td index eb87b1e2feb73..54fa5424e683e 100644 --- a/test/pg-cdc/cluster-graceful-reconfiguration.td +++ b/test/pg-cdc/cluster-graceful-reconfiguration.td @@ -16,13 +16,6 @@ # new replica until cut-over drops the old one. Readiness must therefore not # wait for the source to hydrate on the target, but must still wait for the # target's processes to come online before cutting over. -# -# Both reconfiguration paths are exercised below, the legacy foreground path -# and the controller-owned path. The path-selection flags are pinned -# explicitly so the test does not depend on the harness defaults. - -$ postgres-execute connection=postgres://mz_system:materialize@${testdrive.materialize-internal-sql-addr} -ALTER SYSTEM SET enable_zero_downtime_cluster_reconfiguration = true > CREATE SECRET pgpass AS 'postgres' > CREATE CONNECTION pg TO POSTGRES ( @@ -60,64 +53,32 @@ CREATE PUBLICATION mz_source FOR TABLE t1; 2 3 -# ----- Legacy foreground path ----- - $ postgres-execute connection=postgres://mz_system:materialize@${testdrive.materialize-internal-sql-addr} -ALTER SYSTEM SET enable_cluster_controller = false -ALTER SYSTEM SET enable_background_alter_cluster = false - -# The ALTER blocks until readiness passes. Before the fix it would block until -# the deadline rolls the resize back, failing the test at the statement -# timeout. The raised timeout only gives the success path headroom on slow CI. -$ set-sql-timeout duration=120s +ALTER SYSTEM SET enable_background_alter_cluster = true +# With the background flag on the ALTER returns immediately and the controller +# drives readiness and cut-over. Before the fix the resize would roll back at +# its deadline and the realized size below would never advance. > ALTER CLUSTER source_reconfig SET (SIZE 'scale=1,workers=2') WITH (WAIT UNTIL READY (TIMEOUT '300s', ON TIMEOUT 'ROLLBACK')) -$ set-sql-timeout duration=default +# The raised timeout covers replica boot plus the controller's tick cadence. +$ set-sql-timeout duration=120s # The realized size advanced to the target: the cut-over happened. > SELECT size FROM mz_clusters WHERE name = 'source_reconfig' "scale=1,workers=2" -# The source still serves its data after cut-over. -> SELECT * FROM t1 -1 -2 -3 - -# And it keeps ingesting on the promoted replica. -$ postgres-execute connection=postgres://postgres:postgres@postgres -INSERT INTO t1 VALUES (4), (5); +$ set-sql-timeout duration=default +# The source still serves its data after cut-over, and keeps ingesting on the +# promoted replica. > SELECT * FROM t1 1 2 3 -4 -5 - -# ----- Controller-owned path ----- - -$ postgres-execute connection=postgres://mz_system:materialize@${testdrive.materialize-internal-sql-addr} -ALTER SYSTEM SET enable_cluster_controller = true -ALTER SYSTEM SET enable_background_alter_cluster = true -# With the background flag on the ALTER returns immediately and the controller -# drives readiness and cut-over. Before the fix the resize would roll back at -# its deadline and the realized size below would never advance. -> ALTER CLUSTER source_reconfig SET (SIZE 'scale=1,workers=1') WITH (WAIT UNTIL READY (TIMEOUT '300s', ON TIMEOUT 'ROLLBACK')) - -# The raised timeout covers replica boot plus the controller's tick cadence. -$ set-sql-timeout duration=120s - -> SELECT size FROM mz_clusters WHERE name = 'source_reconfig' -"scale=1,workers=1" - -$ set-sql-timeout duration=default - -# The source keeps serving and ingesting after the controller-driven cut-over. $ postgres-execute connection=postgres://postgres:postgres@postgres -INSERT INTO t1 VALUES (6); +INSERT INTO t1 VALUES (4), (5); > SELECT * FROM t1 1 @@ -125,7 +86,6 @@ INSERT INTO t1 VALUES (6); 3 4 5 -6 > DROP SOURCE mz_source CASCADE > DROP CLUSTER source_reconfig diff --git a/test/sqllogictest/autogenerated/mz_internal.slt b/test/sqllogictest/autogenerated/mz_internal.slt index d4fc2da6c6547..c09f62672307c 100644 --- a/test/sqllogictest/autogenerated/mz_internal.slt +++ b/test/sqllogictest/autogenerated/mz_internal.slt @@ -129,7 +129,7 @@ cluster_id text The␠ID␠of␠the␠cluster.␠Corresponds␠to␠`mz_cluste status text The␠lifecycle␠status␠of␠the␠reconfiguration:␠`in-progress`␠while␠the␠controller␠converges␠on␠the␠target,␠then␠a␠terminal␠`finalized`,␠`timed-out`,␠`cancelled`,␠or␠`resource-exhausted`.␠The␠record␠is␠retained␠after␠it␠settles,␠so␠the␠latest␠outcome␠stays␠inspectable␠until␠a␠later␠reconfiguration␠overwrites␠it. deadline mz_timestamp The␠deadline␠by␠which␠the␠reconfiguration␠must␠complete.␠After␠it␠passes,␠the␠`on_timeout`␠action␠applies. on_timeout text The␠action␠applied␠if␠`deadline`␠passes␠before␠the␠target␠hydrates:␠`commit`␠(cut␠over␠to␠the␠not-yet-hydrated␠target)␠or␠`rollback`␠(revert␠to␠the␠pre-reconfiguration␠shape). -target jsonb The␠config␠shape␠the␠cluster␠is␠reconfiguring␠to,␠as␠JSON:␠`size`,␠`replication_factor`,␠`availability_zones`,␠and␠`logging`.␠The␠realized␠(current)␠shape␠is␠in␠`mz_clusters`. +target jsonb The␠config␠shape␠the␠cluster␠is␠reconfiguring␠to,␠as␠JSON:␠`size`,␠`replication_factor`,␠`availability_zones`,␠`logging`,␠and␠`arrangement_compression`.␠The␠realized␠(current)␠shape␠is␠in␠`mz_clusters`. changes jsonb The␠dimensions␠in␠which␠`target`␠differs␠from␠the␠cluster's␠realized␠configuration,␠as␠a␠JSON␠object␠holding␠the␠target␠value␠per␠changed␠dimension.␠Empty␠(`{}`)␠once␠a␠record␠settles␠with␠its␠target␠applied.␠A␠rolled-back␠record␠keeps␠the␠abandoned␠diff. query TTT diff --git a/test/sqllogictest/catalog_server_explain.slt b/test/sqllogictest/catalog_server_explain.slt index 300281a804373..cfea583326dc5 100644 --- a/test/sqllogictest/catalog_server_explain.slt +++ b/test/sqllogictest/catalog_server_explain.slt @@ -2009,7 +2009,7 @@ mz_internal.mz_show_clusters: →Fused with Child Map/Filter/Project Project: #0, #6, #7 Filter: (#1{status} = "in-progress") - Map: ("reconfiguring " || case when ("" = array_to_string(array[("size to " || (#5{changes} ->> "size")), ("replication factor to " || (#5{changes} ->> "replication_factor")), case when ((#5{changes} -> "availability_zones")) IS NOT NULL then "availability zones" else null end, case when ((#5{changes} -> "logging")) IS NOT NULL then "introspection settings" else null end], ", ")) then null else array_to_string(array[("size to " || (#5{changes} ->> "size")), ("replication factor to " || (#5{changes} ->> "replication_factor")), case when ((#5{changes} -> "availability_zones")) IS NOT NULL then "availability zones" else null end, case when ((#5{changes} -> "logging")) IS NOT NULL then "introspection settings" else null end], ", ") end), true + Map: ("reconfiguring " || case when ("" = array_to_string(array[("size to " || (#5{changes} ->> "size")), ("replication factor to " || (#5{changes} ->> "replication_factor")), case when ((#5{changes} -> "availability_zones")) IS NOT NULL then "availability zones" else null end, case when ((#5{changes} -> "logging")) IS NOT NULL then "introspection settings" else null end, case when ((#5{changes} -> "arrangement_compression")) IS NOT NULL then "arrangement compression" else null end], ", ")) then null else array_to_string(array[("size to " || (#5{changes} ->> "size")), ("replication factor to " || (#5{changes} ->> "replication_factor")), case when ((#5{changes} -> "availability_zones")) IS NOT NULL then "availability zones" else null end, case when ((#5{changes} -> "logging")) IS NOT NULL then "introspection settings" else null end, case when ((#5{changes} -> "arrangement_compression")) IS NOT NULL then "arrangement compression" else null end], ", ") end), true →Arranged mz_internal.mz_cluster_reconfigurations Key: (#0{cluster_id}) →Map/Filter/Project @@ -5326,7 +5326,7 @@ mz_internal.mz_cluster_reconfigurations: Source mz_internal.mz_catalog_raw project=(#3, #5..=#7, #4, #8) filter=(("Cluster" = (#0{data} ->> "kind")) AND (json_null != #2) AND (#2 != json_null)) - map=(((((#0{data} -> "value") -> "config") -> "variant") -> "Managed"), (#1 -> "reconfiguration"), parse_catalog_id(((#0{data} -> "key") -> "id")), (#2 -> "target"), case when ("InProgress" = (#2{reconfiguration} ->> "status")) then "in-progress" else case when ("Finalized" = (#2 ->> "status")) then "finalized" else case when ("TimedOut" = (#2 ->> "status")) then "timed-out" else case when ("Cancelled" = (#2 ->> "status")) then "cancelled" else case when ("ResourceExhausted" = (#2 ->> "status")) then "resource-exhausted" else (#2 ->> "status") end end end end end, text_to_mz_timestamp((#2{reconfiguration} ->> "deadline")), case when ("Commit" = (#2{reconfiguration} ->> "on_timeout")) then "commit" else case when ("Rollback" = (#2 ->> "on_timeout")) then "rollback" else (#2 ->> "on_timeout") end end, (((case when ((#1{config} -> "size") != (#4{target} -> "size")) then jsonb_build_object("size", jsonbable_to_jsonb((#4 -> "size"))) else {} end || case when ((#1{config} -> "replication_factor") != (#4{target} -> "replication_factor")) then jsonb_build_object("replication_factor", jsonbable_to_jsonb((#4 -> "replication_factor"))) else {} end) || case when ((#1{config} -> "availability_zones") != (#4{target} -> "availability_zones")) then jsonb_build_object("availability_zones", jsonbable_to_jsonb((#4 -> "availability_zones"))) else {} end) || case when ((#1{config} -> "logging") != (#4{target} -> "logging")) then jsonb_build_object("logging", jsonbable_to_jsonb((#4 -> "logging"))) else {} end)) + map=(((((#0{data} -> "value") -> "config") -> "variant") -> "Managed"), (#1 -> "reconfiguration"), parse_catalog_id(((#0{data} -> "key") -> "id")), (#2 -> "target"), case when ("InProgress" = (#2{reconfiguration} ->> "status")) then "in-progress" else case when ("Finalized" = (#2 ->> "status")) then "finalized" else case when ("TimedOut" = (#2 ->> "status")) then "timed-out" else case when ("Cancelled" = (#2 ->> "status")) then "cancelled" else case when ("ResourceExhausted" = (#2 ->> "status")) then "resource-exhausted" else (#2 ->> "status") end end end end end, text_to_mz_timestamp((#2{reconfiguration} ->> "deadline")), case when ("Commit" = (#2{reconfiguration} ->> "on_timeout")) then "commit" else case when ("Rollback" = (#2 ->> "on_timeout")) then "rollback" else (#2 ->> "on_timeout") end end, ((((case when ((#1{config} -> "size") != (#4{target} -> "size")) then jsonb_build_object("size", jsonbable_to_jsonb((#4 -> "size"))) else {} end || case when ((#1{config} -> "replication_factor") != (#4{target} -> "replication_factor")) then jsonb_build_object("replication_factor", jsonbable_to_jsonb((#4 -> "replication_factor"))) else {} end) || case when ((#1{config} -> "availability_zones") != (#4{target} -> "availability_zones")) then jsonb_build_object("availability_zones", jsonbable_to_jsonb((#4 -> "availability_zones"))) else {} end) || case when ((#1{config} -> "logging") != (#4{target} -> "logging")) then jsonb_build_object("logging", jsonbable_to_jsonb((#4 -> "logging"))) else {} end) || case when ((#1{config} -> "arrangement_compression") != (#4{target} -> "arrangement_compression")) then jsonb_build_object("arrangement_compression", jsonbable_to_jsonb((#4 -> "arrangement_compression"))) else {} end)) Target cluster: mz_catalog_server diff --git a/test/sqllogictest/managed_cluster.slt b/test/sqllogictest/managed_cluster.slt index b9f8b17dfcde6..064f2770450cc 100644 --- a/test/sqllogictest/managed_cluster.slt +++ b/test/sqllogictest/managed_cluster.slt @@ -309,22 +309,35 @@ CREATE CLUSTER foo SIZE invalid_size, REPLICATION FACTOR 0 statement error creating cluster replica would violate max_replicas_per_cluster limit \(desired: 9999999, limit: 5, current: 0\) CREATE CLUSTER foo SIZE 'scale=1,workers=1', replication factor 9999999; -simple conn=mz_system,user=mz_system -ALTER SYSTEM SET enable_zero_downtime_cluster_reconfiguration = true; ----- -COMPLETE 0 - statement ok CREATE CLUSTER foo (SIZE 'scale=1,workers=1') +# A zero timeout that commits already means "cut over now, hydrated or not", so +# the reshape is transacted by the ALTER itself rather than by a later +# controller tick. Both spellings of it take that path, which is why the size +# below can be read back without retrying. + statement ok ALTER CLUSTER foo set (SIZE 'scale=1,workers=2') WITH (WAIT FOR '0s') +query T +SELECT size FROM mz_clusters WHERE name = 'foo' +---- +scale=1,workers=2 + statement ok ALTER CLUSTER foo set (SIZE 'scale=1,workers=4') WITH (WAIT UNTIL READY (TIMEOUT '0s', ON TIMEOUT 'COMMIT') ) +query TT +SELECT c.size, r.size FROM mz_clusters c JOIN mz_cluster_replicas r ON r.cluster_id = c.id WHERE c.name = 'foo' +---- +scale=1,workers=4 scale=1,workers=4 + +# Any other WAIT writes a reconfiguration record the controller converges on, so +# there is nothing synchronous to assert here. + statement ok -ALTER CLUSTER foo set (SIZE 'scale=1,workers=4') WITH (WAIT UNTIL READY (TIMEOUT '10ms', ON TIMEOUT 'ROLLBACK') ) +ALTER CLUSTER foo set (SIZE 'scale=1,workers=2') WITH (WAIT UNTIL READY (TIMEOUT '10ms', ON TIMEOUT 'ROLLBACK') ) statement ok DROP CLUSTER foo @@ -348,23 +361,35 @@ DROP CLUSTER foo # Regression: a `WAIT` clause on an `ALTER` that touches no replica config # shape dimension (`SIZE`, `AVAILABILITY ZONES`, either `INTROSPECTION` -# option) has nothing to hydrate-overlap on, so it is rejected outright -# rather than silently accepted as a no-op wait. +# option, `EXPERIMENTAL ARRANGEMENT COMPRESSION`) has nothing to +# hydrate-overlap on, so it is rejected outright rather than silently +# accepted as a no-op wait. statement ok CREATE CLUSTER foo (SIZE 'scale=1,workers=1') -statement error WAIT can only be used together with a SIZE, AVAILABILITY ZONES, or INTROSPECTION change +statement error WAIT can only be used together with a SIZE, AVAILABILITY ZONES, INTROSPECTION, or EXPERIMENTAL ARRANGEMENT COMPRESSION change ALTER CLUSTER foo SET (SCHEDULE MANUAL) WITH (WAIT FOR '0s') -statement error WAIT can only be used together with a SIZE, AVAILABILITY ZONES, or INTROSPECTION change +statement error WAIT can only be used together with a SIZE, AVAILABILITY ZONES, INTROSPECTION, or EXPERIMENTAL ARRANGEMENT COMPRESSION change ALTER CLUSTER foo SET (REPLICATION FACTOR 2) WITH (WAIT UNTIL READY (TIMEOUT '0s', ON TIMEOUT 'COMMIT')) # A `WAIT` clause paired with a real shape change is still accepted, including -# alongside a non-shape option in the same statement. +# alongside a non-shape option in the same statement. The zero-timeout cut-over +# applies both in the one transaction it commits. statement ok ALTER CLUSTER foo SET (SIZE 'scale=1,workers=2', REPLICATION FACTOR 2) WITH (WAIT FOR '0s') +query TT +SELECT size, replication_factor FROM mz_clusters WHERE name = 'foo' +---- +scale=1,workers=2 2 + +query I +SELECT count(*) FROM mz_cluster_replicas r JOIN mz_clusters c ON c.id = r.cluster_id WHERE c.name = 'foo' +---- +2 + statement ok DROP CLUSTER foo @@ -380,8 +405,9 @@ ALTER CLUSTER foo SET (MANAGED = false) WITH (WAIT UNTIL READY (TIMEOUT '0s', ON statement ok DROP CLUSTER foo -# Regression: zero-downtime finalization (PR #28836) reads workload_class from -# the catalog instead of the planned config, silently dropping the change. +# A `WORKLOAD CLASS` change riding along with a shape change reaches the durable +# config rather than being silently dropped by the reshape. Both statements below +# request the synchronous cut-over, so the readback needs no retry. simple conn=mz_system,user=mz_system CREATE CLUSTER wc_test SIZE 'scale=1,workers=1' @@ -424,8 +450,12 @@ SHOW CREATE CLUSTER ac_test ---- ac_test CREATE␠CLUSTER␠"ac_test"␠(EXPERIMENTAL␠ARRANGEMENT␠COMPRESSION␠=␠true,␠INTROSPECTION␠DEBUGGING␠=␠false,␠INTROSPECTION␠INTERVAL␠=␠INTERVAL␠'00:00:01',␠MANAGED␠=␠true,␠REPLICATION␠FACTOR␠=␠1,␠SIZE␠=␠'scale=1,workers=1',␠SCHEDULE␠=␠MANUAL) +# The zero-timeout WAIT requests the synchronous cut-over, so the realized +# config (what SHOW CREATE renders) reflects the change as soon as the ALTER +# returns. Without it the change transitions through a reconfiguration record +# and the readback would race the controller. statement ok -ALTER CLUSTER ac_test RESET (EXPERIMENTAL ARRANGEMENT COMPRESSION) +ALTER CLUSTER ac_test SET (EXPERIMENTAL ARRANGEMENT COMPRESSION = false) WITH (WAIT FOR '0s') query TT SHOW CREATE CLUSTER ac_test @@ -433,13 +463,19 @@ SHOW CREATE CLUSTER ac_test ac_test CREATE␠CLUSTER␠"ac_test"␠(EXPERIMENTAL␠ARRANGEMENT␠COMPRESSION␠=␠false,␠INTROSPECTION␠DEBUGGING␠=␠false,␠INTROSPECTION␠INTERVAL␠=␠INTERVAL␠'00:00:01',␠MANAGED␠=␠true,␠REPLICATION␠FACTOR␠=␠1,␠SIZE␠=␠'scale=1,workers=1',␠SCHEDULE␠=␠MANUAL) statement ok -ALTER CLUSTER ac_test SET (EXPERIMENTAL ARRANGEMENT COMPRESSION = true) +ALTER CLUSTER ac_test SET (EXPERIMENTAL ARRANGEMENT COMPRESSION = true) WITH (WAIT FOR '0s') query TT SHOW CREATE CLUSTER ac_test ---- ac_test CREATE␠CLUSTER␠"ac_test"␠(EXPERIMENTAL␠ARRANGEMENT␠COMPRESSION␠=␠true,␠INTROSPECTION␠DEBUGGING␠=␠false,␠INTROSPECTION␠INTERVAL␠=␠INTERVAL␠'00:00:01',␠MANAGED␠=␠true,␠REPLICATION␠FACTOR␠=␠1,␠SIZE␠=␠'scale=1,workers=1',␠SCHEDULE␠=␠MANUAL) +# ALTER CLUSTER ... RESET takes no WITH clause, so a shape-dimension RESET +# always transitions through a reconfiguration record and offers no +# deterministic readback here. Exercised for acceptance only. +statement ok +ALTER CLUSTER ac_test RESET (EXPERIMENTAL ARRANGEMENT COMPRESSION) + statement ok DROP CLUSTER ac_test diff --git a/test/sqllogictest/mz_cluster_schedules.slt b/test/sqllogictest/mz_cluster_schedules.slt index fbc86a6d98ea6..b03458927c1c6 100644 --- a/test/sqllogictest/mz_cluster_schedules.slt +++ b/test/sqllogictest/mz_cluster_schedules.slt @@ -161,25 +161,3 @@ JOIN mz_clusters c ON s.cluster_id = c.id WHERE c.name = 'c_rt' ---- 0 - -# --- cluster_check_scheduling_policies_interval rejects zero ------------------ - -# The interval at which scheduling policies run is passed to -# tokio::time::interval during coordinator bootstrap, which panics on a zero -# period. A zero value would therefore crash-loop environmentd on every boot, -# so it must be rejected at ALTER SYSTEM SET time. -simple conn=mz_system,user=mz_system -ALTER SYSTEM SET cluster_check_scheduling_policies_interval = '0s'; ----- -db error: ERROR: parameter "cluster_check_scheduling_policies_interval" cannot have value "0ns": only supports non-zero durations - -# A non-zero value is still accepted. -simple conn=mz_system,user=mz_system -ALTER SYSTEM SET cluster_check_scheduling_policies_interval = '1s'; ----- -COMPLETE 0 - -simple conn=mz_system,user=mz_system -ALTER SYSTEM RESET cluster_check_scheduling_policies_interval; ----- -COMPLETE 0 diff --git a/test/sqllogictest/show_clusters.slt b/test/sqllogictest/show_clusters.slt index 294abd45721e0..a235c5231a926 100644 --- a/test/sqllogictest/show_clusters.slt +++ b/test/sqllogictest/show_clusters.slt @@ -44,9 +44,9 @@ mz_system r1 (scale=1,workers=2) # `SHOW CLUSTERS` surfaces any in-flight reconfiguration or autoscaling action in a -# single `activity` column. With the cluster controller at its default (off), no -# reconfiguration or burst record is ever written, so `activity` is NULL for every -# cluster, managed (baz) or unmanaged (foo). +# single `activity` column. Neither cluster below is ever reconfigured or bursts, +# so no record is written and `activity` is NULL for both, managed (baz) and +# unmanaged (foo). statement ok CREATE CLUSTER baz (SIZE 'scale=1,workers=1', REPLICATION FACTOR 1) diff --git a/test/sqllogictest/system-cluster.slt b/test/sqllogictest/system-cluster.slt index dfbe8f967b9db..c3325b8959e6f 100644 --- a/test/sqllogictest/system-cluster.slt +++ b/test/sqllogictest/system-cluster.slt @@ -626,21 +626,26 @@ ALTER CLUSTER mz_system SET (REPLICATION FACTOR 1) ---- COMPLETE 0 -# Replicas in system clusters should system IDs. - -simple conn=mz_system,user=mz_system -ALTER CLUSTER mz_system SET (SIZE = 'scale=1,workers=2') ----- -COMPLETE 0 +# The declared factor is written synchronously. The replica set itself is the +# cluster controller's, converged on a later tick, so slt (which does not retry) +# must not assert on it. `test/testdrive/system-cluster.td` covers convergence. query I -SELECT COUNT(*) FROM mz_cluster_replicas WHERE cluster_id = (SELECT id FROM mz_clusters WHERE name = 'mz_system') AND id LIKE 's%'; +SELECT replication_factor FROM mz_clusters WHERE name = 'mz_system' ---- 1 -query I -SELECT COUNT(*) FROM mz_cluster_replicas WHERE cluster_id = (SELECT id FROM mz_clusters WHERE name = 'mz_system') AND id LIKE 'u%'; +# Replicas of system clusters carry system ids. Asserted over mz_catalog_server, +# whose replica this file never touches, so the row is there to check: a count +# over mz_system alone would pass vacuously while its replica is being +# reconciled. `test/testdrive/system-cluster.td` covers the id of a replica the +# controller creates at runtime. + +query II +SELECT COUNT(*), COUNT(*) FILTER (WHERE r.id LIKE 's%') FROM mz_cluster_replicas r +JOIN mz_clusters c ON c.id = r.cluster_id +WHERE c.name = 'mz_catalog_server'; ---- -0 +1 1 reset-server diff --git a/test/testdrive/cluster-controller.td b/test/testdrive/cluster-controller.td index 2240b9413a743..33e3ce28e4d95 100644 --- a/test/testdrive/cluster-controller.td +++ b/test/testdrive/cluster-controller.td @@ -7,29 +7,26 @@ # the Business Source License, use of this software will be governed # by the Apache License, Version 2.0. -# Boundary test for the cluster controller. With the master gate forced on and -# the tick interval driven down, the controller reconciles managed clusters many -# times within the test window. Convergence is asynchronous (a config-shape ALTER -# returns before the realized config settles), so every readback below is a -# retrying testdrive query (`>` re-runs until it matches or the SQL timeout -# elapses), never a fixed-duration sleep. Where a transient in-flight state must -# be observed, the controller is frozen first so the state cannot move; where the -# assertion is a negative ("the controller does nothing"), a sibling -# reconfiguration is used as a liveness synchronizer, proving the reconcile loop -# ran many ticks without acting on the cluster under test. +# Boundary test for the cluster controller. With the tick interval driven down, +# the controller reconciles managed clusters many times within the test window. +# Convergence is asynchronous (a config-shape ALTER returns before the realized +# config settles), so every readback below is a retrying testdrive query (`>` +# re-runs until it matches or the SQL timeout elapses), never a fixed-duration +# sleep. Where a transient in-flight state must be observed, the controller is +# held off first (by cranking the tick interval back up) so the state cannot +# move; where the assertion is a negative ("the controller does nothing"), a +# sibling reconfiguration is used as a liveness synchronizer, proving the +# reconcile loop ran many ticks without acting on the cluster under test. # Give convergence (provision + hydrate + cut-over) ample room on a loaded CI host # without ever sleeping the full budget. `>` returns as soon as it matches. $ set-sql-timeout duration=120s -# Force the gate on and drive the tick interval down so the controller ticks -# ~hundreds of times across the waits below. Both are re-read each tick, so the -# flips take effect without a restart. The graceful cases use the WITH (WAIT ...) -# surface, whose planner acceptance is gated on enable_zero_downtime. +# Drive the tick interval down so the controller ticks ~hundreds of times across +# the waits below. It is re-read each tick, so the flip takes effect without a +# restart. $ postgres-execute connection=postgres://mz_system@${testdrive.materialize-internal-sql-addr}/materialize -ALTER SYSTEM SET enable_cluster_controller = true ALTER SYSTEM SET cluster_controller_tick_interval = '5ms' -ALTER SYSTEM SET enable_zero_downtime_cluster_reconfiguration = true # ----- Baseline reconcile is a no-op ----- # @@ -373,7 +370,10 @@ WHERE c.name = 'cc_force_existing' AND r.size = 'scale=1,workers=4' # Replace only the deadline and timeout action. The existing target shape and # replica are reused, and the elapsed deadline forces cut-over on the next tick. -> ALTER CLUSTER cc_force_existing SET (SIZE 'scale=1,workers=4') WITH (WAIT UNTIL READY (TIMEOUT '0s', ON TIMEOUT 'COMMIT')) +# A '1ms' timeout rather than '0s': a zero timeout with COMMIT is the request for +# a synchronous cut-over, which takes the direct path and recreates the replica +# set instead of reusing the target replica (covered in its own section below). +> ALTER CLUSTER cc_force_existing SET (SIZE 'scale=1,workers=4') WITH (WAIT UNTIL READY (TIMEOUT '1ms', ON TIMEOUT 'COMMIT')) > SELECT r.id::text = '${cc_force_existing_target_id}', r.size FROM mz_cluster_replicas r @@ -435,22 +435,104 @@ FROM mz_internal.mz_cluster_reconfigurations recon JOIN mz_clusters c ON c.id = recon.cluster_id WHERE c.name = 'cc_preserve' +# Baseline and overlap run side by side while the record is in flight. Capture +# both ids: the fold below must bounce only the overlap. +> SELECT r.size, count(*) FROM mz_cluster_replicas r JOIN mz_clusters c ON r.cluster_id = c.id WHERE c.name = 'cc_preserve' GROUP BY r.size +scale=1,workers=1 1 +scale=1,workers=4 1 + +$ set-from-sql var=cc_preserve_baseline_id +SELECT r.id::text +FROM mz_cluster_replicas r +JOIN mz_clusters c ON c.id = r.cluster_id +WHERE c.name = 'cc_preserve' AND r.size = 'scale=1,workers=1' + +$ set-from-sql var=cc_preserve_overlap_id +SELECT r.id::text +FROM mz_cluster_replicas r +JOIN mz_clusters c ON c.id = r.cluster_id +WHERE c.name = 'cc_preserve' AND r.size = 'scale=1,workers=4' + # An unrelated no-WAIT ALTER folds onto the record. The size dimension it did # not mention keeps the in-flight target's workers=4, and the deadline and -# on-timeout action are preserved unchanged. +# on-timeout action are preserved unchanged. The compression flip lands in the +# target and is reported as a changed dimension. > ALTER CLUSTER cc_preserve SET (EXPERIMENTAL ARRANGEMENT COMPRESSION = true) -> SELECT recon.status, recon.on_timeout, recon.deadline::text = '${cc_preserve_deadline}', recon.target->>'size' +> SELECT recon.status, recon.on_timeout, recon.deadline::text = '${cc_preserve_deadline}', recon.target->>'size', recon.target->>'arrangement_compression', recon.changes->>'arrangement_compression' FROM mz_internal.mz_cluster_reconfigurations recon JOIN mz_clusters c ON c.id = recon.cluster_id WHERE c.name = 'cc_preserve' -in-progress rollback true scale=1,workers=4 +in-progress rollback true scale=1,workers=4 true true + +# The realized config is untouched by the fold: the compression flip is part of +# the record's target and transitions only at cut-over. Regression: the reshape +# write used to leak the compression dimension into the realized config, which +# had the controller bounce the baseline replica mid-reconfiguration, defeating +# the graceful overlap. +> SELECT create_sql LIKE '%EXPERIMENTAL ARRANGEMENT COMPRESSION = false%' FROM (SHOW CREATE CLUSTER cc_preserve) +true + +# The retarget bounces the overlap replica, whose shape no longer matches the +# folded target. Once the fresh overlap is up, the baseline must still be the +# original replica. +> SELECT r.id::text != '${cc_preserve_overlap_id}' + FROM mz_cluster_replicas r + JOIN mz_clusters c ON c.id = r.cluster_id + WHERE c.name = 'cc_preserve' AND r.size = 'scale=1,workers=4' +true + +> SELECT r.id::text = '${cc_preserve_baseline_id}' + FROM mz_cluster_replicas r + JOIN mz_clusters c ON c.id = r.cluster_id + WHERE c.name = 'cc_preserve' AND r.size = 'scale=1,workers=1' +true > DROP CLUSTER cc_preserve CASCADE $ postgres-execute connection=postgres://mz_system@${testdrive.materialize-internal-sql-addr} ALTER SYSTEM SET unsafe_enable_unstable_dependencies = false +# ----- Compression-only graceful reconfiguration ----- +# +# EXPERIMENTAL ARRANGEMENT COMPRESSION is a replica config shape dimension like +# SIZE: a compression-only ALTER transitions through the record and a +# hydrate-overlap rather than flipping the realized config in place. Nothing +# pins hydration here, so the overlap replica created at the flipped shape +# hydrates, the cut-over advances the realized config, and the baseline replica +# is retired. + +> CREATE CLUSTER cc_compression (SIZE 'scale=1,workers=1', REPLICATION FACTOR 1) + +$ set-from-sql var=cc_compression_baseline_id +SELECT r.id::text +FROM mz_cluster_replicas r +JOIN mz_clusters c ON c.id = r.cluster_id +WHERE c.name = 'cc_compression' + +> ALTER CLUSTER cc_compression SET (EXPERIMENTAL ARRANGEMENT COMPRESSION = true) + +# The record settles, the realized config carries the new value, and the diff +# is empty again. +> SELECT recon.status, recon.changes::text + FROM mz_internal.mz_cluster_reconfigurations recon + JOIN mz_clusters c ON c.id = recon.cluster_id + WHERE c.name = 'cc_compression' +finalized {} + +> SELECT create_sql LIKE '%EXPERIMENTAL ARRANGEMENT COMPRESSION = true%' FROM (SHOW CREATE CLUSTER cc_compression) +true + +# The transition went through the overlap: the surviving replica is the target +# replica, not a bounced baseline. +> SELECT r.id::text != '${cc_compression_baseline_id}', r.size + FROM mz_cluster_replicas r + JOIN mz_clusters c ON c.id = r.cluster_id + WHERE c.name = 'cc_compression' +true scale=1,workers=1 + +> DROP CLUSTER cc_compression + # ----- A single-replica sink does not block graceful reconfiguration ----- # # With enable_background_alter_cluster off, a config-shape ALTER blocks the session @@ -600,23 +682,27 @@ ALTER SYSTEM RESET max_credit_consumption_rate # The unmanaged variant has no reconfiguration field, so the conversion would # silently drop an in-progress record with no terminal status and no audit # event, and strand any overlap replicas. The conversion is refused instead. -# The controller is frozen (master gate off) right after the record is written -# so the in-flight state holds still while the rejection is asserted. +# +# The controller is held off across the assertion by cranking the tick interval +# up before the reconfiguring ALTER. It still reconciles once right after that +# ALTER (a cluster write wakes it), which is what brings the overlap replica up, +# but from then on nothing wakes it: the refused conversion below writes no +# catalog state, so it cannot race a cut-over. + +$ postgres-execute connection=postgres://mz_system@${testdrive.materialize-internal-sql-addr}/materialize +ALTER SYSTEM SET cluster_controller_tick_interval = '10s' > CREATE CLUSTER cc_unmanaged (SIZE 'scale=1,workers=1', REPLICATION FACTOR 1) > ALTER CLUSTER cc_unmanaged SET (SIZE 'scale=1,workers=2') -$ postgres-execute connection=postgres://mz_system@${testdrive.materialize-internal-sql-addr}/materialize -ALTER SYSTEM SET enable_cluster_controller = false - ! ALTER CLUSTER cc_unmanaged SET (MANAGED = false) contains:cannot convert cluster to unmanaged while a reconfiguration is in progress -# Unfreeze and cancel by ALTERing back to the realized size. Once the record -# settles (the retrying `>` rides out the interim), the conversion goes +# Restore the cadence and cancel by ALTERing back to the realized size. Once the +# record settles (the retrying `>` rides out the interim), the conversion goes # through. $ postgres-execute connection=postgres://mz_system@${testdrive.materialize-internal-sql-addr}/materialize -ALTER SYSTEM SET enable_cluster_controller = true +ALTER SYSTEM SET cluster_controller_tick_interval = '5ms' > ALTER CLUSTER cc_unmanaged SET (SIZE 'scale=1,workers=1') > ALTER CLUSTER cc_unmanaged SET (MANAGED = false) @@ -724,11 +810,12 @@ timed-out > SELECT cluster, replica, size FROM (SHOW CLUSTER REPLICAS) WHERE cluster = 'cc_rollback' cc_rollback r1 scale=1,workers=1 -# A forced cut-over: TIMEOUT '0s' with ON TIMEOUT COMMIT advances the realized -# config to the (necessarily un-hydrated) target on the first tick. The retained -# record carries status `finalized` with the deadline, so a reader can tell it was a -# past-deadline (forced) cut-over. -> ALTER CLUSTER cc_rollback SET (SIZE 'scale=1,workers=2') WITH (WAIT UNTIL READY (TIMEOUT '0s', ON TIMEOUT 'COMMIT')) +# A forced cut-over: an elapsed deadline with ON TIMEOUT COMMIT advances the +# realized config to the (necessarily un-hydrated) target on the first tick. The +# retained record carries status `finalized` with the deadline, so a reader can +# tell it was a past-deadline (forced) cut-over. '1ms' rather than '0s', which +# would request the synchronous direct cut-over instead. +> ALTER CLUSTER cc_rollback SET (SIZE 'scale=1,workers=2') WITH (WAIT UNTIL READY (TIMEOUT '1ms', ON TIMEOUT 'COMMIT')) > SELECT size FROM mz_clusters WHERE name = 'cc_rollback' scale=1,workers=2 > SELECT recon.status FROM mz_internal.mz_cluster_reconfigurations recon JOIN mz_clusters ON mz_clusters.id = recon.cluster_id WHERE mz_clusters.name = 'cc_rollback' @@ -1343,213 +1430,111 @@ manual scale=1,workers=2 0 > DROP CLUSTER cc_wait_manual -# ----- Break-glass handoff to the legacy scheduler ----- -# -# Disabling the controller hands scheduled clusters back to the legacy -# scheduler. The controller runs a scheduled cluster's replica while holding -# the durable replication factor at 0, so the legacy scheduler must decide -# on/off from the physical replica set, not the factor. Otherwise a -# controller-created replica would leak across the handoff. - -# Gate-off with the window still open: the legacy scheduler adopts the running -# replica by aligning the replication factor with it, and retires it once the -# window closes. -> CREATE CLUSTER cc_handoff (SIZE 'scale=1,workers=1', SCHEDULE = ON REFRESH (HYDRATION TIME ESTIMATE = '60 seconds')) -> CREATE TABLE cc_handoff_t (x int) -> CREATE MATERIALIZED VIEW cc_handoff_mv IN CLUSTER cc_handoff WITH (REFRESH = EVERY '1 second') AS SELECT count(*) FROM cc_handoff_t -> SELECT count(*) FROM mz_cluster_replicas r JOIN mz_clusters c ON r.cluster_id = c.id WHERE c.name = 'cc_handoff' -1 -> SELECT replication_factor FROM mz_catalog.mz_clusters WHERE name = 'cc_handoff' -0 - -$ postgres-execute connection=postgres://mz_system@${testdrive.materialize-internal-sql-addr}/materialize -ALTER SYSTEM SET enable_cluster_controller = false - -> SELECT replication_factor FROM mz_catalog.mz_clusters WHERE name = 'cc_handoff' -1 -> SELECT count(*) FROM mz_cluster_replicas r JOIN mz_clusters c ON r.cluster_id = c.id WHERE c.name = 'cc_handoff' -1 - -> DROP MATERIALIZED VIEW cc_handoff_mv -> SELECT count(*) FROM mz_cluster_replicas r JOIN mz_clusters c ON r.cluster_id = c.id WHERE c.name = 'cc_handoff' -0 -> SELECT replication_factor FROM mz_catalog.mz_clusters WHERE name = 'cc_handoff' -0 -> DROP CLUSTER cc_handoff -> DROP TABLE cc_handoff_t - -# Gate-off and MV-drop together: the legacy scheduler sees a closed window and -# a replica the factor does not reflect, and still retires it. -$ postgres-execute connection=postgres://mz_system@${testdrive.materialize-internal-sql-addr}/materialize -ALTER SYSTEM SET enable_cluster_controller = true - -> CREATE CLUSTER cc_handoff2 (SIZE 'scale=1,workers=1', SCHEDULE = ON REFRESH (HYDRATION TIME ESTIMATE = '60 seconds')) -> CREATE TABLE cc_handoff2_t (x int) -> CREATE MATERIALIZED VIEW cc_handoff2_mv IN CLUSTER cc_handoff2 WITH (REFRESH = EVERY '1 second') AS SELECT count(*) FROM cc_handoff2_t -> SELECT count(*) FROM mz_cluster_replicas r JOIN mz_clusters c ON r.cluster_id = c.id WHERE c.name = 'cc_handoff2' -1 - -$ postgres-execute connection=postgres://mz_system@${testdrive.materialize-internal-sql-addr}/materialize -ALTER SYSTEM SET enable_cluster_controller = false -DROP MATERIALIZED VIEW materialize.public.cc_handoff2_mv - -> SELECT count(*) FROM mz_cluster_replicas r JOIN mz_clusters c ON r.cluster_id = c.id WHERE c.name = 'cc_handoff2' -0 -> SELECT replication_factor FROM mz_catalog.mz_clusters WHERE name = 'cc_handoff2' -0 -> DROP CLUSTER cc_handoff2 -> DROP TABLE cc_handoff2_t - -# Regression: a mid-window shape bounce lands the surviving replica on a -# non-canonical name (r1 -> r2), and the break-glass adopt aligns only the -# replication factor. A legacy config-shape ALTER must still replace that -# replica: it drops the cluster's observed replica set by id, not the -# factor-derived canonical names, so the resize cannot miss the survivor and -# leave a duplicate behind. -$ postgres-execute connection=postgres://mz_system@${testdrive.materialize-internal-sql-addr}/materialize -ALTER SYSTEM SET enable_cluster_controller = true - -> CREATE CLUSTER cc_handoff3 (SIZE 'scale=1,workers=1', SCHEDULE = ON REFRESH (HYDRATION TIME ESTIMATE = '60 seconds')) -> CREATE TABLE cc_handoff3_t (x int) -> CREATE MATERIALIZED VIEW cc_handoff3_mv IN CLUSTER cc_handoff3 WITH (REFRESH = EVERY '1 second') AS SELECT count(*) FROM cc_handoff3_t -> SELECT count(*) FROM mz_cluster_replicas r JOIN mz_clusters c ON r.cluster_id = c.id WHERE c.name = 'cc_handoff3' -1 - -# The direct-path resize makes the controller bounce the in-window replica, -# which comes back under a fresh, non-canonical name. -> ALTER CLUSTER cc_handoff3 SET (SIZE 'scale=1,workers=2') -> SELECT r.size FROM mz_cluster_replicas r JOIN mz_clusters c ON r.cluster_id = c.id WHERE c.name = 'cc_handoff3' -scale=1,workers=2 -> SELECT count(*) FROM mz_cluster_replicas r JOIN mz_clusters c ON r.cluster_id = c.id WHERE c.name = 'cc_handoff3' -1 - -$ postgres-execute connection=postgres://mz_system@${testdrive.materialize-internal-sql-addr}/materialize -ALTER SYSTEM SET enable_cluster_controller = false - -# The legacy scheduler adopts the surviving replica. -> SELECT replication_factor FROM mz_catalog.mz_clusters WHERE name = 'cc_handoff3' -1 - -# A legacy resize replaces the adopted replica: still exactly one, at the new -# size. -> ALTER CLUSTER cc_handoff3 SET (SIZE 'scale=1,workers=1') -> SELECT count(*) FROM mz_cluster_replicas r JOIN mz_clusters c ON r.cluster_id = c.id WHERE c.name = 'cc_handoff3' -1 -> SELECT r.size FROM mz_cluster_replicas r JOIN mz_clusters c ON r.cluster_id = c.id WHERE c.name = 'cc_handoff3' -scale=1,workers=1 - -# Once the window closes the legacy scheduler retires the replica and the -# factor follows. -> DROP MATERIALIZED VIEW cc_handoff3_mv -> SELECT count(*) FROM mz_cluster_replicas r JOIN mz_clusters c ON r.cluster_id = c.id WHERE c.name = 'cc_handoff3' -0 -> SELECT replication_factor FROM mz_catalog.mz_clusters WHERE name = 'cc_handoff3' -0 -> DROP CLUSTER cc_handoff3 -> DROP TABLE cc_handoff3_t - -# ----- Break-glass cleanup of a stranded graceful reconfiguration ----- +# ----- Direct cut-over: the zero-timeout commit escape hatch ----- # -# Disabling the controller mid-flight strands a graceful reconfiguration: the -# controller is the only component that drives a reconfiguration record to a -# terminal status, so once it is off the in-progress record and the overlap -# replica it provisioned are left in place with nothing to retire them. The -# cleanup lever is a config-shape ALTER to a new size. With the controller off -# it takes the legacy path, which drops the entire observed owned replica set by -# id (baseline plus stranded overlap), recreates a dense r1..rN at the new size, -# and cancels the carried record in the same transaction. An ALTER back to the -# realized shape is not a lever here: it is byte-identical to the realized -# config, so the sequencer short-circuits it as a no-op and the record survives. +# A zero timeout with COMMIT already means "cut over now, hydrated or not", so +# the ALTER transacts the reshape itself instead of writing a record the +# controller would commit one tick later. It converges the replica set with the +# controller's own reconcile kernel, so it lands the same set the controller +# would have, and it retires any carried reconfiguration record. That makes it +# the escape hatch for a wedged reconfiguration, which is the scenario below. # # A sleeping materialized view pins hydration on every replica of the cluster, -# baseline and overlap alike (the same mz_sleep pattern used above), so the -# target set never hydrates and the reconfiguration cannot cut over. The -# in-flight state then holds still while we disable the controller and observe -# the stranding, no matter the tick cadence. +# baseline and overlap alike, so the target set never hydrates and the +# controller cannot cut over. The default deadline is 24h away, so nothing else +# retires the record either: the reconfiguration is stuck until an operator +# intervenes. $ postgres-execute connection=postgres://mz_system@${testdrive.materialize-internal-sql-addr}/materialize -ALTER SYSTEM SET enable_cluster_controller = true ALTER SYSTEM SET unsafe_enable_unstable_dependencies = true -> CREATE CLUSTER cc_strand (SIZE 'scale=1,workers=1', REPLICATION FACTOR 1) +> CREATE CLUSTER cc_cutover (SIZE 'scale=1,workers=1', REPLICATION FACTOR 1) -> CREATE TABLE cc_strand_t (id int) -> INSERT INTO cc_strand_t VALUES (1) +> CREATE TABLE cc_cutover_t (id int) +> INSERT INTO cc_cutover_t VALUES (1) -> CREATE MATERIALIZED VIEW cc_strand_slow IN CLUSTER cc_strand AS - SELECT mz_unsafe.mz_sleep(id * 3600) AS s FROM cc_strand_t +> CREATE MATERIALIZED VIEW cc_cutover_slow IN CLUSTER cc_cutover AS + SELECT mz_unsafe.mz_sleep(id * 3600) AS s FROM cc_cutover_t -# Start a graceful reconfiguration to a new size. The controller writes the -# in-progress record and brings up an overlap replica at the target shape -# alongside the baseline replica, but the sleeping view keeps it from hydrating, -# so no cut-over can happen. Wait until both shapes are up: baseline (workers=1) -# and the target-shape overlap (workers=2) run at once while the realized size -# stays at workers=1. -> ALTER CLUSTER cc_strand SET (SIZE 'scale=1,workers=2') +> ALTER CLUSTER cc_cutover SET (SIZE 'scale=1,workers=2') -> SELECT r.size, count(*) FROM mz_cluster_replicas r JOIN mz_clusters c ON r.cluster_id = c.id WHERE c.name = 'cc_strand' GROUP BY r.size +# Wedged: baseline and overlap run side by side, the realized size is unchanged, +# and the record stays in progress. +> SELECT r.size, count(*) FROM mz_cluster_replicas r JOIN mz_clusters c ON r.cluster_id = c.id WHERE c.name = 'cc_cutover' GROUP BY r.size scale=1,workers=1 1 scale=1,workers=2 1 - -> SELECT recon.status FROM mz_internal.mz_cluster_reconfigurations recon JOIN mz_clusters ON mz_clusters.id = recon.cluster_id WHERE mz_clusters.name = 'cc_strand' +> SELECT size FROM mz_clusters WHERE name = 'cc_cutover' +scale=1,workers=1 +> SELECT recon.status FROM mz_internal.mz_cluster_reconfigurations recon JOIN mz_clusters ON mz_clusters.id = recon.cluster_id WHERE mz_clusters.name = 'cc_cutover' in-progress -# Break-glass: disable the controller mid-flight. Nothing now drives the record. -$ postgres-execute connection=postgres://mz_system@${testdrive.materialize-internal-sql-addr}/materialize -ALTER SYSTEM SET enable_cluster_controller = false +# Re-stating the in-flight target is the "commit this now" case. The overlap +# replica already has the target shape, so the reconcile keeps it and only +# retires the baseline: forcing the commit must not cost a cold restart of a +# replica that was already up. The record reached its own target, so it settles +# `finalized`, marked forced because nothing waited for hydration. +$ set-from-sql var=cc_cutover_target_id +SELECT r.id::text +FROM mz_cluster_replicas r +JOIN mz_clusters c ON c.id = r.cluster_id +WHERE c.name = 'cc_cutover' AND r.size = 'scale=1,workers=2' -# The stranding holds (the controller is frozen): still two replicas across the -# two shapes, the realized size still at workers=1, and the record still -# in-progress with nothing on its way to retire either. -> SELECT r.size, count(*) FROM mz_cluster_replicas r JOIN mz_clusters c ON r.cluster_id = c.id WHERE c.name = 'cc_strand' GROUP BY r.size -scale=1,workers=1 1 -scale=1,workers=2 1 -> SELECT size FROM mz_clusters WHERE name = 'cc_strand' -scale=1,workers=1 -> SELECT recon.status FROM mz_internal.mz_cluster_reconfigurations recon JOIN mz_clusters ON mz_clusters.id = recon.cluster_id WHERE mz_clusters.name = 'cc_strand' -in-progress +> ALTER CLUSTER cc_cutover SET (SIZE 'scale=1,workers=2') WITH (WAIT FOR '0s') -# Not the lever: an ALTER back to the realized shape is byte-identical, so with -# the controller off the sequencer short-circuits it as a no-op. The record -# stays in-progress and the overlap replica is not retired. -> ALTER CLUSTER cc_strand SET (SIZE 'scale=1,workers=1') -> SELECT recon.status FROM mz_internal.mz_cluster_reconfigurations recon JOIN mz_clusters ON mz_clusters.id = recon.cluster_id WHERE mz_clusters.name = 'cc_strand' -in-progress -> SELECT count(*) FROM mz_cluster_replicas r JOIN mz_clusters c ON r.cluster_id = c.id WHERE c.name = 'cc_strand' -2 +> SELECT r.id::text = '${cc_cutover_target_id}', r.size + FROM mz_cluster_replicas r + JOIN mz_clusters c ON c.id = r.cluster_id + WHERE c.name = 'cc_cutover' +true scale=1,workers=2 +> SELECT size FROM mz_clusters WHERE name = 'cc_cutover' +scale=1,workers=2 +> SELECT recon.status FROM mz_internal.mz_cluster_reconfigurations recon JOIN mz_clusters ON mz_clusters.id = recon.cluster_id WHERE mz_clusters.name = 'cc_cutover' +finalized -# The lever: a config-shape ALTER to a genuinely new size. The legacy path drops -# the whole owned replica set (baseline plus stranded overlap) and recreates a -# dense r1..rN at the new size, cancelling the carried record in the same -# transaction. -> ALTER CLUSTER cc_strand SET (SIZE 'scale=1,workers=4') +> SELECT row_number() OVER (ORDER BY id), details->>'transition', details->>'forced' + FROM mz_catalog.mz_audit_events + WHERE event_type = 'alter' AND object_type = 'cluster' + AND details->>'cluster_name' = 'cc_cutover' AND details->>'transition' IS NOT NULL +1 started +2 finalized true -# Desired end state: realized size is the new one, exactly one replica named r1 -# at that size (no orphan left behind), and the stranded record is terminal. -> SELECT size FROM mz_clusters WHERE name = 'cc_strand' -scale=1,workers=4 -> SELECT cluster, replica, size FROM (SHOW CLUSTER REPLICAS) WHERE cluster = 'cc_strand' -cc_strand r1 scale=1,workers=4 -> SELECT recon.status FROM mz_internal.mz_cluster_reconfigurations recon JOIN mz_clusters ON mz_clusters.id = recon.cluster_id WHERE mz_clusters.name = 'cc_strand' +# Now wedge it again and cut over to a *different* size. Nothing matches the new +# target, so the whole owned set goes, including the stranded overlap replica, +# and the abandoned record settles `cancelled` rather than `finalized`. +> ALTER CLUSTER cc_cutover SET (SIZE 'scale=1,workers=4') + +> SELECT r.size, count(*) FROM mz_cluster_replicas r JOIN mz_clusters c ON r.cluster_id = c.id WHERE c.name = 'cc_cutover' GROUP BY r.size +scale=1,workers=2 1 +scale=1,workers=4 1 + +> ALTER CLUSTER cc_cutover SET (SIZE 'scale=1,workers=8') WITH (WAIT FOR '0s') + +> SELECT size FROM mz_clusters WHERE name = 'cc_cutover' +scale=1,workers=8 +> SELECT r.size, count(*) FROM mz_cluster_replicas r JOIN mz_clusters c ON r.cluster_id = c.id WHERE c.name = 'cc_cutover' GROUP BY r.size +scale=1,workers=8 1 +> SELECT recon.status FROM mz_internal.mz_cluster_reconfigurations recon JOIN mz_clusters ON mz_clusters.id = recon.cluster_id WHERE mz_clusters.name = 'cc_cutover' cancelled -# The lifecycle is audited: the record was started, then cancelled by the -# break-glass resize. No finalize or timeout ever ran. -> SELECT row_number() OVER (ORDER BY id), details->>'transition' FROM mz_catalog.mz_audit_events WHERE event_type = 'alter' AND object_type = 'cluster' AND details->>'cluster_name' = 'cc_strand' AND details->>'transition' IS NOT NULL -1 started -2 cancelled +# The other zero-timeout spelling requests the same thing, here with a factor +# change riding along and no record in flight. Both land in the one transaction +# the ALTER commits. +> ALTER CLUSTER cc_cutover SET (SIZE 'scale=1,workers=2', REPLICATION FACTOR 2) WITH (WAIT UNTIL READY (TIMEOUT '0s', ON TIMEOUT 'COMMIT')) + +> SELECT size, replication_factor FROM mz_clusters WHERE name = 'cc_cutover' +scale=1,workers=2 2 +> SELECT r.size, count(*) FROM mz_cluster_replicas r JOIN mz_clusters c ON r.cluster_id = c.id WHERE c.name = 'cc_cutover' GROUP BY r.size +scale=1,workers=2 2 -> DROP CLUSTER cc_strand CASCADE -> DROP TABLE cc_strand_t +> DROP CLUSTER cc_cutover CASCADE +> DROP TABLE cc_cutover_t $ postgres-execute connection=postgres://mz_system@${testdrive.materialize-internal-sql-addr}/materialize ALTER SYSTEM SET unsafe_enable_unstable_dependencies = false # Restore pristine server state (including the tick-interval override). $ postgres-execute connection=postgres://mz_system@${testdrive.materialize-internal-sql-addr}/materialize -ALTER SYSTEM RESET enable_cluster_controller ALTER SYSTEM RESET cluster_controller_tick_interval -ALTER SYSTEM RESET enable_zero_downtime_cluster_reconfiguration ALTER SYSTEM RESET enable_background_alter_cluster ALTER SYSTEM RESET enable_auto_scaling_strategy ALTER SYSTEM RESET enable_hydration_burst diff --git a/test/testdrive/system-cluster.td b/test/testdrive/system-cluster.td index c4496fb04e21d..4f7ff2a9f78c4 100644 --- a/test/testdrive/system-cluster.td +++ b/test/testdrive/system-cluster.td @@ -113,6 +113,11 @@ mz_system r1 bootstrap "" ! ALTER CLUSTER mz_support SET (replication factor 1, size 'scale=1,workers=2'); contains: must be owner of CLUSTER mz_support +# The cluster controller owns a system cluster's replica set, so every ALTER +# below lands its config synchronously and the replica set (and, for a +# config-shape change, the realized config itself) converges on a later tick. +# Every readback is a retrying `>` for that reason. + $ postgres-execute connection=postgres://mz_support@${testdrive.materialize-internal-sql-addr}/materializeI ALTER CLUSTER mz_support SET (replication factor 0, SIZE 'scale=1,workers=1'); @@ -148,3 +153,62 @@ ALTER CLUSTER mz_support SET (introspection interval = '0s'); > select name, introspection_debugging, introspection_interval from mz_catalog.mz_clusters where name = 'mz_support'; mz_support true + +# Convergence: the controller materializes and retires a system cluster's +# replicas from its replication factor, and a config-shape change cuts the +# realized config over once the target is up. The replicas it creates carry +# system ids, like the ones the catalog-open migration creates. +# +# Every readback below is a retrying `>`, with room for a replica boot plus a +# controller tick on a loaded CI host. `>` returns as soon as it matches, so the +# budget is never actually slept. +$ set-sql-timeout duration=120s + +$ postgres-execute connection=postgres://mz_support@${testdrive.materialize-internal-sql-addr}/materializeI +ALTER CLUSTER mz_support SET (REPLICATION FACTOR 1); + +> SELECT count(*), bool_and(r.id LIKE 's%') + FROM mz_cluster_replicas r + JOIN mz_clusters c ON c.id = r.cluster_id + WHERE c.name = 'mz_support' +1 true + +$ postgres-execute connection=postgres://mz_support@${testdrive.materialize-internal-sql-addr}/materializeI +ALTER CLUSTER mz_support SET (SIZE 'scale=1,workers=2'); + +> SELECT size FROM mz_clusters WHERE name = 'mz_support' +scale=1,workers=2 + +> SELECT r.size, count(*) + FROM mz_cluster_replicas r + JOIN mz_clusters c ON c.id = r.cluster_id + WHERE c.name = 'mz_support' + GROUP BY r.size +scale=1,workers=2 1 + +# The shape change really went through the graceful path: it left a settled +# reconfiguration record behind, which is the capability system clusters did not +# have before the controller owned them. + +> SELECT recon.status + FROM mz_internal.mz_cluster_reconfigurations recon + JOIN mz_clusters c ON c.id = recon.cluster_id + WHERE c.name = 'mz_support' +finalized + +# Back to the default: no replica, and the cluster reports no activity, so the +# reconfiguration above really did settle rather than park in progress. + +$ postgres-execute connection=postgres://mz_support@${testdrive.materialize-internal-sql-addr}/materializeI +ALTER CLUSTER mz_support SET (REPLICATION FACTOR 0, SIZE 'scale=1,workers=1'); + +> SELECT count(*) + FROM mz_cluster_replicas r + JOIN mz_clusters c ON c.id = r.cluster_id + WHERE c.name = 'mz_support' +0 + +> SELECT activity FROM (SHOW CLUSTERS) WHERE name = 'mz_support' + + +$ set-sql-timeout duration=default