Skip to content

Commit 385e39c

Browse files
committed
adapter: delete the staged cluster reconfiguration machine
With the controller owning every managed cluster's replica set, `NeedsFinalization::Yes` has no producer. Delete the machine it drove: the `WaitForHydrated` and `Finalize` stages, the `-pending` overlap replicas, the `pending_cluster_alters` connection state and its retire paths, and the `AlterClusterWhilePendingReplicas` error. The direct reshape path is deliberately kept, as the synchronous cut-over. It is now routed to by an explicitly zero-timeout commit strategy (`WITH (WAIT FOR '0s')`, or `WAIT UNTIL READY (TIMEOUT '0s', ON TIMEOUT 'COMMIT')`) rather than by the absence of a `WAIT` clause. Two reasons. It is the escape hatch: every other reshape depends on the controller ticking and applying, and this is the one that still works when the controller itself is the problem, while also unsticking a wedged reconfiguration by retiring its record. And the semantics are honest: a zero timeout with commit already means "cut over now, hydrated or not", so doing it synchronously in the ALTER is the same outcome minus a tick. "The same outcome" has to be true, so the cut-over does not improvise. It folds its target onto an in-flight one exactly as the reshape path does, and it converges the replica set with the controller's own reconcile kernel, so a replica that already has the target shape is kept rather than bounced. Forcing a stuck-but-hydrating resize to commit therefore keeps the replica that was already up, and lands the record on `finalized` (forced) rather than `cancelled`, since the cut-over reached the record's own target. Creating a replica from the controller's `ReplicaShape` also drops a lossy round-trip through the planner's `ComputeReplicaConfig`, which cannot represent `INTROSPECTION DEBUGGING` without an interval. A cluster in that state used to get a replica whose logging disagreed with the config that called for it.
1 parent 45f74e0 commit 385e39c

14 files changed

Lines changed: 531 additions & 871 deletions

File tree

src/adapter/src/catalog/open.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1430,8 +1430,14 @@ fn remove_invalid_config_param_role_defaults_migration(
14301430
Ok(())
14311431
}
14321432

1433-
/// Cluster Replicas may be created ephemerally during an alter statement, these replicas
1434-
/// are marked as pending and should be cleaned up on catalog open.
1433+
/// Drops replicas left durably marked `pending`.
1434+
///
1435+
/// No runtime path creates one anymore. An upgrade can still come from a version
1436+
/// whose staged reconfiguration machine crashed between the pending-create commit
1437+
/// and the finalize, and those replicas are excluded from the cluster
1438+
/// controller's ownership test, so this catalog-open sweep is their only
1439+
/// remaining cleaner. It goes away together with the durable `pending` field,
1440+
/// once no supported upgrade source can still write one.
14351441
fn remove_pending_cluster_replicas_migration(
14361442
tx: &mut Transaction,
14371443
boot_ts: mz_repr::Timestamp,

src/adapter/src/coord.rs

Lines changed: 3 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -104,8 +104,8 @@ use mz_catalog::config::{AwsPrincipalContext, BuiltinItemMigrationConfig, Cluste
104104
use mz_catalog::durable::OpenableDurableCatalogState;
105105
use mz_catalog::expr_cache::{GlobalExpressions, LocalExpressions};
106106
use mz_catalog::memory::objects::{
107-
CatalogEntry, CatalogItem, ClusterReplicaProcessStatus, ClusterVariantManaged, Connection,
108-
DataSourceDesc, ReconfigurationTarget, Table, TableDataSource,
107+
CatalogEntry, CatalogItem, ClusterReplicaProcessStatus, Connection, DataSourceDesc,
108+
ReconfigurationTarget, Table, TableDataSource,
109109
};
110110
use mz_cloud_resources::{CloudResourceController, VpcEndpointConfig, VpcEndpointEvent};
111111
use mz_compute_client::as_of_selection;
@@ -153,7 +153,7 @@ use mz_sql::names::{QualifiedItemName, ResolvedIds, SchemaSpecifier};
153153
use mz_sql::optimizer_metrics::OptimizerMetrics;
154154
use mz_sql::plan::{
155155
self, AlterSinkPlan, ConnectionDetails, CreateConnectionPlan, HirRelationExpr,
156-
NetworkPolicyRule, OnTimeoutAction, Params, QueryWhen,
156+
NetworkPolicyRule, Params, QueryWhen,
157157
};
158158
use mz_sql::session::user::User;
159159
use mz_sql::session::vars::{MAX_CREDIT_CONSUMPTION_RATE, SystemVars, Var};
@@ -875,8 +875,6 @@ pub struct ExplainTimestampFinish {
875875
#[derive(Debug)]
876876
pub enum ClusterStage {
877877
Alter(AlterCluster),
878-
WaitForHydrated(AlterClusterWaitForHydrated),
879-
Finalize(AlterClusterFinalize),
880878
/// The foreground wait-shim over a controller-driven background
881879
/// reconfiguration: poll the durable `reconfiguration` record until it
882880
/// clears, then report success or timeout depending on whether the realized
@@ -890,24 +888,6 @@ pub struct AlterCluster {
890888
plan: plan::AlterClusterPlan,
891889
}
892890

893-
#[derive(Debug)]
894-
pub struct AlterClusterWaitForHydrated {
895-
validity: PlanValidity,
896-
plan: plan::AlterClusterPlan,
897-
new_config: ClusterVariantManaged,
898-
workload_class: Option<String>,
899-
timeout_time: Instant,
900-
on_timeout: OnTimeoutAction,
901-
}
902-
903-
#[derive(Debug)]
904-
pub struct AlterClusterFinalize {
905-
validity: PlanValidity,
906-
plan: plan::AlterClusterPlan,
907-
new_config: ClusterVariantManaged,
908-
workload_class: Option<String>,
909-
}
910-
911891
#[derive(Debug)]
912892
pub struct AlterClusterAwaitReconfiguration {
913893
validity: PlanValidity,
@@ -1312,10 +1292,6 @@ pub struct ConnMeta {
13121292
#[serde(skip)]
13131293
deferred_lock: Option<OwnedMutexGuard<()>>,
13141294

1315-
/// Cluster reconfigurations that will need to be
1316-
/// cleaned up when the current transaction is cleared
1317-
pending_cluster_alters: BTreeSet<ClusterId>,
1318-
13191295
/// Channel on which to send notices to a session.
13201296
#[serde(skip)]
13211297
notice_tx: mpsc::UnboundedSender<AdapterNotice>,

src/adapter/src/coord/cluster_controller.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -363,7 +363,11 @@ impl Coordinator {
363363

364364
/// Build the controller's view of one managed cluster from the catalog.
365365
/// Returns `None` for a missing or unmanaged cluster.
366-
fn observe_cluster_state(&self, cluster_id: ClusterId) -> Option<ClusterState> {
366+
///
367+
/// Also used by the ALTER sequencer's synchronous cut-over, which runs the
368+
/// controller's reconcile kernel against this same view so both paths
369+
/// converge on the same replica set.
370+
pub(crate) fn observe_cluster_state(&self, cluster_id: ClusterId) -> Option<ClusterState> {
367371
let cluster = self.catalog().try_get_cluster(cluster_id)?;
368372
let ClusterVariant::Managed(managed) = &cluster.config.variant else {
369373
return None;

src/adapter/src/coord/command_handler.rs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -839,7 +839,6 @@ impl Coordinator {
839839
secret_key,
840840
notice_tx,
841841
drop_sinks: BTreeSet::new(),
842-
pending_cluster_alters: BTreeSet::new(),
843842
connected_at: self.now(),
844843
user,
845844
application_name,
@@ -1984,8 +1983,6 @@ impl Coordinator {
19841983
// SQL cancellation has no success response to delay. Each subscribe
19851984
// still waits for its own retraction before it observes retirement.
19861985
drop(retire_notify);
1987-
self.cancel_cluster_reconfigurations_for_conn(&conn_id)
1988-
.await;
19891986
self.cancel_pending_copy(&conn_id);
19901987
if let Some((tx, _rx)) = self.connection_cancel_watches.get_mut(&conn_id) {
19911988
let _ = tx.send(true);

src/adapter/src/coord/ddl.rs

Lines changed: 1 addition & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ use serde_json::json;
5151
use tracing::{Instrument, Level, event, info_span, warn};
5252

5353
use crate::active_compute_sink::{ActiveComputeSink, ActiveComputeSinkRetireReason};
54-
use crate::catalog::{DropObjectInfo, Op, ReplicaCreateDropReason, TransactionResult};
54+
use crate::catalog::{DropObjectInfo, Op, TransactionResult};
5555
use crate::coord::Coordinator;
5656
use crate::coord::appends::{BuiltinTableAppendCompletion, BuiltinTableAppendNotify};
5757
use crate::coord::catalog_implications::parsed_state_updates::ParsedStateUpdate;
@@ -859,43 +859,6 @@ impl Coordinator {
859859
}))
860860
}
861861

862-
/// Drops all pending replicas for a set of clusters
863-
/// that are undergoing reconfiguration.
864-
pub async fn drop_reconfiguration_replicas(
865-
&mut self,
866-
cluster_ids: BTreeSet<ClusterId>,
867-
) -> Result<(), AdapterError> {
868-
let pending_cluster_ops: Vec<Op> = cluster_ids
869-
.iter()
870-
.map(|c| {
871-
self.catalog()
872-
.get_cluster(c.clone())
873-
.replicas()
874-
.filter_map(|r| match r.config.location {
875-
ReplicaLocation::Managed(ref l) if l.pending => {
876-
Some(DropObjectInfo::ClusterReplica((
877-
c.clone(),
878-
r.replica_id,
879-
ReplicaCreateDropReason::Manual,
880-
)))
881-
}
882-
_ => None,
883-
})
884-
.collect::<Vec<DropObjectInfo>>()
885-
})
886-
.filter_map(|pending_replica_drop_ops_by_cluster| {
887-
match pending_replica_drop_ops_by_cluster.len() {
888-
0 => None,
889-
_ => Some(Op::DropObjects(pending_replica_drop_ops_by_cluster)),
890-
}
891-
})
892-
.collect();
893-
if !pending_cluster_ops.is_empty() {
894-
self.catalog_transact(None, pending_cluster_ops).await?;
895-
}
896-
Ok(())
897-
}
898-
899862
/// Cancels all active compute sinks for the identified connection.
900863
#[mz_ore::instrument(level = "debug")]
901864
pub(crate) async fn cancel_compute_sinks_for_conn(
@@ -906,15 +869,6 @@ impl Coordinator {
906869
.await
907870
}
908871

909-
/// Cancels all active cluster reconfigurations sinks for the identified connection.
910-
#[mz_ore::instrument(level = "debug")]
911-
pub(crate) async fn cancel_cluster_reconfigurations_for_conn(
912-
&mut self,
913-
conn_id: &ConnectionId,
914-
) {
915-
self.retire_cluster_reconfigurations_for_conn(conn_id).await
916-
}
917-
918872
/// Retires all active compute sinks for the identified connection with the
919873
/// specified reason.
920874
#[mz_ore::instrument(level = "debug")]
@@ -934,30 +888,6 @@ impl Coordinator {
934888
self.retire_compute_sinks(drop_sinks).await
935889
}
936890

937-
/// Cleans pending cluster reconfiguraiotns for the identified connection
938-
#[mz_ore::instrument(level = "debug")]
939-
pub(crate) async fn retire_cluster_reconfigurations_for_conn(
940-
&mut self,
941-
conn_id: &ConnectionId,
942-
) {
943-
let reconfiguring_clusters = self
944-
.active_conns
945-
.get(conn_id)
946-
.expect("must exist for active session")
947-
.pending_cluster_alters
948-
.clone();
949-
// try to drop reconfig replicas
950-
self.drop_reconfiguration_replicas(reconfiguring_clusters)
951-
.await
952-
.unwrap_or_terminate("cannot fail to drop reconfiguration replicas");
953-
954-
self.active_conns
955-
.get_mut(conn_id)
956-
.expect("must exist for active session")
957-
.pending_cluster_alters
958-
.clear();
959-
}
960-
961891
pub(crate) fn drop_storage_sinks(&mut self, sink_gids: Vec<GlobalId>) {
962892
let storage_metadata = self.catalog.state().storage_metadata();
963893
self.controller

0 commit comments

Comments
 (0)