Skip to content

[SkyServe] Cache output of sky serve status --endpoint #2915

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

Open
cblmemo opened this issue Dec 28, 2023 · 11 comments · May be fixed by #5052
Open

[SkyServe] Cache output of sky serve status --endpoint #2915

cblmemo opened this issue Dec 28, 2023 · 11 comments · May be fixed by #5052
Assignees
Labels
feature-request good first issue Good for newcomers serve features/bugs related to sky serve

Comments

@cblmemo
Copy link
Collaborator

cblmemo commented Dec 28, 2023

Currently, every time we run sky serve status --endpoint, it will contact the sky serve controller to get the endpoint. It is not friendly for a bash-based script that calls this CLI multiple times, say curl $(sky serve status --endpoint svc1)/v1/models. We should have a local database serve as a cache for the service endpoint since it will not change while the service is running.

Notice that we still use the remote database on the SkyServe controller as the only source of truth. If we don't find the service endpoint in the local cache, then we contact the SkyServe controller for the endpoint.

Copy link
Contributor

This issue is stale because it has been open 120 days with no activity. Remove stale label or comment or this will be closed in 10 days.

@github-actions github-actions bot added the Stale label Apr 27, 2024
@cblmemo cblmemo removed the Stale label Apr 27, 2024
@cblmemo cblmemo added the serve features/bugs related to sky serve label Jun 21, 2024
Copy link
Contributor

This issue is stale because it has been open 120 days with no activity. Remove stale label or comment or this will be closed in 10 days.

@github-actions github-actions bot added the Stale label Oct 20, 2024
@cblmemo cblmemo removed the Stale label Oct 20, 2024
Copy link
Contributor

This issue is stale because it has been open 120 days with no activity. Remove stale label or comment or this will be closed in 10 days.

@github-actions github-actions bot added the Stale label Feb 18, 2025
@cblmemo cblmemo removed the Stale label Feb 18, 2025
@kyuds
Copy link
Collaborator

kyuds commented Mar 18, 2025

@cblmemo just wondering if this is still relevant

@cblmemo
Copy link
Collaborator Author

cblmemo commented Mar 19, 2025

@cblmemo just wondering if this is still relevant

Yes

@kyuds
Copy link
Collaborator

kyuds commented Mar 20, 2025

@cblmemo just wondering if this is still relevant

Yes

Could I try to work on this?

@cblmemo
Copy link
Collaborator Author

cblmemo commented Mar 20, 2025

@cblmemo just wondering if this is still relevant

Yes

Could I try to work on this?

Sounds great! cc @Michaelvll for a look here

@kyuds
Copy link
Collaborator

kyuds commented Mar 20, 2025

Perfect. @cblmemo if you could assign this issue to me, I'll get started starting this weekend

@cblmemo
Copy link
Collaborator Author

cblmemo commented Mar 20, 2025

Perfect. @cblmemo if you could assign this issue to me, I'll get started starting this weekend

Done

@kyuds
Copy link
Collaborator

kyuds commented Mar 24, 2025

@cblmemo
ok so I had the opportunity to look through the SkyServe code over the weekend. This is my general plan on how to implement this issue. If this general idea looks good, then I'll go ahead with the implementation and submit a draft PR with more details on the specs:

  • The caching will be done by adding a new column to the serve_state sqlite3 database (under ~/.sky/serve/service.db). This will be named "endpoint", default to None, and will be populated by the "initial" query for status. Though, considering that the sky api server is moving to a client server architecture, api server dbs may not be local anymore, so it might be a better idea to actually create a new, local database (maybe under ~/.sky/serve/cache?) to just cache the state instead of querying the api server. I would appreciate feedback on this part as my understanding is that the local sky api server is the one currently storing state data under the ~/.sky folder
  • I am assuming that we want caching only for the --endpoint flag (correct me if I am wrong), so when we receive such a request then we will check the cache for the endpoint, and if it exists, return, else, query and store.
  • I am also thinking of adding a --force-refresh (can be renamed) flag so that users have the choice to force a re-query to the ground truth skyserve controller.

@cblmemo
Copy link
Collaborator Author

cblmemo commented Mar 25, 2025

@cblmemo ok so I had the opportunity to look through the SkyServe code over the weekend. This is my general plan on how to implement this issue. If this general idea looks good, then I'll go ahead with the implementation and submit a draft PR with more details on the specs:

  • The caching will be done by adding a new column to the serve_state sqlite3 database (under ~/.sky/serve/service.db). This will be named "endpoint", default to None, and will be populated by the "initial" query for status. Though, considering that the sky api server is moving to a client server architecture, api server dbs may not be local anymore, so it might be a better idea to actually create a new, local database (maybe under ~/.sky/serve/cache?) to just cache the state instead of querying the api server. I would appreciate feedback on this part as my understanding is that the local sky api server is the one currently storing state data under the ~/.sky folder
  • I am assuming that we want caching only for the --endpoint flag (correct me if I am wrong), so when we receive such a request then we will check the cache for the endpoint, and if it exists, return, else, query and store.
  • I am also thinking of adding a --force-refresh (can be renamed) flag so that users have the choice to force a re-query to the ground truth skyserve controller.
  • I think putting that into the service db on the api server makes sense since this cache should be shared by all users for this api server.
  • --force-refresh looks good to me.

cc @Michaelvll for a look here

@kyuds kyuds linked a pull request Mar 28, 2025 that will close this issue
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request good first issue Good for newcomers serve features/bugs related to sky serve
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants