agent: backoff session when no remotes are available#2570
Conversation
stevvooe
force-pushed
the
backoff-on-error
branch
from
aa96944 to
4140590
Compare
Codecov Report
@@ Coverage Diff @@
## master #2570 +/- ##
==========================================
- Coverage 61.4% 61.38% -0.03%
==========================================
Files 134 134
Lines 21800 21803 +3
==========================================
- Hits 13387 13384 -3
- Misses 6968 6976 +8
+ Partials 1445 1443 -2 |
| // since we are returning without launching the session goroutine, we | ||
| // need to provide the delay that is guaranteed by calling this | ||
| // function. | ||
| time.Sleep(delay) |
There was a problem hiding this comment.
Why sleep instead of a timer ?
https://github.com/docker/swarmkit/pull/2570/files#diff-15ffe95a2da45f70696ffa3c01949601L84
There was a problem hiding this comment.
Also, would this possibly block here: https://github.com/docker/swarmkit/blob/master/agent/agent.go#L367, thus preventing any other events, such as shutdown or node update, etc. from happening, for up to 8 seconds every time?
Not sure if it would make sense instead to do the sleep/write the error to the channel in a goroutine instead?
There was a problem hiding this comment.
Yes, this probably isn't a great fix. For this to be correct, we need a goroutine. Good catch!
Signed-off-by: Stephen J Day <stephen.day@docker.com>
stevvooe
force-pushed
the
backoff-on-error
branch
from
4140590 to
de0c0d7
Compare
|
@anshulpundir @cyli Give that a look! It should now delay the send on the error channel to simulate that the delay from |
| // avoid blocking the main loop. | ||
| go func() { | ||
| time.Sleep(delay) | ||
| s.errs <- err |
There was a problem hiding this comment.
Just curious, what happens if the context gets done in the mean time ?
There was a problem hiding this comment.
Good point, I guess this wouldn't have a listener anymore. Wondering if we should call s.sendError(err) instead, which selects between writing the error and the session closed channel being closed?
There was a problem hiding this comment.
Nothing: this is a buffered channel. If someone is listening, they pick it up. If no one is listening, it goes into the buffer immediately and the channel then gets garbage collected.
5 participants
Signed-off-by: Stephen J Day stephen.day@docker.com