Skip to content

adapter: validate BILLED AS against the replica size map - #38630

Draft
ggevay wants to merge 1 commit into
MaterializeInc:mainfrom
ggevay:gabor/billed-as-size-validation
Draft

adapter: validate BILLED AS against the replica size map#38630
ggevay wants to merge 1 commit into
MaterializeInc:mainfrom
ggevay:gabor/billed-as-size-validation

Conversation

@ggevay

@ggevay ggevay commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Creating an internal replica with a BILLED AS value that is not a configured replica size crashed environmentd instead of returning an error (SQL-658). This hit a production environment in #incident-db-1257: three coordinator panics from one statement retried twice, about twelve minutes of environmentd downtime across three restarts.

Cause

  • BILLED AS was not validated when the replica location is concretized, only SIZE was validated.
  • The credit-consumption check reads the size map by the billing size with an expect, so an unknown BILLED AS panicked the coordinator thread.

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

  • Coordinator::ensure_valid_billed_as_size checks the value against the size map. In contrast to SIZE, this accepts disabled sizes and ignores the role's allowed sizes, since billing only reads the credit rate.
  • Both DDL entry points call it: CREATE CLUSTER REPLICA and CREATE CLUSTER ... REPLICAS (...). The statement now fails with unknown cluster replica size <size> in BILLED AS.
  • It is deliberately not in concretize_replica_location, which catalog open also runs for every durable replica and expects to succeed. A check there would turn a BILLED AS size removed from the map after the replica exists into a startup panic, whereas at DDL time the statement can simply fail.

Tolerant credit lookups, src/adapter/src/coord.rs and coord/ddl.rs

  • The three lookups by billing size (replica create, replica drop, and the sum over all existing replicas) share Coordinator::replica_credits_per_hour, which counts a replica whose billing size has since left the map as free and raises soft_panic_or_log!.
  • The sum over existing replicas only sees non-internal replicas, whose SIZE is validated at catalog open, so a stale billing size only reaches the drop site. Dropping such a replica used to panic the coordinator.

Tests

  • test/sqllogictest/cluster.slt: CREATE CLUSTER REPLICA and CREATE CLUSTER ... REPLICAS with an unknown BILLED AS now expect the error. The existing BILLED AS 'free' case, a disabled size in the test map, keeps passing.
  • test/cluster/mzcompose.py, workflow test-billed-as-size-removed: creates an internal BILLED AS replica, restarts environmentd with that size removed from the map and soft assertions off, then drops the replica and runs further cluster DDL.

How this was verified

  • The cluster workflow fails on the v26.39.0 release image with the drop-site coordinator panic, and passes on this branch's CI image with the fallback logged once.

Closes: SQL-658
Closes: CPU-236

🤖 Generated with Claude Code

@ggevay ggevay added the A-ADAPTER Topics related to the ADAPTER layer label Sep 2, 2026
CREATE CLUSTER REPLICA ... INTERNAL, BILLED AS '<size>' with a size that
is not in the environment's replica size map panicked the coordinator:
the credit-consumption check looked the billing size up with an expect
that assumed it had been validated like SIZE, but only SIZE was.

Validate BILLED AS at both DDL entry points (CREATE CLUSTER REPLICA and
CREATE CLUSTER ... REPLICAS), accepting disabled sizes and ignoring the
role's allowed sizes, since billing only reads the credit rate.

Make the credit lookups tolerant of a billing size that has since left
the map: such a replica counts as free with a soft panic, so it can
still be dropped. Dropping it used to hit the same expect.

Closes: SQL-658
Closes: CPU-236

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@ggevay
ggevay force-pushed the gabor/billed-as-size-validation branch from 06a3c2d to 6bf5610 Compare September 2, 2026 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-ADAPTER Topics related to the ADAPTER layer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant