[Core] Improve performance of `sky api status -a` #7576

DanielZhangQD · 2025-10-10T08:19:38Z

Improve performance of sky api status -a

Add --limit option and retrieve 50 requests by default
Reduce the db query for user name from O(N) to O(1)
Retrieve the required fields only that are used by the client

Tested (run the relevant ones):

Code formatting: install pre-commit (auto-check on commit) or bash format.sh
Any manual or new tests for this PR (please specify below)
All smoke tests: /smoke-test (CI) or pytest tests/test_smoke.py (local)
Relevant individual tests: /smoke-test -k test_name (CI) or pytest tests/test_smoke.py::test_name (local)
Backward compatibility: /quicktest-core (CI) or pytest tests/smoke_tests/test_backward_compat.py (local)

DanielZhangQD · 2025-10-10T09:01:25Z

Function test:

New server + new client

sky api status
sky api status -a -v
sky api status -a -l none
sky api status f6ce5c8b-1881-4d3a-a359-e24d3c0b439e

New server + old client

sky api status
sky api status -a -v
sky api status xxxx-1881-4d3a-a359-e24d3c0b439e

Old server + new client

sky api status
sky api status -a -v
sky api status -a -l none
sky api status f6ce5c8b-1881-4d3a-a359-e24d3c0b439e

Performance test - with 100000 requests in DB:

Data fetched, new client 33MB vs old client 128 MB, 74% reduced
Request duration

Commands	New server + new client	Old server + old client	Result
multitime -n 5 sky api status > /dev/null	0.691	0.772	No visible difference
multitime -n 5 sky api status -a -v > /dev/null	0.969(50 requests)	19.081(all requests)
multitime -n 5 sky api status -a -l none > /dev/null	8.747(all requests)	19.081(all requests, old client does not support -l, take the duration of `-a -v`)	Duration reduced about 54%
multitime -n 5 sky api status a3282df8-45d6-4ed2-8e97-0bd29f2d6981 > /dev/null	0.697	0.828	No visible difference

Resource consumption
Total memory reduced about 33%, worker memory reduced about from 55%~65%
Old version server: dan-api-server-56c4f448dc-6h925
New version server: dan-api-server-6787c474c-5ptl5, load running from about 09:40

DanielZhangQD · 2025-10-10T09:01:32Z

/quicktest-core
Passed - https://buildkite.com/skypilot-1/quicktest-core/builds/1530#_

DanielZhangQD · 2025-10-11T02:08:11Z

/quicktest-core

SeungjinYang

Logic LGTM, just a few UX related comments

SeungjinYang · 2025-10-11T02:55:49Z

sky/server/requests/requests.py

+    all_users = global_user_state.get_all_users()
+    all_users_map = {user.id: user.name for user in all_users}
+    for request in requests:


Here, could we first get all usernames that appear in the list of requests at least once and query on just those users? For teams with large users we'd expect only a small subset of them to be running active requests at a given time, and also because we did historically have some bug related to user duplication that can make all_users list particularly large.

This is mainly for sky status -a, where there would be many request records, like hundreds of thousands or more. In this case, it would take more time to go through all the request records and probably get almost all the users. So I would prefer to keep the current logic here.

SeungjinYang · 2025-10-11T03:31:11Z

sky/client/cli/command.py

+@click.option(
+    '--limit',
+    '-l',
+    default=_NUM_REQUESTS_TO_SHOW,
+    type=int,
+    required=False,
+    help=(f'Number of requests to show, default is {_NUM_REQUESTS_TO_SHOW},'
+          f' set to 0 to show all requests.'))


Could we merge this PR with all of the support for limits except defaulting to _NUM_REQUESTS_TO_SHOW (i.e. keep default as is, showing everything)? I'm just worried some people might be relying on this behavior (i.e. sky api status | grep <request-id>) for whatever use case they might have.

Cases like sky api status | grep <request-id> should be fine to show all requests since the number of running requests is limited. But I'm a bit concerned that sky api status -a | grep <request_id>, which has unlimited historical requests.

I'm thinking of have different handling of -l with -a and no-a cases, but this seems dirty.

To keep things simple, how about we keep the -l limit and print a warning to the stderr if the number of requests exceeds the default -l limit to hint user use pagination or sky status <request_id>?

Updated to use the default limitation, -l none or -l all to show all requests, and added a message to notify the user for more requests.

SeungjinYang · 2025-10-11T03:31:45Z

sky/client/sdk.py

    # pylint: disable=redefined-builtin
