Skip to content

Commit 27b4877

Browse files
committed
adapter: remove ENABLE_CLUSTER_CONTROLLER and the legacy REFRESH scheduler
The cluster controller has been default-on since v26.29 and owns the replica set of every user managed cluster. The break-glass dyncfg kept two legacy paths reachable: the REFRESH scheduler in `cluster_scheduling.rs`, whose two entry points already returned before doing any work while the gate was on, and the legacy branches in the ALTER sequencer. Delete the gate and everything only it kept alive: the scheduler module, its coordinator plumbing (two messages, the timer, the select-loop tick, the `cluster_scheduling_decisions` state), the two scheduler metrics, the `cluster_check_scheduling_policies_interval` system var, and the `ReplicaCreateDropReason::ClusterScheduling` variant. The persisted audit vocabulary stays: `SchedulingDecisionsWithReasonsV2` and friends are written by the controller's on-refresh path too, and old events must remain decodable.
1 parent 18abe8d commit 27b4877

25 files changed

Lines changed: 131 additions & 1140 deletions

File tree

doc/user/data/metrics.yml

Lines changed: 0 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -453,28 +453,6 @@ metrics:
453453
help: Total number of started transactions.
454454
source: src/catalog/src/durable/metrics.rs
455455
visibility: internal
456-
- name: mz_check_scheduling_policies_seconds_bucket
457-
help: The time each policy in `check_scheduling_policies` takes.
458-
labels:
459-
- le
460-
- policy
461-
- thread
462-
source: src/adapter/src/metrics.rs
463-
visibility: internal
464-
- name: mz_check_scheduling_policies_seconds_count
465-
help: The time each policy in `check_scheduling_policies` takes.
466-
labels:
467-
- policy
468-
- thread
469-
source: src/adapter/src/metrics.rs
470-
visibility: internal
471-
- name: mz_check_scheduling_policies_seconds_sum
472-
help: The time each policy in `check_scheduling_policies` takes.
473-
labels:
474-
- policy
475-
- thread
476-
source: src/adapter/src/metrics.rs
477-
visibility: internal
478456
- name: mz_cluster_handle_command_duration_seconds_bucket
479457
help: Time spent in handling commands.
480458
labels:
@@ -971,25 +949,6 @@ metrics:
971949
help: The time it takes to advance the catalog shard upper for a txns-shard write (group commits and table register/forget).
972950
source: src/adapter/src/metrics.rs
973951
visibility: internal
974-
- name: mz_handle_scheduling_decisions_seconds_bucket
975-
help: The time `handle_scheduling_decisions` takes.
976-
labels:
977-
- altered_a_cluster
978-
- le
979-
source: src/adapter/src/metrics.rs
980-
visibility: internal
981-
- name: mz_handle_scheduling_decisions_seconds_count
982-
help: The time `handle_scheduling_decisions` takes.
983-
labels:
984-
- altered_a_cluster
985-
source: src/adapter/src/metrics.rs
986-
visibility: internal
987-
- name: mz_handle_scheduling_decisions_seconds_sum
988-
help: The time `handle_scheduling_decisions` takes.
989-
labels:
990-
- altered_a_cluster
991-
source: src/adapter/src/metrics.rs
992-
visibility: internal
993952
- name: mz_index_peek_cursor_setup_seconds_bucket
994953
help: Time setting up cursor and literal constraints.
995954
labels:

