adapter: delete the staged cluster reconfiguration and direct replica paths

[aljoscha]

Motivation

The staged cluster-reconfiguration machine is unreachable. Managed cluster
shape changes return through the durable reconfiguration-record path, and
sequence_alter_cluster_managed_to_managed has controller_owns = true, so its
staged/direct branches have no live producer.

Controller-path correctness fixes are split into #38580, the preceding PR. This PR is
dead-code removal.

Description

Deletes:

• ClusterStage::WaitForHydrated and ClusterStage::Finalize
• NeedsFinalization and PENDING_REPLICA_SUFFIX
• pending overlap-replica creation, promotion, and drop logic
• per-connection pending ALTER state and cleanup paths
• the sequencer's managed-replica create/drop reconciliation branches
• dead Op::UpdateClusterReplicaConfig and error plumbing

The surviving managed-to-managed path is a config-only write. The controller
reconciles replication-factor changes and scheduled-cluster shape changes on its
next tick.

The durable pending field and catalog-open cleanup stay. An older binary may
have crashed after creating a pending replica, and an upgraded binary must still
reap that stranded state.

Net diff: +100/-806. Most additions are the small config-only
replacement for the deleted reconciliation function and comments documenting
upgrade cleanup.

Verification

• cargo clippy -p mz-adapter -p mz-cluster-controller --all-targets -- -D warnings
• cargo check -p mz-adapter -p mz-cluster-controller
• cargo fmt --all -- --check
• controller and ALTER coverage lives in the preceding fix PR because it tests
  the production path, not the deleted implementation

Integration suites are left to PR CI.

[aljoscha]

should do an exhaustive destructure here please

[aljoscha]

Done. Destructures ReconfigurationTarget so a new target dimension has to be applied here too.

[aljoscha]

prolly also want exhaustive destructure here

[aljoscha]

Done, and I did options as well as new_managed since it carries the same hazard, arguably the sharper one: a new ALTER option that names a shape dimension has to be either folded or explicitly ruled out.

The non-shape fields are spelled out with _ and a one-line reason rather than .., otherwise the destructure buys nothing. This matches alter_changes_replica_shape a few lines up, which already destructured PlanClusterOption exhaustively.

Verified it actually bites rather than just looking exhaustive: adding a probe_new_shape_dimension field to PlanClusterOption yields

error[E0027]: pattern does not mention field `probe_new_shape_dimension`
    --> src/adapter/src/coord/sequencer/inner/cluster.rs:2259:9

at the new site (and at alter_changes_replica_shape). Reverted the probe, of course.

[mtabebe]

Question on the change

[mtabebe]

One thing I am not quite understanding here...

My understanding was that the controller passes one contribution per strategy into as the desired state (including HydrationBurst). Since a burst replica is owned and has a different size, will reconiciliation drop the burst replica if the wait for is 0s?

If so, would that leave the burst record in place?

Was there silent guarding of this behaviour on the old code path before, with the full reconciliation?

I see some test changes as well in similar scenarios, but not quite sure ...

Either way flagging it.

[aljoscha]

You found a bug! This was somewhat working before, and even is now, but it's tricky:

• Before, even a wait = 0s, commit cutover would go through the controller, and it would do the correct thing.
• I decided to keep this non-controller path for wait = 0s, commit, as a failsafe, but it's complicating things slightly because it duplicates the decision logic, and it did it incorrectly 🙈

What would happen right now: we drop the burst replica, we keep the burst record. On the next controller tick the burst replica is re-created, but we have lost it's state, of course.

I'm slightly leaning towards keeping this reconfiguration path, and making sure it works correctly by re-using the same decision code. But I could probably be convinced to remove it, and let the controller handle the wait =0s, commit path as well. Wdyt?

[mtabebe]

:) Agreed on keeping it! I think if we can somehow refactor to maximize code reuse across the code path that is good as well.

[ggevay]

Heads up: #38139 (just merged) adds a "single-replica sources on a multi-replica cluster" notice with emission points inside sequence_alter_cluster_stage, right where this PR rewrites the routing, so the rebase will hit three small conflicts:

• a single_replica_sources_notice computation just after the no-op short-circuit, the same insertion point as the new cut_over_target declaration (the two are independent, keep both),
• on the reshape branch, the reshape_alter_cluster_managed return is wrapped in a result-capture that emits the notice on success (belongs in the non-immediate else arm here),
• two lines emitting the notice right after the sequence_alter_cluster_managed_to_managed(...).await? call (re-add after the reworked call, which then also covers the new immediate cut-over route).

