Skip to content
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

[SkyServe] Support min_replicas = 0 #2938

Merged
merged 10 commits into from
Jan 6, 2024
Merged

[SkyServe] Support min_replicas = 0 #2938

merged 10 commits into from
Jan 6, 2024

Conversation

MaoZiming
Copy link
Collaborator

@MaoZiming MaoZiming commented Jan 3, 2024

Related issue: #2893
Add a new status NO_REPLICAS.
Lower autoscaler decision interval when target_num_replicas = 0

Tested (run the relevant ones):

  • Code formatting: bash format.sh
  • sky serve up examples/serve/min_replicas_zero.yaml

@MaoZiming MaoZiming changed the title [SkyServe] support min_replicas = 0 [SkyServe] Support min_replicas = 0 Jan 3, 2024
Copy link
Collaborator

@cblmemo cblmemo left a comment

Choose a reason for hiding this comment

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

Thanks for adding this feature! Left some comments 🫡

sky/cli.py Outdated Show resolved Hide resolved
sky/serve/autoscalers.py Outdated Show resolved Hide resolved
sky/serve/controller.py Outdated Show resolved Hide resolved
sky/serve/serve_state.py Show resolved Hide resolved
@MaoZiming
Copy link
Collaborator Author

MaoZiming commented Jan 4, 2024

@cblmemo Thanks for the reviews! PTAL

Copy link
Collaborator

@cblmemo cblmemo left a comment

Choose a reason for hiding this comment

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

Thanks for the quick fix! Some more comments ;)

btw, could you update the tests you run? Please make sure the case mentioned in #2893 does not happen after this PR 🫡

sky/serve/controller.py Outdated Show resolved Hide resolved
sky/serve/serve_state.py Outdated Show resolved Hide resolved
@MaoZiming
Copy link
Collaborator Author

@cblmemo Thanks! Added an example yaml. Checked that a curl will trigger immediate scale up

Copy link
Collaborator

@cblmemo cblmemo left a comment

Choose a reason for hiding this comment

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

Thanks for the quick fix! It is mostly ready for me, except for some nits.

examples/serve/min_replicas_zero.yaml Outdated Show resolved Hide resolved
sky/serve/autoscalers.py Outdated Show resolved Hide resolved
sky/serve/serve_state.py Outdated Show resolved Hide resolved
Comment on lines +5 to +6
# The endpoint will be printed in the console.
# Querying the endpoint will trigger a scale up.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add docstr for how to make the replicas to 0, i.e. don;t send any traffic for how many time

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks. updated

sky/serve/serve_state.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@cblmemo cblmemo left a comment

Choose a reason for hiding this comment

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

LGTM!

@MaoZiming MaoZiming merged commit a31fdd9 into master Jan 6, 2024
19 checks passed
@MaoZiming MaoZiming deleted the num_min_zero branch January 6, 2024 20:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants