-
Notifications
You must be signed in to change notification settings - Fork 2
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
download_sysext: HTTP client timeout and retry #42
Conversation
278002f
to
ca208c2
Compare
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.
It doesn't retry in this case: During download I deactivated my wifi and the connection was stuck because we don't have the read timeout (see issue).
When activating my wifi again the download seems to continue but directly failed without retry:
read 13926158/28371306 ( 49%)Error: unable to download data(url Url { scheme: "https", cannot_be_a_base: false, username: "", password: None, host: Some(Domain("update.release.flatcar-linux.net")), port: None, path: "/amd64-usr/3815.0.0/oem-azure.gz", query: None, fragment: None })
Caused by:
0: failed to get response chunk
1: request or response body error: error reading a body from connection: end of file before message length reached
2: error reading a body from connection: end of file before message length reached
3: end of file before message length reached
I think I see the problem: We only retry getting a response (the header) but we don't retry the full download. We need to wrap the whole function and can't split it into two parts. |
e50ddb8
to
3a9de50
Compare
As discussed, this PR is re-implemented without tryhard crate. |
src/download.rs
Outdated
bytes_read += chunk.len(); | ||
|
||
hasher.update(&chunk); | ||
data.write_all(&chunk).context("failed to write_all chunk")?; |
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.
This continuously appends, we need to truncate the file first to a length of 0 bytes before writing again.
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.
This does not retry on "Connection refused"
If we want to keep |
With #44 this will be easier |
3a9de50
to
b58a052
Compare
Brought back to a simple local wrapper version without tryhard, without passing File, without tokio, based on #44. |
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.
Thanks, works well now
Set the default timeout to 20 seconds, so HTTP client can abort after that timeout.
If HTTP client fails to download, retry to download once in 1000 msec, up to 20 times.
b58a052
to
ee93fd6
Compare
Set the default timeout to 20 seconds, so HTTP client can abort after that timeout.
If HTTP client fails to download, retry to download once in 1000 msec, up to 20 times.
Fixes #15