Skip to content

Add scalability_info field to ClusterState#84

Merged
areina merged 1 commit into
mainfrom
feat/toni/add-scalable-field-to-cluster
May 30, 2025
Merged

Add scalability_info field to ClusterState#84
areina merged 1 commit into
mainfrom
feat/toni/add-scalable-field-to-cluster

Conversation

@areina
Copy link
Copy Markdown

@areina areina commented May 20, 2025

Add a field that indicates whether the cluster can be scaled up or down.

Instead of using a boolean field, which isn't extensible, we're adding a new message to represent information about the scalability of the cluster. At this moment the message contains the status and the reason for that status.

@areina
Copy link
Copy Markdown
Author

areina commented May 20, 2025

@Rendez I think you were the original author of the /:cluster_id/scalable endpoint. For the new API, we are considering adding it as a property of the cluster state instead of as a method. We would like to get your point of view on this and also understand if it could have any downside for the existing use cases.

Additionally, we would prefer to take a more extensible approach, adding a ClusterScalability message instead of using a boolean field. That would allow us to add more information regarding the scaling options for a cluster. For instance, with scalable it isn't clear to me if that means whether the cluster can be scaled up or down. Is there currently more logic in the UI about the scaling of a cluster that could be offered by the API?

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 20, 2025

The latest Buf updates on your PR. Results from workflow Pull Request / linting (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed✅ passedMay 29, 2025, 2:24 PM

Add a field which indicates whether the cluster can be scaled up or down.
@areina areina force-pushed the feat/toni/add-scalable-field-to-cluster branch from a24ba0f to 03cf38e Compare May 29, 2025 14:23
@areina
Copy link
Copy Markdown
Author

areina commented May 29, 2025

I had a discussion with @Rendez the other day, and the main reason to have a specific endpoint to retrieve this information was that the existing endpoint used to retrieve the cluster is expensive to compute and includes more fields than the necessary. I believe we can address both of these issues in the new api, and it's better to include this information as a field of the cluster rather than expose it through a separate method. I've updated the PR accordingly.

@areina areina marked this pull request as ready for review May 29, 2025 14:49
@areina areina requested a review from a team as a code owner May 29, 2025 14:49
@areina areina changed the title Discussion: Add scalable field to ClusterState Add scalability_info field to ClusterState May 29, 2025
// ClusterScalabilityInfo provides information about the
message ClusterScalabilityInfo {
// The current scalability status of the cluster.
ClusterScalabilityStatus status = 1;
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.

I don't see how we can e.g.

  • make it possible to scale up, but not down
  • make it possible to scale up to e.g. 16 nodes
  • to scale down a 10 cluster to 8 (not smaller).

Copy link
Copy Markdown
Contributor

@Robert-Stam Robert-Stam left a comment

Choose a reason for hiding this comment

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

LGTM

@areina areina merged commit 1199a2a into main May 30, 2025
16 checks passed
@areina areina deleted the feat/toni/add-scalable-field-to-cluster branch May 30, 2025 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants