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.

Sign up for GitHub

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

Jump to bottom

[Serve] Rolling update #3249

Merged
merged 95 commits into from
Mar 14, 2024
Merged

[Serve] Rolling update #3249

merged 95 commits into from
Mar 14, 2024

Conversation

Michaelvll
Copy link
Collaborator

@Michaelvll Michaelvll commented Feb 28, 2024 •
edited
Loading

This PR changes the default behavior to rolling update, and we allow the user to specify the update mode to blue_green, which is the same as the current master branch.

Some reference for the two way of update: https://blog.itaysk.com/2017/11/20/deployment-strategies-defined

This also shows the current active version in sky serve status

$ sky serve status http-server

Services
NAME         VERSION  UPTIME  STATUS  REPLICAS  ENDPOINT
http-server  1,2        10m 4s  READY   3/5       44.206.240.249:30002

Service Replicas
SERVICE_NAME  ID  VERSION  IP              LAUNCHED     RESOURCES       STATUS         REGION
http-server   1   1        54.173.203.169  10 mins ago  1x AWS(vCPU=2)  READY          us-east-1
http-server   2   1        52.87.241.103   10 mins ago  1x AWS(vCPU=2)  READY          us-east-1
http-server   3   2        3.93.241.163    1 min ago    1x AWS(vCPU=4)  READY          us-east-1
http-server   4   2        -               1 min ago    1x AWS(vCPU=4)  PROVISIONING   us-east-1
http-server   5   2        -               1 min ago    1x AWS(vCPU=4)  PROVISIONING   us-east-1

Blocked by #3299

Tested (run the relevant ones):

  • Code formatting: bash format.sh
  • Any manual or new tests for this PR (please specify below)
  • All smoke tests: pytest tests/test_smoke.py --serve
  • All smoke tests: pytest tests/test_smoke.py --managed-spot
  • Relevant individual smoke tests: pytest tests/test_smoke.py::test_fill_in_the_name
  • Backward compatibility tests: bash tests/backward_comaptibility_tests.sh

@Michaelvll Michaelvll marked this pull request as ready for review February 29, 2024 21:58
MaoZiming
MaoZiming reviewed Feb 29, 2024
View reviewed changes
sky/serve/autoscalers.py Outdated Show resolved Hide resolved
@Michaelvll Michaelvll requested review from MaoZiming and cblmemo March 12, 2024 17:56
@Michaelvll
Copy link
Collaborator Author

Tested (9f60154)

  • pytest tests/test_smoke.py --serve

MaoZiming
MaoZiming reviewed Mar 14, 2024
View reviewed changes
sky/cli.py
ready. Traffic can be mixed on old and new replicas.
- "blue_green": SkyServe will update the service with blue-green update,
i.e., it will wait for new replicas to be ready and then terminate old
replicas. Traffic will only be switched from old to new replicas after
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
replicas. Traffic will only be switched from old to new replicas after
replicas. Traffic will only be served from old to new replicas after

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It might be more clear with switched since it means we change from the old to new replicas only after the number of replicas is enough.

MaoZiming
MaoZiming reviewed Mar 14, 2024
View reviewed changes
sky/serve/core.py Outdated Show resolved Hide resolved
MaoZiming
MaoZiming reviewed Mar 14, 2024
View reviewed changes
sky/serve/replica_managers.py Outdated
@@ -513,6 +530,7 @@ def __init__(self, service_name: str,
self._next_replica_id: int = 1
self._service_name: str = service_name
self._uptime: Optional[float] = None
self._update_mode = serve_utils.UpdateMode.ROLLING
Copy link
Collaborator

Choose a reason for hiding this comment

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

shall we make this as a constant DEFAULT_UPDATE_MODE?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added. Thanks!

Michaelvll and others added 9 commits March 14, 2024 04:37
@Michaelvll Michaelvll merged commit 527b0ae into master Mar 14, 2024
19 checks passed
@Michaelvll Michaelvll deleted the rolling-update branch March 14, 2024 16:28
@Michaelvll Michaelvll mentioned this pull request Mar 14, 2024
Merged
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@concretevitamin concretevitamin concretevitamin left review comments

@MaoZiming MaoZiming MaoZiming approved these changes

@cblmemo cblmemo Awaiting requested review from cblmemo

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Michaelvll @concretevitamin @MaoZiming @cblmemo