Skip to content

Commit b71b025

Browse files
committed
cluster-controller: swap the replica set at a forced cut-over
A graceful reconfiguration overlaps the realized and target replica sets so the old one keeps serving while the new one hydrates. A forced cut-over has given up on hydration, so that overlap buys nothing and only doubles what the reshape has to fit. The baseline now yields its replicas once the record is past its deadline under ON TIMEOUT COMMIT, which turns the reshape into a single transaction that retires the realized replicas and creates the target's. Only the net has to fit the budget, so a resize lands on a budget with no room for both sets at once, and shrinking a cluster that already sits near its limit works at all. If the swap still does not fit it is rejected whole, leaving the record in progress for the controller to shed. Stop shedding a hydration burst on resource exhaustion. Nothing durable records that a burst was unaffordable, so clearing the record let the unchanged policy arm the same burst on the next tick: every cycle wrote a start and a finish, allocated a replica id, and woke the next reconciliation immediately. An unaffordable burst now keeps retrying its create, bounded by the tick interval, and settles once the steady set hydrates. That drops the durable BurstFinishCause::ResourceExhausted variant, so the stack no longer changes the catalog protos at all. Bound the sleeping dataflow in the cc_nonshape testdrive section. mz_sleep blocks the worker thread and dropping the view does not interrupt a sleep already in flight, so an hour-long sleep wedged the target replica and the section's convergence assertions could never pass.
1 parent 18b429a commit b71b025

12 files changed

Lines changed: 195 additions & 107 deletions

File tree

src/adapter-types/src/cluster_state.rs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -175,9 +175,6 @@ pub enum BurstFinishCause {
175175
/// policy was removed or its hydration size changed, the cluster was
176176
/// turned off, or burst was disabled environment-wide.
177177
NoLongerWarranted,
178-
/// The controller could not provision the desired replica set within the
179-
/// resource budget and shed the burst.
180-
ResourceExhausted,
181178
}
182179

183180
/// The action a graceful reconfiguration applies once its `deadline` passes with

src/adapter/src/catalog/transact.rs

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -630,7 +630,6 @@ impl Catalog {
630630
let cause = match cause {
631631
BurstFinishCause::LingerElapsed => BurstFinishCauseV1::LingerElapsed,
632632
BurstFinishCause::NoLongerWarranted => BurstFinishCauseV1::NoLongerWarranted,
633-
BurstFinishCause::ResourceExhausted => BurstFinishCauseV1::ResourceExhausted,
634633
};
635634
(
636635
HydrationBurstLifecycleV1::Finished,
@@ -3992,21 +3991,6 @@ mod tests {
39923991
);
39933992
assert_eq!(finished.burst_size, "large");
39943993

3995-
let resource_exhausted = Catalog::burst_audit_details(
3996-
&managed(Some(record())),
3997-
&managed(None),
3998-
cluster_id,
3999-
"c",
4000-
BurstAudit::Finished {
4001-
cause: BurstFinishCause::ResourceExhausted,
4002-
},
4003-
)
4004-
.expect("resource shedding carries the cleared record's details");
4005-
assert_eq!(
4006-
resource_exhausted.finish_cause,
4007-
Some(BurstFinishCauseV1::ResourceExhausted)
4008-
);
4009-
40103994
// An intent without a record on the corresponding side contradicts the
40113995
// write and must fail the transaction.
40123996
let incoherent = Catalog::burst_audit_details(

src/audit-log/src/lib.rs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -522,9 +522,6 @@ pub enum BurstFinishCauseV1 {
522522
/// policy was removed or its hydration size changed, the cluster was
523523
/// turned off, or burst was disabled environment-wide.
524524
NoLongerWarranted,
525-
/// The controller could not provision the desired replica set within the
526-
/// resource budget and shed the burst.
527-
ResourceExhausted,
528525
}
529526

530527
/// A cluster-level transition in a hydration burst's lifecycle.

src/catalog-protos/objects_hashes.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
[
22
{
33
"name": "objects.rs",
4-
"md5": "07f8174bfbc0f6678573152bef73da25"
4+
"md5": "033aea36ed61e9181489faffbf9de51d"
55
},
66
{
77
"name": "objects_v74.rs",
@@ -77,6 +77,6 @@
7777
},
7878
{
7979
"name": "objects_v92.rs",
80-
"md5": "07f8174bfbc0f6678573152bef73da25"
80+
"md5": "033aea36ed61e9181489faffbf9de51d"
8181
}
8282
]

src/catalog-protos/src/audit_log.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -431,7 +431,6 @@ impl RustType<crate::objects::audit_log_event_v1::BurstFinishCauseV1> for BurstF
431431
let cause = match self {
432432
BurstFinishCauseV1::LingerElapsed => Cause::LingerElapsed(Empty {}),
433433
BurstFinishCauseV1::NoLongerWarranted => Cause::NoLongerWarranted(Empty {}),
434-
BurstFinishCauseV1::ResourceExhausted => Cause::ResourceExhausted(Empty {}),
435434
};
436435
crate::objects::audit_log_event_v1::BurstFinishCauseV1 { cause }
437436
}
@@ -443,7 +442,6 @@ impl RustType<crate::objects::audit_log_event_v1::BurstFinishCauseV1> for BurstF
443442
Ok(match proto.cause {
444443
Cause::LingerElapsed(_) => BurstFinishCauseV1::LingerElapsed,
445444
Cause::NoLongerWarranted(_) => BurstFinishCauseV1::NoLongerWarranted,
446-
Cause::ResourceExhausted(_) => BurstFinishCauseV1::ResourceExhausted,
447445
})
448446
}
449447
}

