|
| 1 | +# Remove the legacy cluster scheduling and staged reconfiguration paths |
| 2 | + |
| 3 | +- Associated (controller work, merged): #37214, #37452, #37671, #37767 |
| 4 | +- Associated (config owns builtin replicas, merged): #37929 |
| 5 | + |
| 6 | +## The Problem |
| 7 | + |
| 8 | +The cluster controller is merged, default-on since v26.29, and owns the |
| 9 | +replica set of every *user* managed cluster in production. But it still lives |
| 10 | +behind the break-glass dyncfg `ENABLE_CLUSTER_CONTROLLER`, and system/builtin |
| 11 | +clusters are excluded from its ownership (`ManagedClusterIds` filters |
| 12 | +`is_user()`). Those two facts keep three legacy code paths alive: |
| 13 | + |
| 14 | +1. **The legacy REFRESH scheduler** (`src/adapter/src/coord/cluster_scheduling.rs`), |
| 15 | + driven by a coordinator timer, superseded by the controller's |
| 16 | + `OnRefreshStrategy`. Both of its entry points return before doing any work |
| 17 | + while the gate is on, so it is a strict no-op in production. Only the gate |
| 18 | + keeps it reachable. |
| 19 | +2. **The legacy staged reconfiguration machine**: the `WaitForHydrated` and |
| 20 | + `Finalize` cluster stages, the `-pending` overlap replicas, and their |
| 21 | + connection-lifecycle cleanup, superseded by the controller's durable |
| 22 | + reconfiguration record plus the `AwaitReconfiguration` wait-shim. With the |
| 23 | + gate on it is reachable through exactly one door: a config-shape |
| 24 | + `ALTER CLUSTER <system cluster> ... WITH (WAIT ...)`. |
| 25 | +3. **The legacy direct replica create/drop path** in |
| 26 | + `sequence_alter_cluster_managed_to_managed`: synchronous whole-set |
| 27 | + recreation on a size change, direct create/drop on a replication-factor |
| 28 | + change. With the gate on it runs only for system clusters, which is the |
| 29 | + system-cluster exception in action: the sequencer must materialize their |
| 30 | + replicas itself because the controller will not. This path does not go |
| 31 | + away entirely, it shrinks to a single explicitly requested cut-over |
| 32 | + branch (see Part 3). |
| 33 | + |
| 34 | +The system-cluster exception was originally load-bearing: the boot-time |
| 35 | +builtin replica migration and the controller would have been two conflicting |
| 36 | +writers of one replica set. #37929 removed that conflict. |
| 37 | +`reconcile_builtin_cluster_replicas` now converges a builtin cluster's |
| 38 | +replica set on the cluster's own managed config at catalog open, the same |
| 39 | +config the controller derives its targets from, so the two converge on the |
| 40 | +same set by construction. The exception no longer prevents a bug. It only |
| 41 | +forces the sequencer to keep a second replica-materialization implementation |
| 42 | +alive for system clusters. |
| 43 | + |
| 44 | +## Success Criteria |
| 45 | + |
| 46 | +- The cluster controller owns the replica set of *all* managed clusters, user |
| 47 | + and system alike. The `is_user()` ownership conjuncts are gone. |
| 48 | +- `ENABLE_CLUSTER_CONTROLLER` is removed. The controller runs unconditionally |
| 49 | + (it still quiesces while the deployment is read-only). |
| 50 | +- The legacy REFRESH scheduler and the legacy staged reconfiguration machine |
| 51 | + are deleted, not fenced off. The direct create/drop path shrinks to a single |
| 52 | + cut-over branch, kept as an explicitly requested escape hatch (see Part 3). |
| 53 | +- That escape hatch is reachable on any deployment, so the statement that |
| 54 | + requests it is no longer feature-gated (see Part 4). |
| 55 | +- Boot ordering and 0dt read-only behavior are preserved: system clusters have |
| 56 | + their replicas up and hydrated before the serve loop starts and before a 0dt |
| 57 | + cutover, provided by `reconcile_builtin_cluster_replicas` at catalog open, |
| 58 | + which stays. |
| 59 | + |
| 60 | +## Out of Scope |
| 61 | + |
| 62 | +- Removing the controller sub-behavior kill-switches |
| 63 | + `ENABLE_BACKGROUND_ALTER_CLUSTER` and `ENABLE_HYDRATION_BURST`. They gate |
| 64 | + controller behaviors, not legacy paths, and retire separately after burn-in. |
| 65 | +- Removing the feature-acceptance flags `enable_cluster_schedule_refresh` and |
| 66 | + `enable_auto_scaling_strategy`. Those gate user-facing SQL surface under a |
| 67 | + staged rollout. |
| 68 | +- Removing the durable `pending` field on `ReplicaLocation::Managed` and |
| 69 | + `remove_pending_cluster_replicas_migration`. The field becomes vestigial |
| 70 | + (always false) once the staged machine is gone, but the migration is the |
| 71 | + only remaining cleaner for `pending` replicas stranded by a crash on a |
| 72 | + pre-deletion version, so both go together in a later release (see Part 3). |
| 73 | + |
| 74 | +## Solution Proposal |
| 75 | + |
| 76 | +Four parts. Parts 1 and 2 each just widen `controller_owns` (drop one |
| 77 | +conjunct each), Part 3 deletes everything that is dead once `controller_owns` |
| 78 | +is constant-true, and Part 4 makes the surface that reaches the one surviving |
| 79 | +direct path unconditional. 1 and 2 are separable and safe in either order, 3 |
| 80 | +needs both, 4 needs 3. |
| 81 | + |
| 82 | +### Part 1: remove `ENABLE_CLUSTER_CONTROLLER` and the legacy scheduler |
| 83 | + |
| 84 | +Delete the dyncfg (definition, registration, and its five read sites, plus the |
| 85 | +sqllogictest binary's force-on block) and everything only it kept alive: |
| 86 | + |
| 87 | +- `cluster_scheduling.rs` wholesale: `check_scheduling_policies`, |
| 88 | + `check_refresh_policy`, `handle_scheduling_decisions`, and the |
| 89 | + `SchedulingDecision` / `RefreshDecision` types. |
| 90 | +- The plumbing: `Message::CheckSchedulingPolicies` and |
| 91 | + `Message::SchedulingDecisions` with their handlers, the coordinator timer |
| 92 | + and select-loop tick, and the `cluster_scheduling_decisions` coordinator |
| 93 | + state. |
| 94 | +- The two metrics `check_scheduling_policies_seconds` and |
| 95 | + `handle_scheduling_decisions_seconds`. |
| 96 | +- The system var `cluster_check_scheduling_policies_interval` (its sole |
| 97 | + consumer is the timer). |
| 98 | +- `ReplicaCreateDropReason::ClusterScheduling`, constructed only by the |
| 99 | + scheduler. |
| 100 | + |
| 101 | +The persisted audit vocabulary stays: `SchedulingDecisionsWithReasonsV2` and |
| 102 | +the audit-log types are written by the controller's `OnRefresh` path too |
| 103 | +(`refresh_window_decision_to_audit_log`), and old events must remain |
| 104 | +decodable regardless. |
| 105 | + |
| 106 | +### Part 2: controller owns system clusters |
| 107 | + |
| 108 | +Drop the `is_user()` conjunct from `ManagedClusterIds` and from the two |
| 109 | +`controller_owns` computations in the sequencer. Runtime ALTERs of system |
| 110 | +clusters then flow through the controller like any other managed cluster: |
| 111 | +config-shape changes reshape into a durable reconfiguration record, factor |
| 112 | +changes update the config and the controller converges the replica set. |
| 113 | + |
| 114 | +Ownership and the boot migration compose rather than conflict, in both |
| 115 | +directions: |
| 116 | + |
| 117 | +- The controller matches replicas by shape and count, never by name, so the |
| 118 | + migration-created `r1..rN` replicas satisfy its baseline and a steady |
| 119 | + system cluster reconciles to no decisions. |
| 120 | +- The migration converges by canonical name, so a boot after a reshape (or |
| 121 | + with a reconfiguration in flight) renames or re-creates replicas the |
| 122 | + controller had materialized under generator names. That is harmless churn: |
| 123 | + every replica is a cold process at boot anyway, so the cost is replica-id |
| 124 | + and audit-log noise, and an in-flight record is durable, so the controller |
| 125 | + picks the reconfiguration back up on its first tick. |
| 126 | + |
| 127 | +Points to settle in the implementation, none of them blockers: |
| 128 | + |
| 129 | +- `validate_reconfiguration_resource_limits` early-returns for non-user |
| 130 | + clusters with the comment "a system cluster never reshapes into a record", |
| 131 | + which this part makes false. The early return itself stays, since system |
| 132 | + clusters are exempt from `max_replicas_per_cluster` and credit accounting |
| 133 | + everywhere else, but the comment must state the exemption instead. |
| 134 | +- Audit attribution: boot-time creates by the migration keep the `System` |
| 135 | + reason, runtime creates by the controller audit `Manual` (the tag for |
| 136 | + replicas the cluster's own config calls for), the same as user clusters. |
| 137 | +- The controller's create path validates sizes via the cluster owner's |
| 138 | + allowed sizes. Builtin clusters owned by `mz_system` bypass |
| 139 | + `allowed_cluster_replica_sizes` (`get_role_allowed_cluster_sizes`), so no |
| 140 | + new failure mode there. `mz_support` / `mz_analytics` are owned by their |
| 141 | + own roles and see the same restriction their owners' ALTERs already see. |
| 142 | + |
| 143 | +### Part 3: delete the staged machine and slim the direct path to an escape hatch |
| 144 | + |
| 145 | +With `controller_owns` constant-true, `NeedsFinalization::Yes` has no |
| 146 | +producer. Delete: |
| 147 | + |
| 148 | +- `ClusterStage::WaitForHydrated` and `ClusterStage::Finalize`, their structs, |
| 149 | + dispatch arms, and handlers. |
| 150 | +- `NeedsFinalization` and `PENDING_REPLICA_SUFFIX`. |
| 151 | +- The `pending_cluster_alters` connection state and its retire paths: |
| 152 | + `drop_reconfiguration_replicas`, |
| 153 | + `retire_cluster_reconfigurations_for_conn`, |
| 154 | + `cancel_cluster_reconfigurations_for_conn`, and their call sites in |
| 155 | + connection cleanup and cancellation. |
| 156 | +- The `AlterClusterWhilePendingReplicas` error (its raise site goes with the |
| 157 | + machine, its catch site went with the scheduler). |
| 158 | +- In `sequence_alter_cluster_managed_to_managed` (single caller left once the |
| 159 | + scheduler is gone): the pending-replica arm and the separate scale-up/down |
| 160 | + branches. Factor-only changes are config-only writes the controller |
| 161 | + converges, like user clusters today. |
| 162 | + |
| 163 | +The direct reshape branch is deliberately **kept**, as the synchronous |
| 164 | +cut-over path: converge the replica set onto the target and settle any carried |
| 165 | +reconfiguration record, all in one catalog transaction with no controller |
| 166 | +involvement. It is routed to by a zero-timeout commit strategy |
| 167 | +(`WITH (WAIT FOR '0s')`, or `WAIT UNTIL READY (TIMEOUT '0s', ON TIMEOUT |
| 168 | +'COMMIT')`) instead of by `AlterClusterPlanStrategy::None` as today. Two |
| 169 | +reasons: |
| 170 | + |
| 171 | +- **Escape hatch.** Once the gate is gone there is no break-glass flag, and |
| 172 | + every other reshape depends on the controller ticking and applying. The |
| 173 | + direct path is the one reshape that still works when the controller itself |
| 174 | + is the problem, and it simultaneously unsticks a wedged reconfiguration by |
| 175 | + retiring the record. Requesting it stays safe under a live controller: the |
| 176 | + config write invalidates any in-flight tick's compare-and-append witness, |
| 177 | + so a stale controller batch is rejected, the same as any user DDL landing |
| 178 | + mid-tick. |
| 179 | +- **Honest semantics.** A zero timeout with commit already means "cut over |
| 180 | + now, hydrated or not". Doing it synchronously in the ALTER instead of one |
| 181 | + controller tick later is the same outcome, minus the tick. |
| 182 | + |
| 183 | +"The same outcome" is a constraint, not an observation, so the cut-over does |
| 184 | +not get to improvise. It reuses the two things that define what a reshape |
| 185 | +means: |
| 186 | + |
| 187 | +- The target is folded onto an in-flight one exactly as the record path folds |
| 188 | + it (`alter_reconfiguration_target`, shared by both). Otherwise the same |
| 189 | + statement would mean different things depending on which path took it: an |
| 190 | + `ALTER` mentioning one dimension would silently revert the in-flight |
| 191 | + transition along every dimension it did not mention. |
| 192 | +- The replica set is converged with the controller's own reconcile kernel |
| 193 | + (`reconcile_replicas`, made public for this), against the cluster as |
| 194 | + `observe_cluster_state` sees it. So a replica that already carries the |
| 195 | + target shape is kept rather than bounced, which is what makes forcing a |
| 196 | + stuck-but-hydrating resize to commit cost nothing beyond the tick it saves. |
| 197 | + A blind whole-set recreate would instead cold-restart a replica that was |
| 198 | + already up, and would turn a cancel into a full bounce of a healthy set. |
| 199 | + |
| 200 | +Settling the carried record follows from where the cut-over landed |
| 201 | +(`retire_carried_reconfiguration`): on the record's own target it *is* the |
| 202 | +finalization the record was waiting for, so it settles `Finalized` marked |
| 203 | +`forced` (nothing waited for hydration); anywhere else the target was |
| 204 | +abandoned, which is a cancel. |
| 205 | + |
| 206 | +The branch keeps its supporting plumbing alive: the replica-id pre-allocation |
| 207 | +and the replica eval contexts for created replicas. It drops by observed id |
| 208 | +rather than by canonical name, so it works after controller name drift and |
| 209 | +with overlap replicas present. |
| 210 | + |
| 211 | +What stays, and why: |
| 212 | + |
| 213 | +- `AlterClusterPlanStrategy` and `ClusterStage::AwaitReconfiguration`: the |
| 214 | + controller reshape path and its foreground wait-shim consume them, as does |
| 215 | + `cluster_alter_check_ready_interval`. |
| 216 | +- `remove_pending_cluster_replicas_migration`: an upgrade can come from a |
| 217 | + version whose staged machine crashed between the pending-create commit and |
| 218 | + finalize. Those replicas are durably `pending: true`, excluded from the |
| 219 | + controller's ownership test, and after this deletion no runtime path would |
| 220 | + ever clean them, so the catalog-open migration is the only remaining |
| 221 | + cleaner. It is removed together with the durable `pending` field once no |
| 222 | + supported upgrade source can still write pending replicas. |
| 223 | +- `reconcile_builtin_cluster_replicas`: still load-bearing in the two windows |
| 224 | + the controller cannot cover. `Coordinator::bootstrap` brings up only |
| 225 | + replicas already durable and runs before the controller task is spawned, |
| 226 | + and the controller is inactive in 0dt read-only mode, where the migration |
| 227 | + (running against the savepoint catalog) is what gets system clusters up and |
| 228 | + hydrated before cutover. |
| 229 | + |
| 230 | +### Part 4: accept `WITH (WAIT ...)` unconditionally |
| 231 | + |
| 232 | +Part 3 leaves the synchronous cut-over reachable only through a |
| 233 | +`WITH (WAIT ...)` clause, and the planner gates any such clause on |
| 234 | +`enable_zero_downtime_cluster_reconfiguration`, default off. So on a deployment |
| 235 | +running the compiled default the escape hatch does not exist, which defeats the |
| 236 | +reason for keeping it. A break-glass path behind a default-off flag is not a |
| 237 | +break-glass path. |
| 238 | + |
| 239 | +Remove the feature flag and its planner gate. The two rejections that share |
| 240 | +that code path stay: a `WAIT` without a replica-shape change, and a `WAIT` on |
| 241 | +an unmanaged cluster. |
| 242 | + |
| 243 | +This makes the graceful-reconfiguration surface generally available, so the |
| 244 | +private-preview badges on `ALTER CLUSTER`'s reference page come off with it. |
| 245 | + |
| 246 | +`enable_cluster_schedule_refresh` is the near-identical sibling gate a dozen |
| 247 | +lines below, and stays at its current default. It gates a separate SQL surface |
| 248 | +under its own staged rollout (see Out of Scope). |
| 249 | + |
| 250 | +## Behavior changes |
| 251 | + |
| 252 | +- **System-cluster shape ALTERs become controller-driven.** |
| 253 | + `ALTER CLUSTER mz_system SET (SIZE ...)` changes from a synchronous |
| 254 | + whole-set recreation to a background graceful reconfiguration (default |
| 255 | + deadline 24h, `ROLLBACK` on timeout). This is strictly more capable: the |
| 256 | + graceful path is available when wanted (previously it did not exist for |
| 257 | + system clusters), and `WITH (WAIT FOR '0s')` requests an immediate |
| 258 | + cut-over for operators who cannot afford the overlap set, for example when |
| 259 | + resizing on a deployment with no headroom. |
| 260 | +- **Factor changes on system clusters converge asynchronously.** |
| 261 | + `ALTER CLUSTER mz_support SET (REPLICATION FACTOR 1)` updates the config |
| 262 | + and returns, the controller materializes the replica within a tick. Same |
| 263 | + as user clusters today. |
| 264 | +- **A zero-timeout commit `WAIT` becomes synchronous.** Today |
| 265 | + `WITH (WAIT FOR '0s')` writes a record the controller commits on its next |
| 266 | + tick. It now takes the direct path and returns with the cut-over already |
| 267 | + transacted. Same outcome, one tick sooner, and it works even when the |
| 268 | + controller does not. |
| 269 | +- **`WITH (WAIT ...)` is accepted everywhere.** Part 4 removes its feature |
| 270 | + gate, so the escape hatch is reachable without flipping a flag first. |
| 271 | +- **No more break-glass flag.** After Part 1, reverting to the legacy paths |
| 272 | + requires a binary rollback. The gate has been default-on since v26.29, so |
| 273 | + the controller paths have several releases of burn-in. The direct path is |
| 274 | + the remaining operational escape hatch for reshapes. |
| 275 | + |
| 276 | +## Test and rollout surface |
| 277 | + |
| 278 | +- `test/sqllogictest/system-cluster.slt`: the blocks asserting a synchronous |
| 279 | + replica count right after `ALTER CLUSTER mz_system SET (SIZE ...)` and |
| 280 | + after factor flips need rework, since convergence is now asynchronous and |
| 281 | + slt does not retry. Move them to testdrive (which retries) or assert on |
| 282 | + the config instead of the replica set. |
| 283 | +- `test/testdrive/cluster-controller.td`: the gate-off sections go (the |
| 284 | + unmanaged-conversion refusal block and the `cc_handoff*` legacy-handoff |
| 285 | + scenarios). The file's freeze-the-controller technique (flipping the gate |
| 286 | + off to hold an in-flight state still) is replaced by cranking |
| 287 | + `cluster_controller_tick_interval` up. |
| 288 | +- `test/pg-cdc/cluster-graceful-reconfiguration.td`: the explicit legacy |
| 289 | + foreground section goes, the controller section stays. |
| 290 | +- Zero-timeout `WAIT` tests (`cluster-controller.td` and friends) now |
| 291 | + exercise the direct path, so assertions that expect a reconfiguration |
| 292 | + record for the `'0s'` case need adjusting. |
| 293 | +- `test/sqllogictest/mz_cluster_schedules.slt`: the |
| 294 | + `cluster_check_scheduling_policies_interval` validation block goes with the |
| 295 | + var. |
| 296 | +- `test/cluster/resources/resource-limits.td` exercises factor flips on |
| 297 | + `mz_analytics` with retrying queries, so it survives the switch to |
| 298 | + asynchronous convergence and pins the system-cluster limit exemption. |
| 299 | +- Flag-name references: `misc/python/materialize/mzcompose/__init__.py` pins |
| 300 | + `enable_cluster_controller` per version for mixed-version runs and needs an |
| 301 | + upper version bound at the removal version, v26.38 (older binaries keep the |
| 302 | + flag, newer ones warn on the unknown default). |
| 303 | + `misc/python/materialize/parallel_workload/action.py` drops it from the |
| 304 | + do-not-flip list, and the launchdarkly-flag-consistency allowlist entries |
| 305 | + (listed twice) surface as prunable. |
| 306 | +- Part 4 needs the same treatment for |
| 307 | + `enable_zero_downtime_cluster_reconfiguration`, and it is the load-bearing |
| 308 | + case: the graceful-reconfiguration platform check issues its |
| 309 | + `WITH (WAIT ...)` in a manipulate phase that an upgrade scenario runs |
| 310 | + against the previous release, which still enforces the gate. Unlike the |
| 311 | + controller gate this flag is a real LaunchDarkly flag, so its |
| 312 | + flag-consistency entry moves to the stale list until it is archived there. |
| 313 | + |
| 314 | +## Alternatives |
| 315 | + |
| 316 | +- **Delete the direct path entirely and let `WAIT FOR '0s'` ride the |
| 317 | + controller record.** Maximal deletion. Rejected: once the gate is gone, |
| 318 | + every reshape would depend on the controller ticking and applying, so a |
| 319 | + controller bug would leave no way to reshape any cluster, and no flag to |
| 320 | + fall back on. The kept branch is small, exercised by an explicit statement, |
| 321 | + and doubles as the cleanup tool for a wedged reconfiguration. |
| 322 | +- **Keep the system-cluster exception, reject `WITH (WAIT ...)` on system |
| 323 | + clusters, delete only the scheduler and the staged machine.** This closes |
| 324 | + the staged machine's last door with a new error instead of ownership. |
| 325 | + Rejected: it must keep the full legacy direct create/drop path alive for |
| 326 | + system clusters forever, so the sequencer retains a second complete |
| 327 | + replica-materialization implementation, and it removes a currently-working |
| 328 | + statement while this design makes the same statement do the right thing. |
| 329 | + Ownership deletes more code and adds a capability instead of an error. |
| 330 | +- **Accept and ignore `WAIT` on system clusters.** Rejected: an instant |
| 331 | + success that waited for nothing is the same silent lie we fixed for |
| 332 | + unmanaged clusters (see the v26 release notes entry on `WAIT` being |
| 333 | + silently ignored). Moot under this design, where the `WAIT` is honored. |
0 commit comments