[UX - SkyServe] user now is able to select a LB policy from a range of options #4061
Conversation
There was a problem hiding this comment.
This is awesome @AlexCuadron ! It will greatly expand our customizability. Left some discussion ;) We might also think of how to expose this feature to our end user, in our Service YAML - maybe add a load_balancing_policy section under service?
|
@cblmemo Done! PTAL again :D |
There was a problem hiding this comment.
Thanks @AlexCuadron for the awesome work! It mostly looks good to me. Left some discussions ;)
|
Thanks for the comments @cblmemo, fixed and ready for next round 💪 |
There was a problem hiding this comment.
Thanks for the quick fix @AlexCuadron ! Mostly looks good to me. Left some discussions.
btw, I created a branch here and lets merge our PR to the branch first. Merging into master might need more time and we want to move fast ;)
https://github.com/skypilot-org/skypilot/tree/heterogeneous-lb
|
I changed the base branch and updated based on comments :) |
There was a problem hiding this comment.
Thanks for the prompt fix @AlexCuadron ! Mostly looks good to me. Left some nits :))
|
Done! PTAL @cblmemo :D |
There was a problem hiding this comment.
Thanks for the prompt fix @AlexCuadron ! Left some final nits 🚀
Co-authored-by: Tian Xia <cblmemo@gmail.com>
* [Catalog] Silently ignore TPU price not found. * assert for non tpu v6e * format
* Change V100 to H100 * updates * update
Fix GPU labelling
Handle @ in context name
* [Performance] Refactor Azure SDK usage * lazy import and address comments * address comments
|
Done! PTAL! @cblmemo |
|
@cblmemo or @Michaelvll What changes would be needed for this PR to be merged into the master branch? That way, we won’t need to keep updating the #heterogeneous-lb branch. |
There was a problem hiding this comment.
Actually, for exposing LB_POLICIES, pls check the comments. And I do think this should be able to merge to master - cc @Michaelvll for a look. @AlexCuadron I've changed the base branch to master, could you help merging the latest one?
|
I have fixed it according to comments @cblmemo. PTAL @Michaelvll :) |
I added the option to specify different LB policies, by default, RoundRobin is used. This PR is intended to enable easy switching between LB policies to facilitate its development.
The default behaviour (without user interaction) doesn't modify the execution flow and the user is not allowed to use any other LB policy other than round-robing without them being added explicitly.
Tested (run the relevant ones):
bash format.shpytest tests/test_smoke.pypytest tests/test_smoke.py::test_fill_in_the_nameconda deactivate; bash -i tests/backward_compatibility_tests.sh