src/catalog-protos/src/objects.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1478,7 +1478,6 @@ pub mod audit_log_event_v1 {
14781478
pub enum Cause {
14791479
LingerElapsed(Empty),
14801480
NoLongerWarranted(Empty),
1481-
ResourceExhausted(Empty),
14821481
}
14831482
}
14841483

src/catalog-protos/src/objects_v92.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1478,7 +1478,6 @@ pub mod audit_log_event_v1 {
14781478
pub enum Cause {
14791479
LingerElapsed(Empty),
14801480
NoLongerWarranted(Empty),
1481-
ResourceExhausted(Empty),
14821481
}
14831482
}
14841483

src/cluster-controller/src/lib.rs

Lines changed: 32 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,9 @@ use mz_dyncfg::ConfigSet;
4444
use mz_ore::soft_panic_or_log;
4545

4646
use crate::ctx::{
47-
ApplyOutcome, BurstAudit, BurstFinishCause, BurstWrite, ClusterControllerCtx, ClusterState,
48-
CreateReason, Decision, ObservedReplica, ReconfigurationAudit, ReconfigurationRecord,
49-
ReconfigurationStatus, ReconfigurationWrite, RefreshWindowInputs, ReplicaShape, StateWrite,
47+
ApplyOutcome, ClusterControllerCtx, ClusterState, CreateReason, Decision, ObservedReplica,
48+
ReconfigurationAudit, ReconfigurationRecord, ReconfigurationStatus, ReconfigurationWrite,
49+
RefreshWindowInputs, ReplicaShape, StateWrite,
5050
};
5151
use crate::strategy::{
5252
BaselineStrategy, ConfigSignals, DesiredReplica, GracefulReconfigurationStrategy,
@@ -199,42 +199,39 @@ impl ClusterController {
199199
}
200200
}
201201

