storage/adapter: fix graceful cluster reconfiguration for clusters with single-replica sources by aljoscha · Pull Request #37740 · MaterializeInc/materialize

aljoscha · 2026-07-20T07:34:03Z

Motivation

ALTER CLUSTER ... WITH (WAIT UNTIL READY ...) never completes on a cluster
hosting a single-replica source (Postgres, MySQL, SQL Server). The readiness
check gates cut-over on every ingestion reporting hydrated on the pending
replicas, but a single-replica source stays scheduled on the replica it
already runs on and is never placed on a pending replica until cut-over drops
the old one. It can never report hydrated there, so the reconfiguration
deadlocks until its deadline and, with ON TIMEOUT ROLLBACK, reverts without
ever resizing.

Fixes SQL-530

Description

Two commits that together make the readiness check sound:

storage-controller: don't wait for off-target ingestions in hydration
checks. collections_hydrated_on_replicas now skips ingestions whose
scheduled replica set is disjoint from the target replicas: they cannot
hydrate there, so they must not count against the target's hydration.
Multi-replica ingestions get scheduled onto pending replicas too and keep the
existing hydrated-on-target requirement. This also fixes
Instance::get_active_replicas_for_object, which only resolved
ingestion-export ids and fell back to "all replicas" otherwise: a new-syntax
source's primary collection id is not an ingestion export, so the skip would
never have fired for it. The helper now resolves primaries and exports
directly, like its siblings active_replicas and is_active_replica.

adapter: gate reconfiguration cut-over on pending replicas being online.
With off-target ingestions skipped, a cluster hosting only single-replica
sources reports hydrated vacuously the moment its pending replicas are
created, so cut-over could hand the sources to replicas whose processes never
came up. Both readiness paths, the legacy foreground path and the
controller-owned path, now additionally require every pending replica to be
online per the coordinator's replica status tracking (the signal behind
mz_cluster_replica_statuses). Replica health deliberately stays a caller
concern, composed in the adapter, outside the storage controller's hydration
check.

Non-obvious behavior notes:

With more than one pending replica the legacy path is deliberately
tightened: previously cut-over could fire once collections hydrated on one
healthy pending replica while another never came up. Requiring the full
pending set online matches the controller-owned path, which already
requires replication-factor-many hydrated target replicas.
The hydration-burst strategy's steady checks share the hydrated-replicas
signal, so an orchestrator readiness flap on a steady replica can arm a
burst one tick early, but never skips a needed one.
Sinks are unaffected: the hydration check already treats exports as always
hydrated.

Verification

New regression test test/pg-cdc/cluster-graceful-reconfiguration.td: a
Postgres source on a cluster undergoing graceful reconfiguration, exercising
both the legacy foreground path and the controller-owned background path by
pinning the path-selection flags each way, and asserting that the resize
completes and data keeps flowing across cut-over. Both sections deadlock
without the fix.

aljoscha · 2026-07-20T07:38:46Z

+            )
            .map_err(|e| AdapterError::internal("Failed to check hydration", e))?;

+        // Hydration alone does not prove the pending replicas are fit for


need to considerably shorten this comment to the essentials, don't comment on distant code

Cut to the essential reason (vacuous hydration off the pending replicas).

aljoscha · 2026-07-20T07:40:25Z


        let mut checks = Vec::new();
        for replica_id in replicas {
+            // Hydration alone does not prove a replica is fit for cut-over: it


same comment as above, need to boil this down considerably

Boiled down to the vacuous-hydration reason.

aljoscha · 2026-07-20T07:40:51Z

-    /// Of `replicas` on `cluster`, which have *all* current (non-transient)
-    /// collections on the cluster hydrated. The returned set is a subset of
-    /// `replicas`.
+    /// Of `replicas` on `cluster`, which are online (all processes ready per


cut the new parenthetical here

Dropped the parenthetical.

aljoscha · 2026-07-20T07:41:17Z

 pub struct LiveSignals {
-    /// The replicas observed this tick to have *all* current collections on the
-    /// cluster hydrated.
+    /// The replicas observed this tick to be online (all processes ready per


drop the parenthetical and also that bit about flapping

Dropped the parenthetical and the flap note.

aljoscha · 2026-07-20T07:41:38Z

+    /// Collections that are not scheduled on any of the provided replicas do
+    /// not count against hydration: a single-replica source keeps running on
+    /// its current replica and can never hydrate on a replica it is not
+    /// scheduled on. Callers deciding whether a replica set is fit for


cut the bit about callers, not our concern

Cut the callers bit.

aljoscha · 2026-07-20T07:42:04Z

                }
            }
