retry mechanism addition to download bitcoind tarball #1393
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1393 +/- ##
=======================================
Coverage 19.08% 19.08%
=======================================
Files 166 166
Lines 11066 11066
=======================================
Hits 2112 2112
Misses 8954 8954
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
GitGab19
force-pushed
the
add-retry-in-tp-tarball-download
branch
from
9ec4466 to
979b1d5
Compare
|
overall the PR looks good, but the commit title seems a bit confusing... was that intentional? |
|
Why do we need to specify retries? I think a better approach would be to add a timeout, by default, to the function |
GitGab19
force-pushed
the
add-retry-in-tp-tarball-download
branch
from
979b1d5 to
8a60aad
Compare
I did it because I thought that maybe it could be useful for some other future uses. Better than having a default value.. |
GitGab19
force-pushed
the
add-retry-in-tp-tarball-download
branch
from
8a60aad to
f01545c
Compare
If 503 error code is received then the server is probably overloaded. So if we are doing 10 retries in 3 seconds for example, it is not really helpful, but if we say this function run for 30 seconds and make a request every 3 seconds, it is a big enough time-frame to wait for the server to come up but in the same time not overload it with requests. |
|
I agree with you, but there's an incremental delay in this PR to manage that. It works in this way:
|
| "URL {} didn't return 200", | ||
| download_url | ||
| fn download_bitcoind_tarball(download_url: &str, retries: usize) -> Vec<u8> { | ||
| for attempt in 0..retries { |
There was a problem hiding this comment.
| for attempt in 0..retries { | |
| for attempt in 1..=retries { |
| } | ||
| } | ||
|
|
||
| if attempt < retries - 1 { |
There was a problem hiding this comment.
| if attempt < retries - 1 { | |
| if attempt < retries { |
If above suggestion is accepted.
| } | ||
|
|
||
| if attempt < retries - 1 { | ||
| let delay = 2u64.pow(attempt as u32); |
There was a problem hiding this comment.
| let delay = 2u64.pow(attempt as u32); | |
| let delay = 1u64 << (attempt - 1); |
Close #1392