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

Remove Exponential Backoff for Retry Until Up #2821

Merged
merged 11 commits into from
Dec 6, 2023
Merged

Remove Exponential Backoff for Retry Until Up #2821

Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
4fa6195
remove exponential backoff
rohanvaidya45 Nov 25, 2023
ffdd89d
add back random jitter
rohanvaidya45 Nov 26, 2023
02195f2
format
rohanvaidya45 Nov 26, 2023
0ecd351
use backoff
rohanvaidya45 Nov 26, 2023
406a887
format
rohanvaidya45 Nov 26, 2023
86f4e8d
add comment
rohanvaidya45 Nov 27, 2023
7dfab40
rerun checks
rohanvaidya45 Nov 29, 2023
92ae668
format
rohanvaidya45 Nov 29, 2023
dcce5b1
pylint
rohanvaidya45 Nov 29, 2023
cfd41ad
form
rohanvaidya45 Nov 29, 2023
2b2d20d
change to 30
rohanvaidya45 Dec 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions sky/backends/cloud_vm_ray_backend.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@

# Time gap between retries after failing to provision in all possible places.
# Used only if --retry-until-up is set.
_RETRY_UNTIL_UP_INIT_GAP_SECONDS = 60
_RETRY_UNTIL_UP_INIT_GAP_SECONDS = 30

# The maximum retry count for fetching IP address.
_FETCH_IP_MAX_ATTEMPTS = 3
Expand Down Expand Up @@ -2859,7 +2859,13 @@ def _provision(
# TODO(suquark): once we have sky on PyPI, we should directly
# install sky from PyPI.
local_wheel_path, wheel_hash = wheel_utils.build_sky_wheel()
backoff = common_utils.Backoff(_RETRY_UNTIL_UP_INIT_GAP_SECONDS)
# The most frequent reason for the failure of a provision
# request is resource unavailability instead of rate
# limiting; to make users wait shorter, we do not make
# backoffs exponential.
backoff = common_utils.Backoff(
Copy link
Member

Choose a reason for hiding this comment

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

Worth a comment on why we disable exponential backoff. Something like:

The most frequent reason for the failure of a provision request is resource unavailability instead of rate limiting; to make users wait shorter, we do not make the backoffs exponential.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds great

initial_backoff=_RETRY_UNTIL_UP_INIT_GAP_SECONDS,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just tried a few clouds and it seems we can also reduce this _RETRY_UNTIL_UP_INIT_GAP_SECONDS to 30. : )

max_backoff_factor=1)
attempt_cnt = 1
while True:
# For on-demand instances, RetryingVmProvisioner will retry
Expand Down Expand Up @@ -2913,7 +2919,7 @@ def _provision(
f'{colorama.Style.BRIGHT}=== Retry until up ==='
f'{colorama.Style.RESET_ALL}\n'
f'Retrying provisioning after {gap_seconds:.0f}s '
'(exponential backoff with random jittering). '
'(backoff with random jittering). '
f'Already tried {attempt_cnt} attempt{plural}.')
attempt_cnt += 1
time.sleep(gap_seconds)
Expand Down