-
Notifications
You must be signed in to change notification settings - Fork 7.1k
[2/n] move autoscaling control to application state from deployment s… #57548
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request is a solid step in refactoring the autoscaling logic. The movement of the control loop to the application state is well-executed, and the introduction of ApplicationAutoscalingState provides a clear structure for future application-level autoscaling policies. The new tests are comprehensive and cover many edge cases, which is great to see.
I've identified a few areas for improvement:
- A bug in a test helper method that could lead to incorrect test behavior.
- An opportunity to simplify some logic using a dictionary comprehension for better readability.
- A docstring that could be updated to more accurately reflect the current implementation.
Overall, this is a high-quality contribution. Addressing these points will further enhance the clarity and robustness of the code.
akyang-anyscale
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the code lgtm, have not reviewed the tests yet
|
LGTM. |
zcin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
overall lgtm
dbf3f53 to
d7d25d2
Compare
Signed-off-by: abrar <abrar@anyscale.com>
d7d25d2 to
9c3b266
Compare
ray-project#57548) part 2 of ray-project#56149, a significant portion of the code is taken from the original PR. This PR does not introduce any change in functionality. Autoscaling is still performed at the deployment level. This will help us make the transition towards application level autoscaling. The only change in this PR 1. is moving the autoscaling control loop from the deployment state to the application state. 2. adding application autoscaling state class, in the new design autoscaling state manager will manage a list of application autoscaling states and each application autoscaling state will manage a list of deployment autoscaling states Signed-off-by: abrar <abrar@anyscale.com>
ray-project#57548) part 2 of ray-project#56149, a significant portion of the code is taken from the original PR. This PR does not introduce any change in functionality. Autoscaling is still performed at the deployment level. This will help us make the transition towards application level autoscaling. The only change in this PR 1. is moving the autoscaling control loop from the deployment state to the application state. 2. adding application autoscaling state class, in the new design autoscaling state manager will manage a list of application autoscaling states and each application autoscaling state will manage a list of deployment autoscaling states Signed-off-by: abrar <abrar@anyscale.com> Signed-off-by: xgui <xgui@anyscale.com>
#57548) part 2 of #56149, a significant portion of the code is taken from the original PR. This PR does not introduce any change in functionality. Autoscaling is still performed at the deployment level. This will help us make the transition towards application level autoscaling. The only change in this PR 1. is moving the autoscaling control loop from the deployment state to the application state. 2. adding application autoscaling state class, in the new design autoscaling state manager will manage a list of application autoscaling states and each application autoscaling state will manage a list of deployment autoscaling states Signed-off-by: abrar <abrar@anyscale.com> Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
ray-project#57548) part 2 of ray-project#56149, a significant portion of the code is taken from the original PR. This PR does not introduce any change in functionality. Autoscaling is still performed at the deployment level. This will help us make the transition towards application level autoscaling. The only change in this PR 1. is moving the autoscaling control loop from the deployment state to the application state. 2. adding application autoscaling state class, in the new design autoscaling state manager will manage a list of application autoscaling states and each application autoscaling state will manage a list of deployment autoscaling states Signed-off-by: abrar <abrar@anyscale.com>
ray-project#57548) part 2 of ray-project#56149, a significant portion of the code is taken from the original PR. This PR does not introduce any change in functionality. Autoscaling is still performed at the deployment level. This will help us make the transition towards application level autoscaling. The only change in this PR 1. is moving the autoscaling control loop from the deployment state to the application state. 2. adding application autoscaling state class, in the new design autoscaling state manager will manage a list of application autoscaling states and each application autoscaling state will manage a list of deployment autoscaling states Signed-off-by: abrar <abrar@anyscale.com> Signed-off-by: Aydin Abiar <aydin@anyscale.com>
ray-project#57548) part 2 of ray-project#56149, a significant portion of the code is taken from the original PR. This PR does not introduce any change in functionality. Autoscaling is still performed at the deployment level. This will help us make the transition towards application level autoscaling. The only change in this PR 1. is moving the autoscaling control loop from the deployment state to the application state. 2. adding application autoscaling state class, in the new design autoscaling state manager will manage a list of application autoscaling states and each application autoscaling state will manage a list of deployment autoscaling states Signed-off-by: abrar <abrar@anyscale.com> Signed-off-by: Future-Outlier <eric901201@gmail.com>
part 2 of #56149, a significant portion of the code is taken from the original PR.
This PR does not introduce any change in functionality. Autoscaling is still performed at the deployment level. This will help us make the transition towards application level autoscaling.
The only change in this PR