Skip to content

remove replicas fields of CN Statefulset when hpa is enabled #247

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

yandongxiao
Copy link
Collaborator

No description provided.

@yandongxiao yandongxiao changed the title remove replicas fields of CN when hpa is enabled remove replicas fields of CN Statefulset when hpa is enabled Aug 29, 2023
@@ -166,6 +166,9 @@ starrocksFESpec:

# spec for compute node, compute node provide compute function.
starrocksCnSpec:
# number of replicas to deploy for cn component.
# we should not set default replicas, because if hpa is enabled, the replicas should be unset.
Copy link
Collaborator

Choose a reason for hiding this comment

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

can this be done in operator? if hpa is provided, the replica setting will be ignored no matter if it is provided or not in the CR

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The operator will eliminate replicas from the CN Statefulset.
However, if the HPA section is removed from the CN Spec, the default replicas will come into play, which is undesirable.

Copy link
Collaborator

Choose a reason for hiding this comment

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

then what's the exact behavior?

If replica is provided, it will take precedence over HPA settings?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In the implementation of the operator: when both the replica and HPA are set in the CN Spec, the replicas field of the cn statefulset created by the operator will be set to null. This ensures that the replicas field is controlled by the HPA. Next, if a user's deployment, for example, does not involve changes to CN, then the Pod will not be deleted and recreated. However, if the user removes the hpa part from the CN Spec, the corresponding hpa object should be deleted, and if the replicas in the CN Spec still remain null, the pod will not be recreated.

If we set a default value for replicas in the chart, once the user removes the hpa part of the CN Spec, the number of pods will immediately revert to the replicas count, even though the user did not specify the replicas count in their values.yaml.

Copy link
Collaborator

@kevincai kevincai Aug 31, 2023

Choose a reason for hiding this comment

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

it is fine we don't set the default value for the replica, it is the implementation detail that when the user's values.yaml provides both replica and hpa setting, what the exact behavior will be.

And we need to describe the accurate behavior, right now the statement because if hpa is enabled, the replicas should be unset. is missleading.

@kevincai kevincai merged commit 51216ce into StarRocks:main Sep 1, 2023
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.

None yet

2 participants