One non-conflict to also pick up: the doc comment on notice_relevant_replica_count explains that reconfiguration overlap replicas are excluded under both mechanisms and mentions that the legacy graceful alter marks them pending. That clause becomes stale with this PR and can simply be dropped. The counting logic itself (in-flight target replication factor plus INTERNAL/BILLED AS replicas) needs no change.

[ggevay]

Two small comments

[ggevay]

The AlterClusterReplicationFactorWhileReconfiguring refusal still fires for the immediate cut-over, so the escape hatch can't change the factor while unsticking a wedged record (the new test's factor case has no record in flight). The refusal's rationale, silent clobber at the async cut-over, doesn't apply here: the cut-over retires the record in the same transaction, and nothing consumes a retired record's target. Consider exempting requests_immediate_cut_over, or noting the two-step workaround (cut over the shape first, then alter the factor) in the refusal's comment.

[aljoscha]

Good catch, fixed in #38174 (stacked on top, since #38112 is mid-stack and rewrites the neighbouring dimension comments).

Your reasoning checks out end to end: fold_reconfiguration_target already computes the right answer for a re-targeted factor, the cut-over retires the record in the same transaction, and every consumer of a record's target sits behind is_in_progress() — so there's no later cut-over left to clobber the write. Only the guard made that branch unreachable.

Went with exempting rather than documenting the workaround, because the two-step version makes the cluster run the old factor at the new size in between, which for a scale-down is the most expensive combination available, exactly while someone is firefighting.

Kept it narrow in one respect: a zero-timeout rollback still leaves a record in flight to settle on a tick, so it keeps the refusal. And you were right about why this held — the existing size-and-factor case runs with no record in flight, so the wedged case was simply unexercised. #38174 adds it, plus an assertion that the refusal still fires without a WAIT.

The wider question your comment raises — should this be allowed generally, not just on the cut-over — I've kept out of that PR. Worth noting the surface is further along than it looks: the factor is already re-targetable at record creation, and mz_cluster_reconfigurations.changes and the SHOW CLUSTERS summary already render replication factor to N. So only re-targeting an existing record is refused. But it's a user-visible semantics change on a surface about to go GA, so it's getting tracked separately.

[aljoscha]

Filed the general-semantics question as SQL-626, in the Cluster Autoscaling on hydration project alongside the rest of this work, related to SQL-342. It writes up the three designs (fold into the record / apply immediately and sync the target / drop the factor from ReconfigurationTarget entirely) and what each costs the user, including the combined-statement overlap-peak change, which is billing-visible.

[ggevay]

nit: this was the last producer of Op::UpdateClusterReplicaConfig, so the op variant, its transact.rs apply arm, and the ddl.rs audit match arm can go with the machine too (here or in a follow-up).

[aljoscha]

Agreed, done in #38174. Checked there are no producers left anywhere in src/ or test/ — the variant, the transact.rs apply arm and the ddl.rs audit match arm all went.

[def-]

How do we plan to handle the scenario where the new small replica never finishes hydrating? It would be nice if we could handle that gracefully. We have a temporary quick-hydration replica that stays around during that scenario, but if you run ALTER CLUSTER ... SET (SIZE '...') WITH (WAIT FOR '0s') the temp replica also gets destroyed, and then immediately recreates the temp replica again, destroying its state. Small test:

diff --git a/test/testdrive/cluster-controller.td b/test/testdrive/cluster-controller.td
index e2d0f236b5..afab201939 100644
--- a/test/testdrive/cluster-controller.td
+++ b/test/testdrive/cluster-controller.td
@@ -1449,6 +1449,78 @@ scale=1,workers=2 2
 > DROP CLUSTER cc_cutover CASCADE
 > DROP TABLE cc_cutover_t

+# ----- Direct cut-over: a running hydration burst survives -----
+#
+# The cut-over converges the replica set with the same reconcile kernel a tick
+# does, and that kernel unions *every* strategy's contribution, so a replica
+# survives iff some strategy still desires its shape. The cut-over supersedes
+# the reconfiguration record but not the burst record beside it, which the
+# reshape leaves in place and warranted, so the burst's contribution belongs in
+# that union too. Drop it and the same reshape churns the replica set
+# differently depending on which path ran it: the next tick puts the burst
+# replica back under a fresh id, discarding its hydration progress.
+#
+# Same wedge as above: a sleeping view pins hydration on every replica, which
+# both arms the burst and keeps the reconfiguration from cutting over by itself.
+
+$ postgres-execute connection=postgres://mz_system@${testdrive.materialize-internal-sql-addr}/materialize
+ALTER SYSTEM SET enable_auto_scaling_strategy = true
+ALTER SYSTEM SET enable_hydration_burst = true
+
+> CREATE CLUSTER cc_cutover_burst (SIZE 'scale=1,workers=1', REPLICATION FACTOR 1, AUTO SCALING STRATEGY = (ON HYDRATION (HYDRATION SIZE = 'scale=2,workers=2', LINGER DURATION = '600s')))
+
+> CREATE TABLE cc_cutover_burst_t (id int)
+> INSERT INTO cc_cutover_burst_t VALUES (1)
+
+> CREATE MATERIALIZED VIEW cc_cutover_burst_slow IN CLUSTER cc_cutover_burst AS
+  SELECT mz_unsafe.mz_sleep(id * 3600) AS s FROM cc_cutover_burst_t
+
+> SELECT r.size, count(*) FROM mz_cluster_replicas r JOIN mz_clusters c ON r.cluster_id = c.id WHERE c.name = 'cc_cutover_burst' GROUP BY r.size ORDER BY r.size
+scale=1,workers=1 1
+scale=2,workers=2 1
+
+$ set-from-sql var=cc_cutover_burst_replica_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_burst' AND r.size = 'scale=2,workers=2'
+
+# Control arm: the record path reshapes around the burst, whose replica keeps
+# running beside the new overlap replica.
+> ALTER CLUSTER cc_cutover_burst SET (SIZE 'scale=1,workers=2')
+
+> SELECT r.size, r.id::text = '${cc_cutover_burst_replica_id}' FROM mz_cluster_replicas r JOIN mz_clusters c ON r.cluster_id = c.id WHERE c.name = 'cc_cutover_burst' ORDER BY r.size
+scale=1,workers=1 false
+scale=1,workers=2 false
+scale=2,workers=2 true
+
+# The reshape is wedged (the target can never hydrate), so the operator forces
+# it with the escape hatch, re-stating the in-flight target exactly as the
+# cc_cutover section above does. The overlap replica is kept because it already
+# has the target shape, the baseline is retired, and the burst replica is
+# neither, so it must be left exactly as it is: same id, still running.
+> ALTER CLUSTER cc_cutover_burst SET (SIZE 'scale=1,workers=2') WITH (WAIT FOR '0s')
+
+> SELECT r.size, r.id::text = '${cc_cutover_burst_replica_id}' FROM mz_cluster_replicas r JOIN mz_clusters c ON r.cluster_id = c.id WHERE c.name = 'cc_cutover_burst' ORDER BY r.size
+scale=1,workers=2 false
+scale=2,workers=2 true
+
+# Liveness synchronizer: a reconfiguration elsewhere proves the controller ran
+# many ticks since the cut-over, so the count below is a settled state and not
+# one the controller has yet to churn.
+> CREATE CLUSTER cc_cutover_burst_tick (SIZE 'scale=1,workers=1', REPLICATION FACTOR 1)
+> ALTER CLUSTER cc_cutover_burst_tick SET (REPLICATION FACTOR 2)
+> SELECT count(*) FROM mz_cluster_replicas r JOIN mz_clusters c ON r.cluster_id = c.id WHERE c.name = 'cc_cutover_burst_tick'
+2
+> DROP CLUSTER cc_cutover_burst_tick
+
+# One burst replica was created across both reshapes, so neither bounced it.
+> SELECT count(*) FROM mz_catalog.mz_audit_events WHERE object_type = 'cluster-replica' AND details->>'cluster_name' = 'cc_cutover_burst' AND details->>'reason' = 'hydration-burst'
+1
+
+> DROP CLUSTER cc_cutover_burst CASCADE
+> DROP TABLE cc_cutover_burst_t
+
 $ postgres-execute connection=postgres://mz_system@${testdrive.materialize-internal-sql-addr}/materialize
 ALTER SYSTEM SET unsafe_enable_unstable_dependencies = false

currently fails:

cluster-controller.td:1504:1: error: non-matching rows: expected:
[["scale=1,workers=2", "false"], ["scale=2,workers=2", "true"]]
got:
[["scale=1,workers=2", "false"], ["scale=2,workers=2", "false"]]
Poor diff:
+ scale=2,workers=2 false
- scale=2,workers=2 true

[aljoscha]

Confirmed, and thanks for the test. It reproduces exactly, and the id flip in your output is the tell: the burst replica is dropped and recreated rather than kept.

The mechanism is that the cut-over hand-builds its desired set as replication_factor slots at the new target shape with reason Baseline, then hands that single contribution to reconcile_replicas. That kernel's contract is that an owned replica survives iff some contribution desires its shape, so the burst replica's shape (burst_size) is unmatched and gets dropped. The burst record survives, because a size change doesn't un-warrant it (burst_record_warranted keys on rf != 0 and hydration_size == burst_size), so the next tick sees a record with no replica and makes one. Cold, under a fresh id, and steady_hydrated_at persists so the linger clock doesn't restart either.

It is a regression from this PR specifically. Before it, a zero-timeout commit wrote a record and let the controller converge, and a tick unions all four strategies, so the burst kept its replica. It only becomes reachable once #38104 removes the enable_zero_downtime_cluster_reconfiguration gate, because the cut-over needs a WAIT clause to route to it. That is why the fix belongs here rather than in a follow-up.

@mtabebe spotted the same thing further up this PR. Your test is better than what I had sketched, in particular the audit-event count, which catches the bounce even if the id assertion somehow passes. Taking it as the regression test.

On handling it gracefully, agreed that is the bar. The repair is to stop hand-building the contribution and run the real strategy set, since HydrationBurstStrategy::desired_replicas is pure over state and would keep the replica for free. The wrinkle is that two of the four strategies read live signals, and both of those signals are deliberately barred from the coordinator loop: hydration waits on the compute instance task, and the refresh window does an oracle round-trip. The synchronous cut-over runs on that loop. So sharing the decision means dealing with the signal dependency rather than assuming it away, and I am settling that before writing it.

[ggevay]

On the signal dependency: it may dissolve if the cut-over runs only phase 2. In any post-write state the cut-over can produce, desired_replicas across the full strategy set reads no live signal: the graceful strategy returns empty because the same transaction retired the record, the burst strategy never reads its signals argument (hydration only feeds its update_state), and the refresh window is only read for scheduled clusters, where the carried-record edge settles on the next tick either way, as today.

Phase 1 the cut-over has no need to run: the writes it would produce (burst arm/teardown, on-refresh normalization) land on the following ticks, same as after a plain config write. And since the burst shape is the record's burst_size plus the cluster's other dimensions, evaluating it over the post-write state bounces the burst replica exactly when a tick after a plain write would.

One wrinkle: "phase 2 reads no signals in these states" is a per-strategy fact, not a trait contract, and signal_request can't check it because it is phase-agnostic (the burst strategy legitimately requests hydration for update_state even post-cut-over). Making it checkable needs the declaration split by phase, so the cut-over can assert that the phase-2 request set is empty. And it is a real commitment: if some planned strategy needs a live signal in desired_replicas itself, then no synchronous construction of the desired set works, hand-built ones included, and the cut-over needs a different answer entirely. The burst strategy's shape (signals feed phase 1, phase 1 writes a durable record, phase 2 is pure over it) is the idiom that keeps the escape hatch well-defined.

[aljoscha]

Yes, and it holds for a structural reason rather than by timing.

The baseline yields on record.is_in_progress() && now >= deadline && Commit.
The graceful strategy contributes target.replication_factor replicas whenever
the record is in progress, and its only early return is the Rollback timeout
branch, which the Commit conjunct excludes. So every state in which the
baseline gives up its replicas is a state in which the target set takes their
place. The union is never short, and the moment the record leaves in-progress
both flip back together: the baseline resumes at the realized shape, which the
cut-over has already advanced to the target, so the replicas the swap created
are exactly the ones it desires. Nothing to suppress and nothing to churn.

Two failure modes worth naming, since "on time" is the right thing to worry
about:

• If the cut-over write is rejected, the record stays in progress and the
  baseline keeps yielding. That is fine, the target replicas already exist and
  stay desired. The rejection can only come from CheckClusterState, so the
  same concurrent change also invalidates the shed's witness. There is no
  interleaving that retires the record without advancing the realized config
  while the swapped-in replicas are live.
• If a create cannot be built, the whole batch is skipped, drops included. The
  realized replicas are never retired without the target's being created in the
  same transaction.

Backed by three tests rather than by argument. forced_cutover_yield_is_covered _by_the_target_contribution walks the status, on-timeout and deadline matrix
and asserts that whenever the baseline yields, the graceful contribution is the
full target set, with a counter so the matrix cannot pass by never reaching the
window. The swap test now runs a third tick and asserts no churn after the
cut-over. And forced_cutover_swap_preserves_the_burst_replica pins that only
the baseline yields: an in-flight burst keeps its replica, and its id, across
the swap.

[aljoscha]

Rebuilt and restacked this on current main.

The controller-path fixes discussed in the recent review threads are now #38580.
They affect code already live in production and can merge independently. This PR
is now only the unreachable legacy deletion: the staged stages, pending replicas,
per-connection cleanup, sequencer reconciliation, and dead op/error plumbing.
Its current diff is +100/-806, with the durable pending field and catalog-open
upgrade cleanup intentionally retained.

The GitHub stack is now #38580 -> #38103 -> #38104 -> #38446 (reference only).