Skip to content

Commit fbc61b7

Browse files
committed
adapter: simplify the cut-over target fold and replica shape plumbing
Follow-ups on deleting the staged reconfiguration machine, no behavior change. Collapse the cut-over routing's bool-plus-Option pair into a single optional cut-over target, computed unconditionally on that path. The fold with nothing in flight returns exactly the statement's shape, so applying it is the identity and the "nothing in flight, nothing to fold" special case disappears. The cut-over now computes its target through the same call the reshape path makes, so the two agreeing is structural rather than commented. Applying a target onto a managed config is the inverse of `realized_reconfiguration_target`, so it moves next to it in objects.rs as `apply_reconfiguration_target`, keeping the exhaustive-destructure compile guard beside the type it guards. `sequence_create_managed_cluster` builds the one `ReplicaShape` all its replicas share once, instead of a separate logging binding plus a per-iteration shape construction in the loop. Also fix the `sequence_alter_cluster_managed_to_managed` doc, which claimed the carried record is retired as cancelled and the replica set dropped wholesale: `retire_carried_reconfiguration` settles an on-target record as finalized (forced), and the reconcile keeps already-matching replicas.
1 parent fe148a0 commit fbc61b7

1 file changed

Lines changed: 44 additions & 54 deletions

File tree

src/adapter/src/coord/sequencer/inner/cluster.rs