misc/python/materialize/mzcompose/__init__.py

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -97,14 +97,9 @@ def get_minimal_system_parameters(
9797
"enable_refresh_every_mvs": "true",
9898
"enable_replacement_materialized_views": "true",
9999
"enable_cluster_schedule_refresh": "true",
100-
# The cluster controller and background ALTER CLUSTER dyncfgs default on
101-
# in current versions. Pin them explicitly so runs against older versions
102-
# (which predate the flags or defaulted them off) exercise the legacy
103-
# paths while current versions exercise the controller owning the
104-
# managed-cluster replica set.
105-
"enable_cluster_controller": (
106-
"true" if version >= MzVersion.parse_mz("v26.29.0-dev") else "false"
107-
),
100+
# Pinned explicitly so runs against older versions (which predate the
101+
# flag or defaulted it off) behave like current ones, where it defaults
102+
# on.
108103
"enable_background_alter_cluster": (
109104
"true" if version >= MzVersion.parse_mz("v26.29.0-dev") else "false"
110105
),
@@ -127,6 +122,15 @@ def get_minimal_system_parameters(
127122
if version < MzVersion.parse_mz("v0.163.0-dev"):
128123
config["enable_compute_active_dataflow_cancelation"] = "true"
129124

125+
# The cluster controller's break-glass gate. Removed in v26.37, where the
126+
# controller runs unconditionally. Older binaries still read it, and
127+
# defaulted it off before v26.29, so pin it on for them to keep mixed-version
128+
# runs exercising the same path as current versions.
129+
if version < MzVersion.parse_mz("v26.37.0-dev"):
130+
config["enable_cluster_controller"] = (
131+
"true" if version >= MzVersion.parse_mz("v26.29.0-dev") else "false"
132+
)
133+
130134
return config
131135

132136

misc/python/materialize/parallel_workload/action.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3225,7 +3225,6 @@ def __init__(
32253225
"oidc_group_role_sync_strict",
32263226
"console_oidc_client_id",
32273227
"console_oidc_scopes",
3228-
"enable_cluster_controller",
32293228
"cluster_controller_tick_interval",
32303229
"enable_background_alter_cluster",
32313230
"default_cluster_reconfiguration_timeout",

src/adapter-types/src/dyncfgs.rs

Lines changed: 3 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -353,23 +353,7 @@ pub const ENABLE_SCOPED_SYSTEM_PARAMETERS: Config<bool> = Config::new(
353353
"Whether per-cluster and per-replica scoped system parameters are evaluated and applied.",
354354
);
355355

356-
/// Top-level gate for the cluster controller. When on, the controller owns the
357-
/// managed-cluster replica set and the legacy paths (the graceful 3-stage
358-
/// machine and `cluster_scheduling.rs`) are bypassed. The replica set cannot
359-
/// have two writers, so this is a clean switch, not a per-strategy toggle.
360-
///
361-
/// Defaults on. This is the break-glass switch to fall back to the legacy
362-
/// paths if the controller misbehaves.
363-
pub const ENABLE_CLUSTER_CONTROLLER: Config<bool> = Config::new(
364-
"enable_cluster_controller",
365-
true,
366-
"Whether the cluster controller owns the managed-cluster replica set. When false, the legacy scheduling and graceful-reconfiguration paths run instead.",
367-
);
368-
369356
/// Cadence of the cluster controller's reconcile tick.
370-
///
371-
/// Replaces `cluster_check_scheduling_policies_interval` once the controller is
372-
/// the sole owner; while the controller is dark both intervals exist.
373357
pub const CLUSTER_CONTROLLER_TICK_INTERVAL: Config<Duration> = Config::new(
374358
"cluster_controller_tick_interval",
375359
Duration::from_secs(5),
@@ -380,9 +364,6 @@ pub const CLUSTER_CONTROLLER_TICK_INTERVAL: Config<Duration> = Config::new(
380364
/// controller converging in the background, or blocks the session on a
381365
/// wait-shim until the reconfiguration completes or its deadline passes.
382366
///
383-
/// Only consulted while [`ENABLE_CLUSTER_CONTROLLER`] is on, when the
384-
/// controller owns the reconfiguration.
385-
///
386367
/// Defaults on. This is the break-glass switch back to the blocking wait-shim
387368
/// if returning immediately causes trouble.
388369
pub const ENABLE_BACKGROUND_ALTER_CLUSTER: Config<bool> = Config::new(
@@ -404,9 +385,9 @@ pub const DEFAULT_CLUSTER_RECONFIGURATION_TIMEOUT: Config<Duration> = Config::ne
404385
/// runs a burst replica; graceful reconfiguration and `ON REFRESH` scheduling
405386
/// are unaffected.
406387
///
407-
/// Only consulted while [`ENABLE_CLUSTER_CONTROLLER`] is on. A cluster can only
408-
/// carry an `AUTO SCALING STRATEGY` while its SQL acceptance feature flag is
409-
/// on, so this is the second of the two gates burst sits behind.
388+
/// A cluster can only carry an `AUTO SCALING STRATEGY` while its SQL acceptance
389+
/// feature flag is on, so this is the second of the two gates burst sits
390+
/// behind.
410391
pub const ENABLE_HYDRATION_BURST: Config<bool> = Config::new(
411392
"enable_hydration_burst",
412393
true,
@@ -426,7 +407,6 @@ pub const DEFAULT_HYDRATION_BURST_LINGER: Config<Duration> = Config::new(
426407
pub fn all_dyncfgs(configs: ConfigSet) -> ConfigSet {
427408
configs
428409
.add(&ALLOW_USER_SESSIONS)
429-
.add(&ENABLE_CLUSTER_CONTROLLER)
430410
.add(&CLUSTER_CONTROLLER_TICK_INTERVAL)
431411
.add(&ENABLE_BACKGROUND_ALTER_CLUSTER)
432412
.add(&DEFAULT_CLUSTER_RECONFIGURATION_TIMEOUT)

src/adapter/src/catalog/open.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1227,9 +1227,9 @@ fn reconcile_builtin_cluster_replicas(
12271227
}
12281228

12291229
// Reading the cluster's factor is what makes this compose with the other
1230-
// writers of a replica set. The refresh scheduler parks a scheduled cluster
1231-
// by writing its factor to 0, so converging on the factor honors that
1232-
// instead of resurrecting a replica the scheduler just dropped.
1230+
// writers of a replica set. The controller's on-refresh strategy parks a
1231+
// scheduled cluster by writing its factor to 0, so converging on the
1232+
// factor honors that instead of resurrecting a replica it just dropped.
12331233
let mut surplus = replicas_by_cluster.remove(&cluster.id).unwrap_or_default();
12341234
for index in 0..managed.replication_factor {
12351235
let replica_name = managed_cluster_replica_name(index);

src/adapter/src/catalog/transact.rs

Lines changed: 9 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,6 @@ use crate::catalog::{
8686
use crate::config::{ScopedParameters, ScopedParametersScope};
8787
use crate::coord::ConnMeta;
8888
use crate::coord::catalog_implications::parsed_state_updates::ParsedStateUpdate;
89-
use crate::coord::cluster_scheduling::SchedulingDecision;
9089
use crate::util::ResultExt;
9190

9291
/// A manually injected audit event.
@@ -360,9 +359,6 @@ pub enum ReplicaCreateDropReason {
360359
/// - ALTERing various options on a managed cluster,
361360
/// - CREATE/DROP CLUSTER REPLICA on an unmanaged cluster.
362361
Manual,
363-
/// The automated cluster scheduling initiated the replica create or drop, e.g., a
364-
/// materialized view is needing a refresh on a SCHEDULE ON REFRESH cluster.
365-
ClusterScheduling(Vec<SchedulingDecision>),
366362
/// The cluster controller's graceful-reconfiguration strategy created the replica while
367363
/// converging a cluster onto an in-flight `reconfiguration` target (a background
368364
/// `ALTER CLUSTER`).
@@ -373,11 +369,7 @@ pub enum ReplicaCreateDropReason {
373369
/// The cluster controller's on-refresh strategy created the replica for a refresh window on
374370
/// a `SCHEDULE = ON REFRESH` cluster. Audited as the `schedule` reason, carrying the tick's
375371
/// window decision (which MVs needed a refresh or compaction time, and the hydration-time
376-
/// estimate) as the `scheduling_policies` detail, the same detail the legacy scheduler's
377-
/// [`ReplicaCreateDropReason::ClusterScheduling`] records. Deliberately not that variant
378-
/// itself: its legacy shape carries a per-policy `Vec` and an on/off flag for auditing
379-
/// off-decisions, neither of which the controller has (controller drops are uniformly
380-
/// `Retired`), and it is removed together with the legacy scheduler.
372+
/// estimate) as the `scheduling_policies` detail.
381373
OnRefresh(RefreshWindowDecision),
382374
/// The cluster controller dropped the replica because the cluster's configuration no longer
383375
/// calls for it. The uniform reason on every controller-emitted drop (e.g. a
@@ -394,12 +386,6 @@ impl ReplicaCreateDropReason {
394386
) {
395387
match self {
396388
ReplicaCreateDropReason::Manual => (CreateOrDropClusterReplicaReasonV1::Manual, None),
397-
ReplicaCreateDropReason::ClusterScheduling(scheduling_decisions) => (
398-
CreateOrDropClusterReplicaReasonV1::Schedule,
399-
Some(SchedulingDecision::reasons_to_audit_log_reasons(
400-
&scheduling_decisions,
401-
)),
402-
),
403389
ReplicaCreateDropReason::GracefulReconfiguration => {
404390
(CreateOrDropClusterReplicaReasonV1::Reconfiguration, None)
405391
}
@@ -416,8 +402,8 @@ impl ReplicaCreateDropReason {
416402
}
417403

418404
/// Convert the controller's on-refresh window decision into the audit log's
419-
/// `scheduling_policies` detail, the same shape the legacy scheduler records:
420-
/// ids as strings and the hydration-time estimate as an interval string.
405+
/// `scheduling_policies` detail: ids as strings and the hydration-time estimate
406+
/// as an interval string.
421407
fn refresh_window_decision_to_audit_log(
422408
decision: RefreshWindowDecision,
423409
) -> SchedulingDecisionsWithReasonsV2 {
@@ -514,8 +500,8 @@ impl Catalog {
514500
/// status change, a fresh record, or the drop of an in-progress record.
515501
///
516502
/// Every such movement is an audit-log transition, so a write performing
517-
/// one must declare the matching intent. Status-preserving copies (legacy
518-
/// paths carrying a record forward, re-targets that stay in progress with a
503+
/// one must declare the matching intent. Status-preserving copies (a write
504+
/// carrying a record forward, re-targets that stay in progress with a
519505
/// declared `Started`) and drops of already-settled records move nothing.
520506
fn reconfiguration_lifecycle_moved(
521507
old_config: &ClusterConfig,
@@ -3827,8 +3813,8 @@ mod tests {
38273813
&unmanaged,
38283814
));
38293815

3830-
// Not movements: no record at all, a status-preserving copy (legacy
3831-
// paths carry the record forward), and dropping a settled record.
3816+
// Not movements: no record at all, a status-preserving copy (a write
3817+
// that carries the record forward), and dropping a settled record.
38323818
assert!(!Catalog::reconfiguration_lifecycle_moved(
38333819
&managed(None),
38343820
&managed(None),
@@ -3985,9 +3971,8 @@ mod tests {
39853971

39863972
use crate::catalog::ReplicaCreateDropReason;
39873973

3988-
// `OnRefresh` shares the `schedule` audit word with the legacy
3989-
// `ClusterScheduling` variant and converts the controller's window
3990-
// decision into the same `scheduling_policies` detail blob: ids as
3974+
// `OnRefresh` audits the `schedule` word and converts the controller's
3975+
// window decision into the `scheduling_policies` detail blob: ids as
39913976
// strings, the hydration-time estimate as an interval string, and the
39923977
// decision hardcoded `on` (the controller produces a create, and so
39933978
// this detail, only for an open window).

src/adapter/src/coord.rs

Lines changed: 0 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,6 @@ use crate::coord::appends::{
195195
PendingWriteTxn,
196196
};
197197
use crate::coord::caught_up::CaughtUpCheckContext;
198-
use crate::coord::cluster_scheduling::SchedulingDecision;
199198
use crate::coord::id_bundle::CollectionIdBundle;
200199
use crate::coord::introspection::IntrospectionSubscribe;
201200
use crate::coord::peek::PendingPeek;
@@ -220,7 +219,6 @@ use crate::{AdapterNotice, ReadHolds, flags};
220219
pub(crate) mod appends;
221220
pub(crate) mod catalog_serving;
222221
pub(crate) mod cluster_controller;
223-
pub(crate) mod cluster_scheduling;
224222
pub(crate) mod consistency;
225223
pub(crate) mod id_bundle;
226224
pub(crate) mod in_memory_oracle;
@@ -444,13 +442,6 @@ pub enum Message {
444442
},
445443
DrainStatementLog,
446444
PrivateLinkVpcEndpointEvents(Vec<VpcEndpointEvent>),
447-
CheckSchedulingPolicies,
448-
449-
/// Scheduling policy decisions about turning clusters On/Off.
450-
/// `Vec<(policy name, Vec of decisions by the policy)>`
451-
/// A cluster will be On if and only if there is at least one On decision for it.
452-
/// Scheduling decisions for clusters that have `SCHEDULE = MANUAL` are ignored.
453-
SchedulingDecisions(Vec<(&'static str, Vec<(ClusterId, SchedulingDecision)>)>),
454445

455446
/// One pull/apply call from the cluster controller task, answered on the main
456447
/// coordinator message loop from the catalog and live controller signals.
@@ -558,8 +549,6 @@ impl Message {
558549
Message::DrainStatementLog => "drain_statement_log",
559550
Message::AlterConnectionValidationReady(..) => "alter_connection_validation_ready",
560551
Message::PrivateLinkVpcEndpointEvents(_) => "private_link_vpc_endpoint_events",
561-
Message::CheckSchedulingPolicies => "check_scheduling_policies",
562-
Message::SchedulingDecisions { .. } => "scheduling_decision",
563552
Message::ClusterControllerRequest(_) => "cluster_controller_request",
564553
Message::DeferredStatementReady => "deferred_statement_ready",
565554
}
@@ -2132,14 +2121,6 @@ pub struct Coordinator {
21322121
/// a timestamp oracle backend is configured.
21332122
timestamp_oracle_config: Option<TimestampOracleConfig>,
21342123

2135-
/// Periodically asks cluster scheduling policies to make their decisions.
2136-
check_cluster_scheduling_policies_interval: Interval,
2137-
2138-
/// This keeps the last On/Off decision for each cluster and each scheduling policy.
2139-
/// (Clusters that have been dropped or are otherwise out of scope for automatic scheduling are
2140-
/// periodically cleaned up from this Map.)
2141-
cluster_scheduling_decisions: BTreeMap<ClusterId, BTreeMap<&'static str, SchedulingDecision>>,
2142-
21432124
/// When doing 0dt upgrades/in read-only mode, periodically ask all known
21442125
/// clusters/collections whether they are caught up.
21452126
caught_up_check_interval: Interval,
@@ -4039,13 +4020,6 @@ impl Coordinator {
40394020
linearize_reads_notified.set(linearize_reads_notify.notified());
40404021
messages.push(Message::LinearizeReads);
40414022
}
4042-
// `tick()` on `Interval` is cancel-safe:
4043-
// https://docs.rs/tokio/1.19.2/tokio/time/struct.Interval.html#cancel-safety
4044-
// Receive a single command.
4045-
_ = self.check_cluster_scheduling_policies_interval.tick() => {
4046-
messages.push(Message::CheckSchedulingPolicies);
4047-
},
4048-
40494023
// `tick()` on `Interval` is cancel-safe:
40504024
// https://docs.rs/tokio/1.19.2/tokio/time/struct.Interval.html#cancel-safety
40514025
// Receive a single command.
@@ -4926,12 +4900,6 @@ pub fn serve(
49264900
let coord_now = now.clone();
49274901
let advance_timelines_interval =
49284902
tokio::time::interval(catalog.system_config().default_timestamp_interval());
4929-
let mut check_scheduling_policies_interval = tokio::time::interval(
4930-
catalog
4931-
.system_config()
4932-
.cluster_check_scheduling_policies_interval(),
4933-
);
4934-
check_scheduling_policies_interval.set_missed_tick_behavior(MissedTickBehavior::Delay);
49354903

49364904
let clusters_caught_up_check_interval = if read_only_controllers {
49374905
let dyncfgs = catalog.system_config().dyncfgs();
@@ -5120,8 +5088,6 @@ pub fn serve(
51205088
statement_logging: StatementLogging::new(coord_now.clone()),
51215089
webhook_concurrency_limit,
51225090
timestamp_oracle_config,
5123-
check_cluster_scheduling_policies_interval: check_scheduling_policies_interval,
5124-
cluster_scheduling_decisions: BTreeMap::new(),
51255091
caught_up_check_interval: clusters_caught_up_check_interval,
51265092
caught_up_check: clusters_caught_up_check,
51275093
installed_watch_sets: BTreeMap::new(),

0 commit comments

Comments
 (0)