Skip to content

Reverted poll interval when waiting request status #6947

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
Merged
aylei merged 2 commits into from
Aug 29, 2025
Merged

Reverted poll interval when waiting request status #6947

aylei merged 2 commits into from
Aug 29, 2025

Conversation

@aylei
Copy link
Collaborator

@aylei aylei commented Aug 29, 2025 •
edited
Loading

Increase the poll interval in #6889 introduce high latency for short requests, should consider other potential solution to reduce the CPU usage.

Master:

multitime -n 5 sky status
            Mean        Std.Dev.    Min         Median      Max
real        3.889       0.163       3.656       3.831       4.117
user        1.084       0.136       0.881       1.122       1.277
sys         0.743       0.197       0.518       0.670       1.103

This PR:

multitime -n 5 sky status
1: sky status
            Mean        Std.Dev.    Min         Median      Max
real        1.610       0.261       1.353       1.459       2.025
user        0.841       0.085       0.775       0.785       1.000
sys         0.811       0.151       0.618       0.763       0.993

Before #6889

multitime -n 5 sky status
1: sky status
            Mean        Std.Dev.    Min         Median      Max
real        1.981       0.894       1.251       1.598       3.702
user        1.023       0.216       0.857       0.922       1.438
sys         0.737       0.093       0.658       0.674       0.880

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)

aylei added 2 commits August 29, 2025 15:57
@aylei
Revert sleep duration
12892bc 
Signed-off-by: Aylei <rayingecho@gmail.com>
@aylei
Revert debug change
50c6667 
Signed-off-by: Aylei <rayingecho@gmail.com>
@aylei aylei marked this pull request as ready for review August 29, 2025 08:05
Michaelvll
Michaelvll approved these changes Aug 29, 2025
View reviewed changes
Copy link
Collaborator

@Michaelvll Michaelvll 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 @aylei!

sky/server/stream_utils.py
current_time - last_refresh_time >= _REFRESH_INTERVAL):
# Refresh the request status from DB every _REFRESH_INTERVAL
last_refresh_time = current_time
if request_id is not None:
Copy link
Collaborator

@Michaelvll Michaelvll Aug 29, 2025

Choose a reason for hiding this comment

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

This is likely causing the largest delay in the benchmark. Not sure how much difference the 0.1 and 0.5 above causes.

@aylei aylei merged commit 27f74c7 into master Aug 29, 2025
16 checks passed
@aylei aylei deleted the revert-sleep branch August 29, 2025 08:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Michaelvll Michaelvll Michaelvll approved these changes

@SeungjinYang SeungjinYang Awaiting requested review from SeungjinYang

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@aylei @Michaelvll