Repository navigation
console: configure a cluster from a drawer on the redesigned cluster page - #39992
Conversation
QA LLM Review1. MEDIUM -- Edit during a hydration burst silently raises the replication factor
The drawer's Edit opens DetailsScenario: a managed The bug is in the modal's default and also affects the classic Alter menu item. This PR adds a new entry point for it, makes it the only edit path on the redesigned page, and places it in the panel built to surface bursts. The Replicas panel copy at Fix: pass |
035c674 to
25dfeec
Compare
25dfeec to
cb64252
Compare
43f9e6a to
d1ae6a3
Compare
|
Edit during a hydration burst raises the replication factor is addressed in d1ae6a3. |
d1ae6a3 to
1bcb91e
Compare
|
Follow-up on the replication factor, in 1bcb91e: the factor now comes with each cluster from the app-wide clusters subscribe, and |
1bcb91e to
c6ae1cb
Compare
c6ae1cb to
8134f31
Compare
31f75f4 to
9210892
Compare
9210892 to
7dd1a6a
Compare
71c4b3b to
bf9719b
Compare
bf9719b to
3215409
Compare
| const Detail = ({ | ||
| label, | ||
| tooltip, | ||
| children, | ||
| }: React.PropsWithChildren<{ label: string; tooltip?: string }>) => { |
There was a problem hiding this comment.
🟡 Drawer components lack named props interfaces
New drawer components such as Detail declare props inline instead of using the required exported component-specific interfaces. The header and other panels repeat this convention violation.
Learn more
The Console component convention requires an exported, component-specific props interface above each component. The drawer uses inline prop declarations in Detail, PanelHeader, EditClusterButton, DropClusterCard, OverviewPanel, ReplicaStatusCell, ReplicasPanel, AutoScalingPanel, and ConfigurationContent; the new ClusterTitle and ConfigurationDrawer do the same. This makes the new component contracts inconsistent with the explicit repository convention.
Example: Detail currently places { label: string; tooltip?: string } inside React.PropsWithChildren. Its props have no exported DetailProps interface for a caller to import.
Recommended fix: Define and export a named props interface above each new component and use it in the component signature. Keep each existing prop's optionality and child type unchanged.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Done in 3274d31. Each drawer component, plus ClusterTitle and ConfigurationDrawer, takes a named props interface. The ones for file-private components stay unexported, as in the Workers tab's WorkerBars.tsx.
| const replicaColumnHelper = createColumnHelper<ReplicaRow>(); | ||
|
|
||
| const replicaColumns = [ |
There was a problem hiding this comment.
🟡 Replica table constants break naming convention
The new replicaColumnHelper and replicaColumns are module-level constants named in camelCase. Console rules require SCREAMING_CAPS for module-level constants.
Learn more
Console's module-level constant naming convention requires SCREAMING_CAPS. Both the column helper and column array are initialized once at module scope, and the drawer uses the array in ReplicasPanel. Renaming only the declarations leaves references inconsistent.
Example: replicaColumns is a fixed array constructed at module initialization, but its name follows the naming pattern for a local variable.
Recommended fix: Rename both constants to uppercase names and update their uses throughout the module.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Leaving these as they are. Module-level column helpers and column arrays are camelCase across the Console, columnHelper in nine tables and columns in ClusterTable.tsx and ClusterUsageTable.tsx, so renaming only these would make them the odd ones out.
| id: Generated<string>; | ||
| } | ||
|
|
||
| export interface MzClusterAutoScalingStrategies { |
There was a problem hiding this comment.
🟡 Manual catalog type lacks rollout annotation
The new MzClusterAutoScalingStrategies declaration is hand-added for version compatibility without the required cleanup TODO. Future type generation can overwrite it unnoticed.
Learn more
Console's backwards-compatibility instructions require a TODO annotation when materialize.d.ts is manually modified so subsequent code generation does not silently remove rollout compatibility work. The new relation type and database entry are hand-added while the runtime query is gated by AUTO_SCALING_MIN_VERSION.
Example: A later regeneration from a catalog snapshot without the relation could erase this interface without a nearby note explaining why it was added.
Recommended fix: Add a TODO next to the manually added declaration explaining when it can be replaced by generated types, and preserve the version gate until supported environments contain the relation.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Added in 3274d31, in the form the file already uses for hand-added relations. The query stays gated on AUTO_SCALING_MIN_VERSION.
| ` ON HYDRATION (HYDRATION SIZE = '${hydrationSize}'${ | ||
| lingerMs === undefined | ||
| ? "" | ||
| : `, LINGER DURATION = ${lingerLiteral(lingerMs)}` | ||
| })`, |
3215409 to
76965d2
Compare
4f25b2b to
af35c37
Compare
bc57f6d to
0d22ae4
Compare
QA LLM Review1. MEDIUM -- Gear does nothing on the Objects tab's Sources and Sinks lists
The drawer's open state is the Details
Reproduced at this head with a variant of Possible fixes: have |
…page Adds a gear button next to the redesigned cluster page's title. It opens a configuration drawer in the connect drawer's format, with cards for the cluster's overview, its replicas, and its autoscaling policy, and replaces the Replicas tab. Editing reuses the existing alter cluster, new replica, and drop flows.
0d22ae4 to
6e0a7dd
Compare
…y, URL params kept, props interfaces The Alter cluster modal sends SIZE and REPLICATION FACTOR only when they differ from what it opened with, and skips the SET when neither does. While a resize runs, mz_clusters reports the realized size and factor, and setting both back to them cancels the resize, so saving a rename used to cancel it. useSyncObjectToSearchParams keeps search params it didn't write. It rebuilt the whole query string, so on the Objects tab's Sources and Sinks lists it stripped the drawer's `configure` param and the gear did nothing. It still removes a key it wrote once the object drops it. The drawer's and page's components take named props interfaces, the copied autoscaling statement escapes the size, and the hand-added mz_cluster_auto_scaling_strategies type carries the usual TODO.
|
Edit during a background resize cancels the resize is fixed in 3274d31. The Alter cluster modal now sends SIZE and REPLICATION FACTOR only when they differ from the values it opened with, and leaves out the SET when neither changed, so a rename during a resize leaves its target alone. That covers the classic page's Alter cluster item too, and the modal's comment no longer reads as if resending the factor were safe mid-resize. |
|
Gear does nothing on the Objects tab's Sources and Sinks lists is fixed in 3274d31. |
…rview While a resize runs, mz_clusters, and so the overview, keeps the cluster's current size and replication factor. The drawer's request now also reads the in-progress reconfiguration's target from mz_cluster_reconfigurations, behind the same v26.34 gate as autoscaling, and the overview names the target size and replication factor in an info alert.
|
Follow-up on the background resize finding: 846bf30 adds the optional part rather than leaving it for later. The drawer's request now reads the in-progress target from |
…rm (#40043) ### Motivation Part of the cluster details redesign stack, on top of #39992. The configuration drawer shows a cluster's autoscaling policy, but changing it meant copying an `ALTER CLUSTER` statement into the SQL shell. Review feedback asked for a form. This adds the policy to the Alter cluster form. ### Description **The form.** Alter cluster gets an optional **Auto-Scaling Policy** section for managed clusters on environments with autoscaling (v26.34.0 and later): an Enable checkbox, a burst size, and a linger duration such as `15s`, `2m`, `1m 30s` or `1500ms`, empty for the environment's `default_hydration_burst_linger`. The section is a labelled group, and the checkbox is described by its explanation. An **Equivalent Code** panel shows the statements a submit will run. - The form opens on a policy it reads after opening, so a policy set or removed moments earlier is never shown stale, from the Clusters list or the configuration drawer. It diffs against the policy it was built from, so a refetch landing while it's open doesn't turn an unchanged checkbox into a change. If the policy can't be read, the section is left out rather than shown as off. - It names only what changes. Enabling a policy or renaming during a graceful resize never retargets or cancels it: any SET naming `SIZE` or `REPLICATION FACTOR` while a reconfiguration is in flight goes through `reshape_alter_cluster_managed`, where restoring the realized values cancels it (`cluster.rs` ~325-335). With nothing to change, the preview says so and a submit closes the form without a request. - The burst size defaults to the next larger size in the cluster's size family, and to nothing at a family boundary. During an in-progress resize it defaults to the size after the resize's target, and the target itself is rejected while the size is unchanged, as the server does (`HYDRATION SIZE must differ from the target SIZE ... of the in-progress cluster resize`, `cluster.rs` ~1503). A submit that also changes Size retargets the resize, and that SET takes the reshape path, which accepts the old target as the burst size. - While a resize is in flight, Size reads "Resizing to <size>." under the field. - From v26.35.0 a size change shows that the cluster keeps serving from its current replicas until the new ones catch up; earlier versions keep the downtime warning. The modal is shared, so the classic page's Alter cluster item gets the section and these fixes too. It used to send the realized `SIZE` and `REPLICATION FACTOR` on every submit, which cancels an in-flight resize. The flag behind the SQL, `enable_auto_scaling_strategy`, defaults on and has no LaunchDarkly override. **The SQL.** Each statement commits on its own, in order: - **Settings:** one `ALTER CLUSTER ... SET (...)` naming only the changed `SIZE`, `REPLICATION FACTOR` and `AUTO SCALING STRATEGY`. A policy is `AUTO SCALING STRATEGY = (ON HYDRATION (HYDRATION SIZE = '...'[, LINGER DURATION = '...']))`. - **Removing a policy with other changes** joins the same SET as `AUTO SCALING STRATEGY = ()`, so a SET the server rejects (a size that isn't allowed, a replica or credit limit) leaves the policy in place. The planner checks the new size against the effective config, so resizing to the old burst size in the same statement passes. A removal on its own is `ALTER CLUSTER ... RESET (AUTO SCALING STRATEGY)`, which also works with the feature flag off. Neither cancels an in-flight resize. - **Rename:** `ALTER CLUSTER ... RENAME TO ...` only for a new name; a same-name rename writes an audit event. - **Reads:** the form reads `mz_internal.mz_cluster_auto_scaling_strategies` and the in-progress target from `mz_internal.mz_cluster_reconfigurations` (`status = 'in-progress'`, `target->>'size'`) in one request. Both are fast-path index lookups on `mz_catalog_server`, and both shipped in v26.34.0. The request reads at strict serializable: the form makes it right after an ALTER, and a serializable read can come back from before the write. **Self-managed.** Self-managed has no LaunchDarkly, so a flagged UI preview ships there outright. A preview can now set `selfManagedOptIn`, which offers the same per-browser opt-in there: the profile menu's UI previews and the page pill. The cluster page redesign sets it, and `cluster-details-page-cns-173` leaves `disabledFlexibleDeploymentFlags`, so self-managed users get the classic page until they opt in. The cluster list preview still ships outright. **The drawer.** The Auto-Scaling panel's copyable SET and RESET statements become an **Edit** action that opens the form at the auto-scaling section. The form does its own post-mount read, since it also needs the resize target, so Edit waits only for the cluster sizes. ### Verification - `alterCluster.test.sql.ts`, against a real Materialize: - setting, replacing and keeping a policy; - a rejected removal-plus-resize leaves the policy in place, and removing a policy while resizing to its burst size succeeds in one statement; - during an in-flight resize, the target is read, a burst equal to it is rejected, and setting a policy, removing it and renaming all leave the resize running until it finalizes at the new size; - one SET retargets an in-flight resize and sets its old target as the burst size; - settled or absent resizes read no target. - `clusterConfiguration.test.sql.ts`: policies with and without a linger, parsed as the drawer parses them. - `alterCluster.test.ts`: statements for size-only, replicas-only, strategy-only, removal with and without other changes, rename-only and nothing changed. - `autoScalingForm.test.ts`: linger parsing and display, size families, the default burst size with and without an in-progress resize, and when a submit sets, removes or keeps the policy. - `AlterClusterModal.test.tsx`: - the reopen sequence that used to revert a just-set or just-removed policy now shows it and sends only the change; - props changing under an open form; - no request when nothing changed, and the preview without `NaN` while Replicas is empty; - the in-progress resize's target under Size; - errors for an invalid linger, a burst equal to the cluster's size, and one equal to an in-progress resize's target; - the labelled group, the version-gated resize copy, no section before v26.34.0, for unmanaged clusters, or when the policy can't be read. - `useUiPreview.test.tsx`: the cluster page redesign is offered in self-managed, off until the user opts in. - `ClusterConfigurationDrawer.test.tsx`: the Auto-Scaling panel's Edit action, and Edit waiting only for the sizes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- devin-review-badge-begin --> --- <a href="https://app.devin.ai/review/materializeinc/materialize/pull/40043" target="_blank"><picture><source media="(prefers-color-scheme: dark)" srcset="https://static.devin.ai/assets/gh-devin-review-dark.svg?v=4"><img src="https://static.devin.ai/assets/gh-devin-review-light.svg?v=4" alt="Devin Review"></picture></a> <!-- devin-review-badge-end -->
MaterializeInc#39992 typed mz_cluster_auto_scaling_strategies and mz_cluster_reconfigurations by hand while gen:types was broken. The generator now emits both, and AUTO_SCALING_MIN_VERSION gates them at runtime, so the file is exactly the generator output.
### Motivation `yarn gen:types` fails with `function "pg_get_serial_sequence" does not exist`. kysely-codegen runs kysely's Postgres introspector for its table query. Since the bump to kysely 0.28 (#35669), that query calls `pg_get_serial_sequence`, which Materialize lacks. kysely 0.29 still does. The query also lists only relkinds `r`, `v` and `p`, so the catalog's builtin materialized views (`m`) never reached the types under kysely-codegen 0.17. #40044's types were generated with both patched by hand in `node_modules`. This also bumps kysely to 0.29 and kysely-codegen to 0.20, superseding #39686. ### Description - `bin/kysely-codegen.ts` runs kysely-codegen programmatically with an introspector that overrides only the table query. kysely-codegen's materialized view query, enum handling and serializer still apply. - The script writes the copyright header to `types/materialize.d.ts`. The codegen output lacks it, and the copyright lint fails without it. - `bin/generate-types` exits on errors, pulls the requested image (a cached `latest` went stale), and stops the container on exit. Before, each run left a container bound to port 6875, and the next run generated types from that old container. - kysely 0.28.17 → 0.29.8 and kysely-codegen 0.17 → 0.20.0: - 0.29 rejects `limit(sql.raw(...))`. `rawLimit` and the lag history limit now call `query.limit(sql.lit(n))`, which still inlines the limit as a literal, as Materialize requires. - `rawLimit` used to append its limit with `modifyEnd`. In 0.29, compiling end modifiers calls `Array.prototype.toSorted`, which Chrome 109 lacks. No console query uses end modifiers now. Neither `rawLimit` caller has an OFFSET or another end modifier, so the compiled SQL is unchanged. - kysely 0.29 is ESM-only and kysely-codegen 0.20 `require`s it, so `yarn gen:types` needs Node 22.12 or later. CI and the Volta pin run Node 26, and #39683 updates `engines.node`. - `types/materialize.d.ts` is regenerated against v26.49.1. kysely-codegen 0.17 and 0.20 produce identical output. - Adds `mz_builtin_columns` and `mz_builtin_index_columns`. - Builtin columns lose their `Generated<>` wrappers, because #40316 changed `pg_attribute.atthasdef` to report no default for them. The console only selects from these relations, where `Generated<T>` and `T` read the same. - Drops the two hand-added `TODO` markers from #39992. The generator now emits `mz_cluster_auto_scaling_strategies` and `mz_cluster_reconfigurations`, and `AUTO_SCALING_MIN_VERSION` still gates them at runtime. The file is now exactly the generator output. Worth checking on the preview, since these run the changed limits: - the Query History list; - a sink's Errors tab; - the freshness lag charts on the environment overview and the cluster pages. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Motivation
Closes: CNS-204
Seventh PR in the cluster details redesign stack, on top of #39889. The redesigned cluster page kept the classic page's Replicas tab as a stopgap, and its only way to edit or drop the cluster was the breadcrumb's overflow menu. This moves a cluster's configuration into a drawer opened from the page header, in the same format as the Connect drawer, and drops the Replicas tab.
Description
Opening it. A gear button next to the cluster's title, after its size and replica pills, opens Configure ‹cluster›.
configuresearch parameter (overview,replicasorauto-scaling), so it survives a reload and can be linked.useSyncObjectToSearchParams, which lists such as the Objects tab's Sources and Sinks use to put their sort in the URL, now keeps search parameters it didn't write, soconfiguresurvives on every tab. It still removes a key it wrote once its object drops it, such as a cleared filter.…/replicas, redirects to?configure=replicas.Layout. It's a 640px side drawer, like the Connect drawer, with a row of
ConnectMethodCards that switch the panel below:mz_clusters, and so the details, keep the current values until it finishes.mz_clustersreports the realized size and factor, and setting both back to them cancels the resize, so saving a rename used to cancel it. Both fix the classic page's Alter cluster item too.mz_internal.mz_cluster_auto_scaling_strategiesfirst shipped.ALTER CLUSTER … SET (AUTO SCALING STRATEGY = …)andRESETstatements to change or remove the policy, or the statement to turn one on. The linger is written as whole seconds where it is one, and in milliseconds otherwise, so copying the statement keeps a 0s or 1.5s linger. System clusters get none, since they can't be altered.Data. The cluster list already carries name, size, management, the replication factor (added to its subscribe here), owner id and replica statuses, and
useOwnersalready has every role's name. Everything else comes from oneexecuteSqlV2request when the drawer opens:mz_object_lifetimes;mz_cluster_reconfigurations, which shipped with the autoscaling relation in v26.34.0.The request reads at strict serializable, the session default. A serializable read can come back from before a write that just committed, and since the drawer doesn't poll, a replica added or a policy altered moments earlier could stay missing until the drawer reopened.
Cluster and replica ids share the
u<N>form but come from separate counters, so a cluster's id can also be a replica's.buildObjectCreationTimesQuerynow takes the object types to read and returns each row'sobject_type, and the drawer matches creation times on type and id.When the replica set changes, for example a resize or a burst replica coming and going, the request's key changes. The drawer keeps showing the previous answer for the same cluster until the new one arrives, rather than blanking the overview.
mz_cluster_auto_scaling_strategiesandmz_cluster_reconfigurationsare added totypes/materialize.d.tsby hand, matching the generated shape, with the file's TODO for hand-added relations.The owners, the replica limit and, where Edit can show, the sizes are read once by the drawer and passed to its sections, so switching sections makes no requests.
buildObjectCreationTimesQueryrequires its object types, and the earlier caller passes its two explicitly.Also: a
SettingsIconin the icon set, and anownerNamelookup onuseOwners.Verification
clusterConfigurationModel.test.ts:ClusterConfigurationDrawer.test.tsx, with creation-time fixtures where each id is also an object of the other type:ClusterDetail.test.tsx:useSyncObjectToSearchParams.test.ts: a search parameter the hook didn't write survives, and one it wrote goes once its object drops it.queries.test.ts:useOwnersnames an owner by its role id.clusterConfiguration.test.sql.ts, against a real Materialize: creation times told apart for a cluster and a replica that share an id, and autoscaling policies, with and without a linger, parsed as the drawer parses them, and no resize target once a resize settles or for a cluster never resized.clusterList.test.sql.tsreads a managed cluster's replication factor and none for an unmanaged one, and the clusters subscribe tests expect the new column.AlterClusterModal.test.tsx: the Replicas field starts at the replication factor, not a running burst's replica count; a rename sends only the RENAME; a replication factor change sends no SIZE.🤖 Generated with Claude Code