-    all_status: bool = False
+    all_status: bool = False,
+    request_limit: int = 0,


I prefer this value be an Optional[int] where None = return everything, instead of 0 = return everything.
Also I think naming this field just limit should be fine

SeungjinYang · 2025-10-11T03:32:43Z

sky/server/requests/payloads.py

    """The request body for the API request status endpoint."""
    request_ids: Optional[List[str]] = None
    all_status: bool = False
+    request_limit: int = 0


Also here

Suggested change

request_limit: int = 0

limit: Optional[int] = None

where API defaults to sending back everything

aylei · 2025-10-11T03:15:44Z

sky/server/requests/requests.py

+    # Valid empty values for pickled fields (base64-encoded pickled None)
+    # base64.b64encode(pickle.dumps(None)).decode('utf-8')
+    empty_pickled_value = 'gAROLg=='


Feel a bit hacky, can we abstract this a bit or make it a magic constant?

aylei · 2025-10-11T03:33:47Z

sky/server/requests/requests.py

+    empty_pickled_value = 'gAROLg=='
+
+    # Required fields in RequestPayload
+    if 'request_id' not in fields:


Can we loop over the table definition instead of duplicate it here?

Different fields are handled differently here, we need to know the exact field name.

aylei · 2025-10-11T03:35:13Z

docs/source/reference/async.rst

+    $ # List finished and ongoing requests, default to show 50 requests
+    $ # add `--limit 0` to show all finished and ongoing requests
    $ sky api status -a


Shall we support pagination?

Usually, the CLI needs to use cursor-based pagination, which would require more refactoring work, we can support this later if necessary.

Traced in #7586

DanielZhangQD · 2025-10-11T08:18:00Z

/quicktest-core

DanielZhangQD · 2025-10-11T08:22:04Z

/smoke-test --kubernetes -k test_api_server_start_stop
/smoke-test --kubernetes -k test_requests_scheduling

DanielZhangQD · 2025-10-11T08:44:40Z

/smoke-test --kubernetes -k test_api_server_start_stop
/smoke-test --kubernetes -k test_requests_scheduling
/quicktest-core

aylei

LGTM, thanks @DanielZhangQD !

DanielZhangQD marked this pull request as ready for review October 10, 2025 11:30

DanielZhangQD requested review from Michaelvll and aylei and removed request for Michaelvll October 10, 2025 11:30

DanielZhangQD marked this pull request as draft October 10, 2025 13:22

SeungjinYang self-requested a review October 10, 2025 18:31

DanielZhangQD marked this pull request as ready for review October 11, 2025 02:14

SeungjinYang reviewed Oct 11, 2025

View reviewed changes

aylei reviewed Oct 11, 2025

View reviewed changes

DanielZhangQD mentioned this pull request Oct 11, 2025

[Core] Support pagination for sky api status #7586

Open

DanielZhangQD added 5 commits October 11, 2025 16:43

improve performance of sky api status -a

427e59e

add ut

12ffd11

update docs

2659ea0

retrieve required fields only from db

52aab86

address comments

e4b16e0

DanielZhangQD force-pushed the 3521 branch from b6b2bad to e4b16e0 Compare October 11, 2025 08:43

aylei approved these changes Oct 11, 2025

View reviewed changes

DanielZhangQD merged commit 9f28ca4 into master Oct 11, 2025
22 checks passed

DanielZhangQD deleted the 3521 branch October 11, 2025 09:07

This was referenced Oct 13, 2025

[db] Request database optimizations #7602

Closed

[db] only query for specific fields of requests #7658

Merged

[Core] Improve performance of sky api status -a #7576

[Core] Improve performance of sky api status -a #7576

Uh oh!

Conversation

DanielZhangQD commented Oct 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DanielZhangQD commented Oct 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DanielZhangQD commented Oct 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DanielZhangQD commented Oct 11, 2025

Uh oh!

SeungjinYang left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DanielZhangQD commented Oct 11, 2025

Uh oh!

DanielZhangQD commented Oct 11, 2025

Uh oh!

DanielZhangQD commented Oct 11, 2025

Uh oh!

aylei left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

[Core] Improve performance of `sky api status -a` #7576

[Core] Improve performance of `sky api status -a` #7576

DanielZhangQD commented Oct 10, 2025 •

edited

Loading

DanielZhangQD commented Oct 10, 2025 •

edited

Loading

DanielZhangQD commented Oct 10, 2025 •

edited

Loading