-
Notifications
You must be signed in to change notification settings - Fork 13
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
Use incremental backoff instead of hardcoding fixed sleep times #340
Use incremental backoff instead of hardcoding fixed sleep times #340
Conversation
fca4f9b
to
b7de427
Compare
run-until-success-with-backoff
Outdated
max_retries=20 | ||
until "$@"; do | ||
((i+=1)) | ||
if [[ $i == "$max_retries" ]]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we start printing messages as soon as something takes longer than 10 seconds, to give some additional visibility to things that are slow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
run-until-success-with-backoff
Outdated
echo "$@" "still failing after $max_retries retries" | ||
exit 1 | ||
fi | ||
sleep $i |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to move the sleep before the first attempt so we're giving some cpu time to what we're willing to wait for here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
run-until-success-with-backoff
Outdated
set -x | ||
|
||
i=0 | ||
max_retries=20 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With max_retries 20, we're going from 5 seconds to about 200s. That's x 40.
max_retries 10 brings us to about 50s. x 10 seems plenty.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea. Done.
run-until-success-with-backoff
Outdated
|
||
set -euo pipefail | ||
|
||
set -x |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should leave this out to avoid more output in the test, and let this helper print useful messages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
b7de427
to
e15c0c2
Compare
With fixed/long sleep times, tests run slower when there is no need to wait for that long. When the machine is slow (or under heavy load), we often to need to wait for longer before things are running and the fixed sleep times are not sufficient.
e15c0c2
to
d8af8e8
Compare
I am merging this because the insufficient-sleep issue is making it much more difficult to get passing tests for me. If there are any outstanding concerns with this PR, please let me know and we can work through them in a separate PR. TIA. |
With fixed/long sleep times, tests run slower when there is no need to wait for that long. When the machine is slow (or under heavy load), we often to need to wait for longer before things are running and the fixed sleep times are not sufficient.