Don't loop forever on errors writing installinator slots #9471
Conversation
|
I found this while working on something else, I had a bad check for a |
labbott
force-pushed
the
installinator_stop_loop
branch
from
69d14eb to
410c0ae
Compare
| if success_this_iter == self.drives.len() || success_prev_iter > 0 { | ||
| // 3. We had the same number of successes as the previous iteration, | ||
| // which implies that we seem to be permanetly stuck and unlikely | ||
| // to succeed |
There was a problem hiding this comment.
Hmm, this is a pretty significant behavior change to the real installinator, right? I think the intent was to loop forever if we're not having success, because there's no way to restart this on failure other than aborting the entire mupdate and starting over.
That's pretty terrible for tests, though. I wonder if we should have a cap on the number of attempts (which this code implicitly has, I think, with the cap set to "2"), and allow prod to pass a cap of "loop forever" while tests can pass something much smaller?
There was a problem hiding this comment.
yeah we're getting into "timeouts timeouts always wrong" territory here. I don't actually mind looping forever for tests because that is a sign I need to fix something because tests should finish. Looping forever in production actually seems worse though if there's some kind of permanent error we'll just be stuck which seems bad. Maybe wicket will give more information than tests though? If we're actually okay with the current behavior I can also just close this.
Currently if there's a permanent error while writing to the M.2 drives we may loop/retry forever. This isn't great behavior so attempt to break if it looks like we aren't making progress writing.
Currently if there's a permanent error while writing to the M.2 drives we may loop/retry forever. This isn't great behavior so attempt to break if it looks like we aren't making progress writing.