Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 5 additions & 8 deletions src/adapter/src/coord/cluster_controller.rs
Original file line number Diff line number Diff line change
Expand Up @@ -985,14 +985,11 @@ impl Coordinator {
} else {
Some(&shape.availability_zones.0)
};
let location = self.catalog().concretize_replica_location(
location,
&self
.catalog()
.get_role_allowed_cluster_sizes(&Some(owner_id)),
azs,
false,
)?;
// DDL authorizes sizes before committing the configuration. Reconcile
// must still realize it if the owner's size allow-list later shrinks.
Comment on lines +988 to +989

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The list isn't per owner: it's one system variable, with only mz_system exempt. "DDL authorizes" also misses sizes that older releases stored through the conversion this PR fixes. Maybe:

Suggested change
// DDL authorizes sizes before committing the configuration. Reconcile
// must still realize it if the owner's size allow-list later shrinks.
// The allow-list is a DDL-time check on the role writing a size.
// Reconcile does not repeat it, so a size the current list rejects
// still gets its replica, for example after the list shrinks.

let location = self
.catalog()
.concretize_replica_location(location, &vec![], azs, false)?;

let config = mz_controller::clusters::ReplicaConfig {
location,
Expand Down
9 changes: 8 additions & 1 deletion src/adapter/src/coord/sequencer/inner/cluster.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1690,7 +1690,14 @@ impl Coordinator {
AlterOptionParameter::Reset | AlterOptionParameter::Unchanged => {
coord_bail!("Missing SIZE for empty cluster")
}
AlterOptionParameter::Set(_) => {} // Was set within the calling function.
AlterOptionParameter::Set(size) => {
let role_id = Some(session.role_metadata().current_role);
self.catalog.ensure_valid_replica_size(
&self.catalog().get_role_allowed_cluster_sizes(&role_id),
size,
false,
)?;
}
}
} else if sizes.len() == 1 {
let size = sizes.into_iter().next().expect("must exist");
Expand Down
72 changes: 67 additions & 5 deletions test/testdrive/cluster-controller.td
Original file line number Diff line number Diff line change
Expand Up @@ -1052,6 +1052,16 @@ ALTER SYSTEM SET unsafe_enable_unstable_dependencies = false
> SELECT count(*) FROM mz_cluster_replicas r JOIN mz_clusters c ON r.cluster_id = c.id WHERE c.name = 'cc_sched'
0

# SQL-564: scheduling an accepted configuration must not reauthorize its size.
$ postgres-execute connection=mz_system
ALTER SYSTEM SET allowed_cluster_replica_sizes = 'scale=1,workers=2'

! CREATE CLUSTER cc_disallowed SIZE 'scale=1,workers=1'
contains:unknown cluster replica size scale=1,workers=1

! ALTER CLUSTER cc_sched SET (SIZE = 'scale=2,workers=2')
contains:unknown cluster replica size scale=2,workers=2

# Bind a REFRESH EVERY materialized view: the cluster is now inside a refresh window
# (it must rehydrate ahead of each refresh), so the controller brings up exactly one
# replica at the cluster size. The replication factor still reads 0 (it is the
Expand Down Expand Up @@ -1092,6 +1102,9 @@ drop retired true
> DROP TABLE sched_t
> DROP CLUSTER cc_sched

$ postgres-execute connection=mz_system
ALTER SYSTEM RESET allowed_cluster_replica_sizes

# A config-shape ALTER on a scheduled cluster takes the direct path: no
# reconfiguration record is written (so no lifecycle transition is audited),
# the realized size advances immediately, and the controller reconciles the
Expand Down Expand Up @@ -1382,6 +1395,14 @@ ALTER SYSTEM SET unsafe_enable_unstable_dependencies = true

> CREATE CLUSTER cc_burst_e2e (SIZE 'scale=1,workers=1', REPLICATION FACTOR 1, AUTO SCALING STRATEGY = (ON HYDRATION (HYDRATION SIZE = 'scale=2,workers=2', LINGER DURATION = '600s')))

# The retained hydration size remains realizable after the allow-list shrinks,
# but a new policy using it must still be rejected by DDL authorization.
$ postgres-execute connection=mz_system
ALTER SYSTEM SET allowed_cluster_replica_sizes = 'scale=1,workers=1', 'scale=1,workers=2'

! CREATE CLUSTER cc_burst_disallowed (SIZE 'scale=1,workers=1', AUTO SCALING STRATEGY = (ON HYDRATION (HYDRATION SIZE = 'scale=2,workers=2')))
contains:unknown cluster replica size scale=2,workers=2

> CREATE TABLE cc_burst_e2e_t (id int)

> INSERT INTO cc_burst_e2e_t VALUES (1)
Expand Down Expand Up @@ -1416,6 +1437,15 @@ started scale=2,workers=2
! ALTER CLUSTER cc_burst_e2e SET (MANAGED = false)
contains:cannot convert cluster to unmanaged while a hydration burst is in progress

# An allowed resize must progress without waiting for the retained burst to
# retire. The sleeping view keeps all three replicas unhydrated.
> ALTER CLUSTER cc_burst_e2e SET (SIZE = 'scale=1,workers=2')

> SELECT r.size FROM mz_cluster_replicas r JOIN mz_clusters c ON r.cluster_id = c.id WHERE c.name = 'cc_burst_e2e'
scale=1,workers=1
scale=1,workers=2
scale=2,workers=2

# RESET tears the burst down: the record clears with a finished event, the
# burst replica is retired, and the relation row disappears with the policy.
> ALTER CLUSTER cc_burst_e2e RESET (AUTO SCALING STRATEGY)
Expand All @@ -1433,6 +1463,9 @@ no-longer-warranted

> DROP TABLE cc_burst_e2e_t

$ postgres-execute connection=mz_system
ALTER SYSTEM RESET allowed_cluster_replica_sizes

# NOTE: The next section covers a singleton source riding a hydration burst,
# which requires the two-cluster replica topology: on a unified cluster the
# blocking mz_sleep gate starves co-hosted sources and their command
Expand Down Expand Up @@ -1587,15 +1620,44 @@ ALTER SYSTEM RESET enable_unified_cluster

# ----- Direct-path invariants on scheduled clusters -----

# Converting a nonempty unmanaged cluster to ON REFRESH adopts its replica
# rather than failing replication-factor validation, and the controller then
# normalizes the factor to 0.
# An empty cluster introduces a size when converted, even if ON REFRESH leaves
# it without replicas until a refresh window opens.
> CREATE CLUSTER cc_empty_conv REPLICAS ()
> CREATE CLUSTER cc_conv (REPLICAS (r1 (SIZE 'scale=1,workers=1')))

! ALTER CLUSTER cc_empty_conv SET (MANAGED, SIZE = 'notasize')
contains:unknown cluster replica size notasize

! ALTER CLUSTER cc_empty_conv SET (MANAGED, SIZE = 'free')
contains:unknown cluster replica size free

$ postgres-execute connection=mz_system
ALTER SYSTEM SET allowed_cluster_replica_sizes = 'scale=1,workers=2'

! ALTER CLUSTER cc_empty_conv SET (MANAGED, SIZE = 'scale=1,workers=1', SCHEDULE = ON REFRESH)
contains:unknown cluster replica size scale=1,workers=1

> SELECT managed FROM mz_clusters WHERE name = 'cc_empty_conv'
false

> ALTER CLUSTER cc_empty_conv SET (MANAGED, SIZE = 'scale=1,workers=2', SCHEDULE = ON REFRESH)

> SELECT managed, size FROM mz_clusters WHERE name = 'cc_empty_conv'
true scale=1,workers=2

> DROP CLUSTER cc_empty_conv

# Converting a nonempty unmanaged cluster to ON REFRESH adopts its replica
# and its accepted size, even after the allow-list shrinks. The controller
# normalizes the replication factor to 0.
> ALTER CLUSTER cc_conv SET (MANAGED = true, SIZE = 'scale=1,workers=1', SCHEDULE = ON REFRESH (HYDRATION TIME ESTIMATE = '60 seconds'))
> SELECT replication_factor FROM mz_catalog.mz_clusters WHERE name = 'cc_conv'
0
> SELECT size, replication_factor FROM mz_catalog.mz_clusters WHERE name = 'cc_conv'
scale=1,workers=1 0
> DROP CLUSTER cc_conv

$ postgres-execute connection=mz_system
ALTER SYSTEM RESET allowed_cluster_replica_sizes

# An availability-zone change on a scheduled cluster is validated like on every
# other path, not written unchecked into the realized config.
> CREATE CLUSTER cc_az_sched (SIZE 'scale=1,workers=1', SCHEDULE = ON REFRESH (HYDRATION TIME ESTIMATE = '60 seconds'))
Expand Down
Loading