Arvados

Tom Clegg wrote in #note-6:

22434-spot-check-warning @ c22b3ee97ef626e800ff0f72f3af65ea47838542 -- developer-run-tests: #4597

This is all good. Since we're thinking about the usability of these log messages, I'm looking at this:

if err != nil {
    runner.CrunchLog.Printf("Temporarily unable to check spot instance interruptions: %s (will retry in %v)", err, spotInterruptionCheckInterval)
    failures++
    if failures > 5 {
        runner.CrunchLog.Printf("Giving up on checking spot instance interruptions after too many consecutive errors")
        return
    }
    continue
}

It's a little unfortunate that the logs can say "(will retry)" and then immediately after that "nm I lied I'm giving up." What if this flowed like failures++; if failures <= 5 { log the first message; continue; } else { log the give up message; return; }?

Right, "will retry" followed immediately by "giving up" is unfortunate.

Rearranged messages, now it looks like this:

zzzzz-zzzzz-zzzzzzzzzzzzzzz 2025-01-10T20:09:26.721233648Z Unable to check spot instance interruptions: 503 Service Unavailable -- will retry in 125ms
zzzzz-zzzzz-zzzzzzzzzzzzzzz 2025-01-10T20:09:26.845810061Z Unable to check spot instance interruptions: 503 Service Unavailable -- will retry in 125ms
zzzzz-zzzzz-zzzzzzzzzzzzzzz 2025-01-10T20:09:26.971103911Z Unable to check spot instance interruptions: 503 Service Unavailable -- will retry in 125ms
zzzzz-zzzzz-zzzzzzzzzzzzzzz 2025-01-10T20:09:27.096125533Z Unable to check spot instance interruptions: 503 Service Unavailable -- will retry in 125ms
zzzzz-zzzzz-zzzzzzzzzzzzzzz 2025-01-10T20:09:27.220723738Z Unable to check spot instance interruptions: 503 Service Unavailable -- now giving up after too many consecutive errors

22434-spot-check-warning @ 6ac713a1fc7805ffe47d733fca1495e856be54be -- developer-run-tests: #4599

Tom Clegg wrote in #note-8:

22434-spot-check-warning @ 6ac713a1fc7805ffe47d733fca1495e856be54be -- developer-run-tests: #4599

This flows even better, thanks. My one last nit here is now with this change, we lost the word "Temporarily" so now "Unable" sounds permanent and a little bit spooky. I get that the later retry text should clarify that, but I wonder if we can do better. What about "Spot instance interruption check did not succeed (%s)"? I think putting the subject before the verb can help contextualize this.

If that's too big or too off-style, maybe "Could not check for spot instance interruption: %s"? I feel like this is just vaguer but I feel like that kind of is what we're after.

Happy to workshop it in chat, or not. I don't feel super strongly about this, just my personal take given the tone we're trying to strike in this ticket. Thanks.

22434-spot-check-warning @ fd55bde2aef8288b63c7f084982041060006d711 -- developer-run-tests: #4602

"Spot instance interruption check was inconclusive: %s"