202-
/// The decision that sheds this cluster's most expendable transient strategy
203-
/// after a resource-exhausted apply, or `None` if nothing sheddable is
204-
/// active.
202+
/// The decision that sheds this cluster's in-progress graceful
203+
/// reconfiguration after a resource-exhausted apply, or `None` if there is
204+
/// none to shed.
205205
///
206-
/// The strategy to shed is chosen by presence, ranked by expendability, not
207-
/// by which create failed: validation is aggregate, and the strategy worth
208-
/// giving up may be one whose replicas already materialized rather than one
209-
/// in the failed batch. The graceful reconfiguration is the most expendable:
210-
/// a discretionary user change that fails cleanly and can be retried.
211-
/// Aborting it leaves the cluster running at its realized shape. A hydration
212-
/// burst is next. The baseline is never shed, it is the committed floor.
206+
/// A reconfiguration is a discretionary user change: it fails cleanly, it
207+
/// can be retried, and abandoning it leaves the cluster running at its
208+
/// realized shape. That makes it the one strategy worth giving up under
209+
/// budget pressure. It is shed on presence, not on which create failed:
210+
/// validation is aggregate, so the batch that failed may not be the one
211+
/// whose replicas are the problem.
213212
///
214-
/// We shed one strategy per exhausted apply. If that was not enough, the
215-
/// next tick recomputes and sheds the next one.
213+
/// The baseline is the committed floor and is never shed. Neither is a
214+
/// hydration burst, even though it is transient: nothing in the durable
215+
/// burst state records that a burst was unaffordable, so clearing it would
216+
/// let the unchanged policy arm the same burst on the next tick. Each
217+
/// cycle would write a start and a finish, allocate a replica id, and wake
218+
/// the next reconciliation immediately. An unaffordable burst keeps
219+
/// retrying its create instead, which costs one rejected transaction per
220+
/// tick and settles once the steady set hydrates.
216221
fn shed_decision(state: &ClusterState) -> Option<Decision> {
217-
let write = match &state.reconfiguration {
218-
Some(record) if record.is_in_progress() => StateWrite {
219-
reconfiguration: Some(ReconfigurationWrite {
220-
record: Some(ReconfigurationRecord {
221-
status: ReconfigurationStatus::ResourceExhausted,
222-
..record.clone()
223-
}),
224-
audit: Some(ReconfigurationAudit::ResourceExhausted),
222+
let record = state
223+
.reconfiguration
224+
.as_ref()
225+
.filter(|record| record.is_in_progress())?;
226+
let write = StateWrite {
227+
reconfiguration: Some(ReconfigurationWrite {
228+
record: Some(ReconfigurationRecord {
229+
status: ReconfigurationStatus::ResourceExhausted,
230+
..record.clone()
225231
}),
226-
..Default::default()
227-
},
228-
_ if state.burst.is_some() => StateWrite {
229-
burst: Some(BurstWrite {
230-
record: None,
231-
audit: Some(BurstAudit::Finished {
232-
cause: BurstFinishCause::ResourceExhausted,
233-
}),
234-
}),
235-
..Default::default()
236-
},
237-
_ => return None,
232+
audit: Some(ReconfigurationAudit::ResourceExhausted),
233+
}),
234+
..Default::default()
238235
};
239236
Some(Decision::UpdateClusterState {
240237
cluster_id: state.cluster_id,

src/cluster-controller/src/strategy.rs

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -178,20 +178,51 @@ pub struct LiveSignals {
178178
/// contributor. (The on-refresh strategy also normalizes a scheduled cluster's
179179
/// `replication_factor` to `0` via `update_state`, so the two views agree after
180180
/// the first tick regardless.)
181+
///
182+
/// The one case where the baseline steps aside is a forced cut-over, see
183+
/// [`forced_cutover_pending`].
181184
#[derive(Clone, Copy, Debug, Default)]
182185
pub struct BaselineStrategy;
183186

187+
/// Whether a forced cut-over is imminent: an in-progress reconfiguration is
188+
/// past its deadline under `ON TIMEOUT COMMIT`, so the next cut-over commits
189+
/// the target whether or not it hydrated.
190+
///
191+
/// In that window the baseline yields its realized-shape replicas. Overlapping
192+
/// the two sets only buys availability while the target hydrates, and a forced
193+
/// cut-over has given up on hydration. Yielding turns the reshape into one
194+
/// transaction that retires the realized replicas and creates the target's, so
195+
/// it has to fit the larger of the two shapes rather than their sum. That is
196+
/// what lets a resize succeed on a budget that has no room for overlap, and it
197+
/// is the only way to shrink a cluster that is already near its limit.
198+
///
199+
/// If that single transaction still does not fit, it is rejected whole and the
200+
/// record is left in progress for [`ClusterController::shed_decision`] to shed,
201+
/// so an unaffordable target stays observable rather than half-applied.
202+
///
203+
/// [`ClusterController::shed_decision`]: crate::ClusterController::shed_decision
204+
fn forced_cutover_pending(state: &ClusterState, now: Timestamp) -> bool {
205+
state.reconfiguration.as_ref().is_some_and(|record| {
206+
record.is_in_progress()
207+
&& now >= record.deadline
208+
&& matches!(record.on_timeout, OnTimeout::Commit)
209+
})
210+
}
211+
184212
impl Strategy for BaselineStrategy {
185213
fn desired_replicas(
186214
&self,
187215
state: &ClusterState,
188216
_signals: &LiveSignals,
189217
_config: &ConfigSignals,
190-
_now: Timestamp,
218+
now: Timestamp,
191219
) -> Vec<DesiredReplica> {
192220
if !matches!(state.schedule, ClusterSchedule::Manual) {
193221
return Vec::new();
194222
}
223+
if forced_cutover_pending(state, now) {
224+
return Vec::new();
225+
}
195226
let shape = state.realized_shape();
196227
(0..state.replication_factor)
197228
.map(|_| DesiredReplica {
@@ -212,9 +243,11 @@ impl Strategy for BaselineStrategy {
212243
/// record is marked finalized, and the old replicas fall out of the union and
213244
/// are dropped. Success takes precedence over the deadline. On a timeout,
214245
/// `Commit` cuts over once the complete target set exists without waiting for
215-
/// hydration. `Rollback` (the default) marks the record timed out without
216-
/// touching the realized config and stops desiring the target replicas,
217-
/// reverting to the pre-reconfiguration set.
246+
/// hydration, and the baseline stops contributing in that window so the two
247+
/// sets swap in one transaction rather than overlapping (see
248+
/// [`forced_cutover_pending`]). `Rollback` (the default) marks the record timed
249+
/// out without touching the realized config and stops desiring the target
250+
/// replicas, reverting to the pre-reconfiguration set.
218251
///
219252
/// Both functions are pure over the observed [`ClusterState`] and the fetched
220253
/// [`LiveSignals`]. Hydration is requested via [`Strategy::signal_request`]
@@ -298,6 +331,9 @@ impl Strategy for GracefulReconfigurationStrategy {
298331
// concrete create transaction can enforce resource limits. Otherwise a
299332
// zero-timeout commit could finalize first, fail to create the new
300333
// baseline, and leave no in-progress strategy for the controller to shed.
334+
// The baseline yields while we wait (see `forced_cutover_pending`), so
335+
// that create arrives in the same transaction that retires the realized
336+
// replicas and does not have to fit alongside them.
301337
let hydrated = self.target_hydrated(state, signals, record);
302338
let deadline_reached = now >= record.deadline;
303339
let commit_on_timeout = deadline_reached && matches!(record.on_timeout, OnTimeout::Commit);

src/cluster-controller/src/tests.rs

Lines changed: 76 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2045,7 +2045,7 @@ async fn graceful_commit_at_timeout_cuts_over_through_seam() {
20452045
#[mz_ore::test(tokio::test)]
20462046
async fn resource_exhaustion_sheds_a_zero_timeout_reconfiguration() {
20472047
// The elapsed commit deadline does not finalize before the target replica
2048-
// exists. Phase 2 attempts the create, which reports resource exhaustion,
2048+
// exists. Phase 2 attempts the swap, which reports resource exhaustion,
20492049
// then the controller sheds the still-in-progress reconfiguration.
20502050
let c = cluster(1);
20512051
let (state, _signals) = reconfiguring_state(
@@ -2063,11 +2063,13 @@ async fn resource_exhaustion_sheds_a_zero_timeout_reconfiguration() {
20632063

20642064
controller.reconcile(&mut ctx).await;
20652065

2066-
// Two applies: the exhausted create batch, then the shed.
2066+
// Two applies: the exhausted swap batch, then the shed.
20672067
assert_eq!(ctx.applied.len(), 2);
20682068
assert!(
2069-
matches!(ctx.applied[0][0], Decision::CreateReplica { .. }),
2070-
"the exhausted batch was the target create"
2069+
ctx.applied[0]
2070+
.iter()
2071+
.any(|d| matches!(d, Decision::CreateReplica { .. })),
2072+
"the exhausted batch carries the target create"
20712073
);
20722074
let [shed] = &ctx.applied[1][..] else {
20732075
panic!("the shed is a single decision, got {:?}", ctx.applied[1]);
@@ -2114,8 +2116,68 @@ async fn resource_exhaustion_sheds_a_zero_timeout_reconfiguration() {
21142116
}
21152117

21162118
#[mz_ore::test(tokio::test)]
2117-
async fn resource_exhaustion_sheds_reconfiguration_then_burst() {
2118-
use crate::ctx::{BurstFinishCause, BurstRecord, OnHydrationPolicy};
2119+
async fn forced_cutover_swaps_the_replica_set_in_one_transaction() {
2120+
// A forced cut-over never overlaps the two replica sets: the baseline
2121+
// yields, so the tick that provisions the target also retires the realized
2122+
// replicas, in one transaction. That is what lets a reshape land on a
2123+
// budget with no room for both sets at once.
2124+
let c = cluster(1);
2125+
let (state, _signals) = reconfiguring_state(
2126+
c,
2127+
"100cc",
2128+
1,
2129+
vec![observed(replica(1), "r0", "100cc")],
2130+
record_on_timeout("200cc", 1, 0, OnTimeout::Commit),
2131+
BTreeSet::new(),
2132+
);
2133+
let mut ctx = FakeCtx::new(vec![state]);
2134+
let controller = controller();
2135+
2136+
controller.reconcile(&mut ctx).await;
2137+
2138+
assert_eq!(ctx.applied.len(), 1, "the swap is a single apply");
2139+
let swap = &ctx.applied[0];
2140+
assert_eq!(
2141+
swap.iter()
2142+
.filter(|d| matches!(d, Decision::CreateReplica { .. }))
2143+
.count(),
2144+
1,
2145+
"the target replica is created"
2146+
);
2147+
assert_eq!(
2148+
swap.iter()
2149+
.filter(|d| matches!(d, Decision::DropReplica { .. }))
2150+
.count(),
2151+
1,
2152+
"the realized replica is retired in the same batch"
2153+
);
2154+
let sizes: Vec<_> = ctx.states[&c]
2155+
.replicas
2156+
.iter()
2157+
.filter_map(|r| r.owned_shape().map(|shape| shape.size.as_str()))
2158+
.collect();
2159+
assert_eq!(sizes, vec!["200cc"], "no overlap was ever materialized");
2160+
assert_eq!(
2161+
ctx.states[&c].size, "100cc",
2162+
"the cut-over itself waits for the next tick's first phase"
2163+
);
2164+
2165+
// With the target now present, the deadline commits it.
2166+
controller.reconcile(&mut ctx).await;
2167+
assert_eq!(ctx.states[&c].size, "200cc", "the forced cut-over lands");
2168+
assert_eq!(
2169+
reconfiguration_status(&ctx.states[&c]),
2170+
Some(ReconfigurationStatus::Finalized)
2171+
);
2172+
}
2173+
2174+
#[mz_ore::test(tokio::test)]
2175+
async fn resource_exhaustion_leaves_an_unaffordable_burst_armed() {
2176+
// A hydration burst is not shed on exhaustion. Nothing durable would record
2177+
// that it was unaffordable, so the unchanged policy would arm it again on
2178+
// the next tick and every cycle would write a start and a finish. The burst
2179+
// stays armed and its create is simply retried.
2180+
use crate::ctx::{BurstRecord, OnHydrationPolicy};
21192181

21202182
let c = cluster(1);
21212183
let (mut state, _signals) = reconfiguring_state(
@@ -2147,30 +2209,24 @@ async fn resource_exhaustion_sheds_reconfiguration_then_burst() {
21472209
assert_eq!(
21482210
reconfiguration_status(&ctx.states[&c]),
21492211
Some(ReconfigurationStatus::ResourceExhausted),
2150-
"the graceful reconfiguration is shed first"
2212+
"the graceful reconfiguration is shed"
21512213
);
21522214
assert!(
21532215
ctx.states[&c].burst.is_some(),
2154-
"the first exhausted apply leaves the lower-priority burst active"
2216+
"the burst survives the shed"
21552217
);
21562218

21572219
ctx.exhaust_next = 1;
2220+
let before = ctx.applied.len();
21582221
controller.reconcile(&mut ctx).await;
21592222
assert!(
2160-
ctx.states[&c].burst.is_none(),
2161-
"a later exhausted apply clears the burst"
2223+
ctx.states[&c].burst.is_some(),
2224+
"a later exhausted apply still leaves the burst alone"
21622225
);
2163-
let [shed] = &ctx.applied.last().expect("burst shed was applied")[..] else {
2164-
panic!("the burst shed is one decision")
2165-
};
2166-
let Decision::UpdateClusterState { write, .. } = shed else {
2167-
panic!("expected the burst shed to update cluster state")
2168-
};
21692226
assert_eq!(
2170-
written_burst_audit(write),
2171-
Some(BurstAudit::Finished {
2172-
cause: BurstFinishCause::ResourceExhausted,
2173-
})
2227+
ctx.applied.len(),
2228+
before + 1,
2229+
"the exhausted create is the only apply: no shed follows it"
21742230
);
21752231
}
21762232

0 commit comments

Comments
 (0)