Fix long timeout when loosing connection during app download #3236
Conversation
|
What is "eve 413 3"? And what is the description of the issue? |
it is still wip ;-) |
|
Ah, just a placeholder for you to recall? Got it. |
christoph-zededa
changed the title
christoph-zededa
force-pushed
the
fix_eve-413-3
branch
3 times, most recently
from
702d882 to
136ea75
Compare
christoph-zededa
force-pushed
the
fix_eve-413-3
branch
2 times, most recently
from
2f96639 to
88efa73
Compare
christoph-zededa
changed the title
| var ok bool | ||
| for { | ||
| select { | ||
| case stats, ok = <-prgNotif: | ||
| case newStats, ok = <-prgNotif: |
There was a problem hiding this comment.
Can you please add a comment why exactly we use a 'newStats' variable?
Actually this is a different bug fix (race fix) and ideally would be great a separate commit for that, but if you write an explicit comment and also mention the fix for the race in the commit message it seems would be sufficient.
There was a problem hiding this comment.
here it is: 3653d4c
| @@ -262,20 +262,26 @@ func NewDronaCtx(name string, noHandlers int) (*DronaCtx, error) { | |||
| return &dSync, nil | |||
| } | |||
|
|
|||
| func reqPostSize(req *DronaRequest, dronaCtx *DronaCtx, stats types.UpdateStats) { | |||
There was a problem hiding this comment.
Please don't commit the vendor changes. The hash should be updated.
There was a problem hiding this comment.
@rouming Presumably need both the hash updated and the vendor directory updated based on the updated hash.
There was a problem hiding this comment.
@eriknordmark yes, what I meant is better not to mix the actual changes with autogenerated changes, which at least should go in a separate commit. (with a lib hash update, for sure)
christoph-zededa
force-pushed
the
fix_eve-413-3
branch
2 times, most recently
from
e939d8c to
dfc452c
Compare
There was a problem hiding this comment.
We do need to get the new hash for the libs and then the vendor directory updated; are you doing that in a separate PR?
Once @rouming has signed off, can you squash these commits?
If eve looses connection while downloading an app image, it first has to timeout to retry downloading with another network interface. The reason is that the error from io.Copy (copying from the network connection) is not propagating the error to the caller that decides to cancel this connection and retry on a different interface. This change propagates the error to the downloader goroutine and this one then cancels the connection. Signed-off-by: Christoph Ostarek <christoph@zededa.com>
when the download is finished; this is needed to forward the error if there is any and not to loose it Signed-off-by: Christoph Ostarek <christoph@zededa.com>
to make it easier readable by clarifying which variables this piece of code is dependent on; also it makes it easier for the reader to skip uninteresting code. Signed-off-by: Christoph Ostarek <christoph@zededa.com>
- use format string - automatically use attempt variable from lambda capture Signed-off-by: Christoph Ostarek <christoph@zededa.com>
Signed-off-by: Christoph Ostarek <christoph@zededa.com>
Just autogenerated stuff after the following commands: make proto && make proto-vendor Nothing to look at. Signed-off-by: Christoph Ostarek <christoph@zededa.com>
christoph-zededa
force-pushed
the
fix_eve-413-3
branch
from
dfc452c to
14ce7b2
Compare
If eve looses connection while downloading an app image, it
first has to timeout to retry downloading with another network
interface.
The reason is that the error from io.Copy (copying from the network
connection) is not propagating the error to the caller that decides
to cancel this connection and retry on a different interface.
This change propagates the error to the downloader goroutine and this
one then cancels the connection.