Lines changed: 44 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -349,11 +349,14 @@ impl Coordinator {
349349
// cancel. With nothing in flight the values decide: a shape option set
350350
// to its current value reconfigures nothing, and reshaping it anyway
351351
// would write a spurious pre-cancelled record.
352-
let mut cut_over = false;
353-
// The folded target the cut-over must materialize, when a
354-
// reconfiguration is in flight. Applied to `new_config` below, once the
355-
// borrow taken here is released.
356-
let mut folded_target = None;
352+
//
353+
// The target the cut-over must materialize: the shape and factor the
354+
// statement asks for, folded onto any in-flight record's target,
355+
// exactly as the reshape path computes it. Applied to `new_config`
356+
// below, once the borrow taken here is released. With nothing in
357+
// flight the fold returns the statement's shape unchanged, so applying
358+
// it is the identity.
359+
let mut cut_over_target = None;
357360
if let (Managed(old_managed), Managed(new_managed)) = (&config.variant, &new_config.variant)
358361
{
359362
let needs_record = if reconfiguration_in_flight {
@@ -387,18 +390,15 @@ impl Coordinator {
387390
}
388391
if needs_record && !scheduled_direct {
389392
if requests_immediate_cut_over(strategy) {
390-
cut_over = true;
391-
// The cut-over transacts the same target the reshape path
392-
// would have written, so it folds onto an in-flight one the
393-
// same way. With nothing in flight there is nothing to fold
394-
// and `new_config` already carries the target.
395-
folded_target = old_managed
393+
let in_flight = old_managed
396394
.reconfiguration
397395
.as_ref()
398-
.filter(|record| record.is_in_progress())
399-
.map(|record| {
400-
alter_reconfiguration_target(new_managed, options, Some(&record.target))
401-
});
396+
.filter(|record| record.is_in_progress());
397+
cut_over_target = Some(alter_reconfiguration_target(
398+
new_managed,
399+
options,
400+
in_flight.map(|record| &record.target),
401+
));
402402
} else {
403403
let result = self
404404
.reshape_alter_cluster_managed(
@@ -419,24 +419,12 @@ impl Coordinator {
419419
}
420420
}
421421
}
422-
if let Some(target) = folded_target {
422+
let cut_over = cut_over_target.is_some();
423+
if let Some(target) = cut_over_target {
423424
let Managed(target_managed) = &mut new_config.variant else {
424-
unreachable!("a folded target is produced only for a managed config");
425+
unreachable!("a cut-over target is produced only for a managed config");
425426
};
426-
// Destructured so a new target dimension fails to compile until it
427-
// is applied here too.
428-
let ReconfigurationTarget {
429-
size,
430-
replication_factor,
431-
availability_zones,
432-
logging,
433-
arrangement_compression,
434-
} = target;
435-
target_managed.size = size;
436-
target_managed.replication_factor = replication_factor;
437-
target_managed.availability_zones = availability_zones;
438-
target_managed.logging = logging;
439-
target_managed.arrangement_compression = arrangement_compression;
427+
target_managed.apply_reconfiguration_target(target);
440428
}
441429

442430
match (&config.variant, &new_config.variant) {
@@ -978,15 +966,20 @@ impl Coordinator {
978966

979967
self.ensure_valid_azs(availability_zones.iter())?;
980968

981-
// The same logging the cluster's own config carries (see
982-
// `sequence_create_cluster`), so the replicas created below match the
983-
// shape the controller will reconcile them against.
984-
let replica_logging = match compute.introspection {
985-
Some(config) => ReplicaLogging {
986-
log_logging: config.debugging,
987-
interval: Some(config.interval),
969+
// The shape every replica below is created at, matching the cluster's
970+
// own config (see `sequence_create_cluster`) so the controller
971+
// reconciles the replicas as already conforming.
972+
let replica_shape = ReplicaShape {
973+
size: size.clone(),
974+
availability_zones: AvailabilityZones(availability_zones.clone()),
975+
logging: match compute.introspection {
976+
Some(config) => ReplicaLogging {
977+
log_logging: config.debugging,
978+
interval: Some(config.interval),
979+
},
980+
None => ReplicaLogging::default(),
988981
},
989-
None => ReplicaLogging::default(),
982+
arrangement_compression: compute.arrangement_compression,
990983
};
991984

992985
let role_id = session.role_metadata().current_role;
@@ -1055,12 +1048,7 @@ impl Coordinator {
10551048
cluster_id,
10561049
replica_id,
10571050
replica_name.clone(),
1058-
&ReplicaShape {
1059-
size: size.clone(),
1060-
availability_zones: AvailabilityZones(availability_zones.clone()),
1061-
logging: replica_logging.clone(),
1062-
arrangement_compression: compute.arrangement_compression,
1063-
},
1051+
&replica_shape,
10641052
&mut ops,
10651053
*session.current_role_id(),
10661054
ReplicaCreateDropReason::Manual,
@@ -1654,15 +1642,17 @@ impl Coordinator {
16541642
///
16551643
/// `cut_over` is the direct path an explicitly zero-timeout commit `WAIT`
16561644
/// requests (see [`requests_immediate_cut_over`]): the observed owned
1657-
/// replica set is dropped and recreated at the target shape and factor, and
1658-
/// any carried reconfiguration record is retired as cancelled, all in this
1659-
/// one catalog transaction with no controller involvement. It is the one
1660-
/// reshape that still works when the controller itself is the problem, and
1661-
/// it simultaneously unsticks a reconfiguration nothing else would retire.
1662-
/// Requesting it under a live controller stays safe: the config write
1663-
/// invalidates any in-flight tick's compare-and-append witness, so a stale
1664-
/// controller batch is rejected like it would be for any user DDL landing
1665-
/// mid-tick.
1645+
/// replica set is converged onto the target shape and factor (replicas
1646+
/// that already match are kept, up to the factor), and any carried
1647+
/// reconfiguration record is settled to a terminal status (see
1648+
/// [`retire_carried_reconfiguration`]), all in this one catalog
1649+
/// transaction with no controller involvement. It
1650+
/// is the one reshape that still works when the controller itself is the
1651+
/// problem, and it simultaneously unsticks a reconfiguration nothing else
1652+
/// would retire. Requesting it under a live controller stays safe: the
1653+
/// config write invalidates any in-flight tick's compare-and-append
1654+
/// witness, so a stale controller batch is rejected like it would be for
1655+
/// any user DDL landing mid-tick.
16661656
///
16671657
/// # Panics
16681658
///

0 commit comments

Comments
 (0)