+        } else if let Some(ingestion) = self.active_ingestions.get(id) {
+            // A primary ingestion id is not necessarily an ingestion export:


make this more succinct, local, please

Made it succinct and local.

aljoscha · 2026-07-20T07:42:44Z

                    match &target_replicas {
-                        Some(target_replicas) => !state.hydrated_on.is_disjoint(target_replicas),
+                        Some(target_replicas) => {
+                            // An ingestion that is not scheduled on any of the


need to boil down this comment considerably, don't we already document this on the method signature, if yes, focus on subtle parts here, if there are even any

Boiled down; leans on the trait doc now, keeps only the subtle fail-open note.

aljoscha · 2026-07-20T07:43:41Z

+# the Business Source License, use of this software will be governed
+# by the Apache License, Version 2.0.
+
+# Regression test for SQL-530: a graceful cluster reconfiguration (WAIT UNTIL


make sure the comments in here are nice and succinct

Tightened the header and the section comments throughout.

… checks `collections_hydrated_on_replicas` gated a graceful cluster reconfiguration's cut-over on every ingestion reporting hydrated on the new target replicas. A single-replica source (Postgres, MySQL, SQL Server) stays scheduled on the replica it already runs on and is never placed on a freshly added target replica until cut-over drops the old one, so it can never report hydrated there. WAIT UNTIL READY therefore deadlocked until its deadline on any cluster hosting such a source. Skip ingestions whose scheduled replica set is disjoint from the target replicas: they cannot hydrate there, so they must not count against the target's hydration. Multi-replica ingestions are scheduled onto target replicas too and keep the existing hydrated-on-target requirement. Whether the target is otherwise fit for cut-over (are its replicas even up?) is the caller's concern, addressed separately in the readiness composition. The scheduled set comes from `Instance::get_active_replicas_for_object`, which only resolved ingestion-export ids and fell back to "all replicas" otherwise. A new-syntax source's primary collection id is not an ingestion export (its `source_exports` holds only the tables), so the skip would never have fired for it. Give the helper the direct `active_ingestions` and `active_exports` resolution its siblings `active_replicas` and `is_active_replica` already have. Its existing drop-path callers are unaffected in practice: by the time they run, the instance has already forgotten the object and they take the unchanged all-replicas fallback. Sinks are unaffected by the check change: it already treats exports as always hydrated. Both the legacy foreground reconfiguration path and the controller-owned path share this check, so both are fixed, and the regression test exercises both by pinning the path-selection flags each way. The hydration-burst strategy's steady checks also share the check and now likewise ignore off-target single-replica sources, consistent with the contract. Fixes SQL-530.

aljoscha · 2026-07-20T08:06:23Z

+            )
            .map_err(|e| AdapterError::internal("Failed to check hydration", e))?;

+        // Collections scheduled off the pending replicas (a cluster of only


please just say that we also wait for the replica to be online, in case there are no objects that need hydration on it

Done, reworded both gate comments to say we also wait for the replica to be online in case it has no objects needing hydration on it.

The graceful-reconfiguration readiness check composes compute and storage hydration, but hydration alone does not prove the pending replicas are fit for cut-over: collections scheduled off the pending replicas report hydrated vacuously. A cluster hosting only single-replica sources (e.g. Postgres) reports ready the moment its pending replicas are created, so cut-over could hand the sources to a replica whose processes never came up. Additionally require every pending replica's processes to be online, per the coordinator's replica status tracking (the same signal that feeds mz_cluster_replica_statuses). Online is the orchestrator's process-readiness signal, it does not prove the replica's controller connections are established, but it closes the vacuous-readiness hole and makes cut-over overlap the new replica's boot. For a single pending replica that must hydrate collections this changes nothing, hydration there already implies the replica is up. With more than one pending replica it is a deliberate tightening of the legacy path: previously cut-over could fire once collections hydrated on one healthy pending replica while another never came up. Requiring the full pending set online matches the controller-owned path, which already requires replication-factor-many hydrated target replicas. On the controller-owned path the gate sits in the shared hydration-check assembly, so every strategy's hydrated-replicas signal now also requires online, including the hydration-burst strategy's steady checks. An orchestrator readiness flap on a hydrated steady replica reads as not steady-hydrated for a tick, which can arm a burst early but never skips a needed one. Replica health stays a caller concern, deliberately outside the storage controller's hydration check. Part of SQL-530.

ggevay

LGTM, but I didn't check the S&S part too deeply.

And a test suggestion from Claude:

coverage for the multi-replica-source direction

The new cluster-graceful-reconfiguration.td nails the single-replica case (skip fires → no deadlock). The other side of the new skip — a multi-replica source (Kafka/loadgen), where scheduled_on overlaps the target so cut-over must still wait for it to hydrate — doesn't seem to be behaviorally asserted anywhere:

zdtaltertest in test_managed_cluster.py does run a new-syntax Kafka source through the readiness loop, but with WAIT FOR '5s' (i.e. ON TIMEOUT COMMIT), so it cuts over at the deadline regardless of hydration. Good code coverage, but the assertions pass whether or not the skip logic is right.
slow_hydration asserts a hydration-gated rollback, but on a compute view, not a source.

Cheap way to get a positive test of the non-skip branch: switch zdtaltertest's WAIT FOR '5s' to WAIT UNTIL READY (TIMEOUT '60s', ON TIMEOUT ROLLBACK). The existing "settles to r2 / size advanced" assertions then genuinely depend on the Kafka source hydrating on the target before cut-over. Totally optional — the branch looks correct by construction (multi-replica ingestions schedule onto all replicas incl. pending, so scheduled_on is never disjoint from the target), so this is about guarding future regressions, not a bug.

aljoscha · 2026-07-20T11:02:26Z

LGTM, but I didn't check the S&S part too deeply.

And a test suggestion from Claude:

coverage for the multi-replica-source direction

The new cluster-graceful-reconfiguration.td nails the single-replica case (skip fires → no deadlock). The other side of the new skip — a multi-replica source (Kafka/loadgen), where scheduled_on overlaps the target so cut-over must still wait for it to hydrate — doesn't seem to be behaviorally asserted anywhere:

zdtaltertest in test_managed_cluster.py does run a new-syntax Kafka source through the readiness loop, but with WAIT FOR '5s' (i.e. ON TIMEOUT COMMIT), so it cuts over at the deadline regardless of hydration. Good code coverage, but the assertions pass whether or not the skip logic is right.

slow_hydration asserts a hydration-gated rollback, but on a compute view, not a source.

Cheap way to get a positive test of the non-skip branch: switch zdtaltertest's WAIT FOR '5s' to WAIT UNTIL READY (TIMEOUT '60s', ON TIMEOUT ROLLBACK). The existing "settles to r2 / size advanced" assertions then genuinely depend on the Kafka source hydrating on the target before cut-over. Totally optional — the branch looks correct by construction (multi-replica ingestions schedule onto all replicas incl. pending, so scheduled_on is never disjoint from the target), so this is about guarding future regressions, not a bug.

@def- do we want to change the existing tests here, or add something new for more coverage?

martykulma

Thanks @aljoscha! We seem to have the same decision branching in 3 places now. Not sure if there's ever a reason for them to drift, so might be worth having a single function.

Not blocking, and certainly can be handled in this is subsequent PR.

🐟

martykulma · 2026-07-20T13:04:48Z

+            // the primary ingestion id is not in `ingestion_exports`.
+            ingestion.active_replicas.clone()
+        } else if let Some(export) = self.active_exports.get(id) {
+            export.active_replicas.clone()


with this change, it looks like we have the same branching in multiple places: fn active_replicas(), fn is_active_replica(), and fn get_active_replicas_for_object().

Presumably, these should have always shared the same definition (i.e. the changes in instance.rs are actually fixing the drift)?

👋 aj the agent 🤖 here (working on Aljoscha's branch). You're right on both counts: get_active_replicas_for_object was missing the active_ingestions/active_exports arms, so this PR fixes exactly the drift you spotted. I'm consolidating the shared branching into a single resolver now, as a separate commit on this PR. Will ping when it's up.

Consolidation is up in commit 6cf9d4d ("storage-controller: share active-replica resolution across accessors"). Extracted a single active_replica_ids resolver returning an ActiveReplicas { Scheduled(&set), All } enum, and the three accessors now just project from it. No behavior change. Ran it through review + full CI. 🤖

`active_replicas`, `is_active_replica`, and `get_active_replicas_for_object` each carried the same four-way resolution of a `GlobalId` to the replicas running it: ingestion export, primary ingestion, export, or (for objects without per-replica scheduling) all replicas. The three had already drifted once. `get_active_replicas_for_object` was missing the primary-ingestion and export arms, so a source's primary collection resolved to all replicas instead of its single scheduled replica. Extract the resolution into one `active_replica_ids` helper returning an `ActiveReplicas` enum, and project the three accessors from it. This removes the duplication that let them drift and leaves a single place to change per-replica scheduling. No behavior change. `active_replicas` takes an owned copy of the scheduled set so the shared borrow ends before it borrows `self.replicas` mutably. The set holds one entry per replica and this runs once per command, not per row. Part of SQL-530.

ALTER CLUSTER ... WITH (WAIT UNTIL READY) deadlocking on clusters that host a single-replica source (SQL-530, fixed in MaterializeInc#37740) shipped because no test combined a readiness-gated reconfiguration with a Postgres, MySQL, or SQL Server source. Close the gap from three sides: * parallel-workload: a new ReconfigureClusterAction gracefully resizes a random managed cluster with ON TIMEOUT ROLLBACK and fails the run if the realized config never cuts over, so a wedged readiness check surfaces as a test failure instead of a silent rollback. Sources land on random clusters, so resizes regularly hit clusters hosting Postgres sources. * platform-checks: a new AlterClusterGracefulReconfiguration check reconfigures a cluster hosting a Postgres source in both manipulate phases and validates that the realized config, the finalized reconfiguration record, and ingestion survive restarts and upgrades. Gated on base version v26.35.0-dev: the fix predates the v26.35.0-rc.1 cut, and older versions wedge on the reconfiguration itself, so no phase may run there. * cloudtest: test_zero_downtime_reconfiguration now resizes a cluster hosting a Postgres source and asserts the reconfiguration finalizes and ingestion continues across cut-over, covering the k8s path where target replica pods must come online before cut-over. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

aljoscha requested review from a team as code owners July 20, 2026 07:34

aljoscha added the ci-nightly PR CI control: also trigger Nightly label Jul 20, 2026

aljoscha commented Jul 20, 2026

View reviewed changes

aljoscha force-pushed the adapter-graceful-reconfig-source-fix branch from 68315dd to 80420e4 Compare July 20, 2026 08:01

aljoscha commented Jul 20, 2026

View reviewed changes

aljoscha force-pushed the adapter-graceful-reconfig-source-fix branch from 80420e4 to 047af37 Compare July 20, 2026 08:08

ggevay reviewed Jul 20, 2026

View reviewed changes

ggevay approved these changes Jul 20, 2026

View reviewed changes

martykulma approved these changes Jul 20, 2026

View reviewed changes

aljoscha merged commit bbe8b69 into MaterializeInc:main Jul 20, 2026
370 checks passed

aljoscha deleted the adapter-graceful-reconfig-source-fix branch July 20, 2026 16:12

def- mentioned this pull request Jul 24, 2026

test: cover graceful cluster reconfiguration with single-replica sources #37854

Open

Uh oh!

Conversation

aljoscha commented Jul 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Description

Verification

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ggevay left a comment

Choose a reason for hiding this comment

Uh oh!

aljoscha commented Jul 20, 2026

Uh oh!

martykulma left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

aljoscha commented Jul 20, 2026 •

edited

Loading