fix(mars-worker): treat TCP exceptions as unrecoverable again - #233
Merged
Conversation
Revert the earlier leniency that classified 'connection reset by peer' / 'socket read failed' as recoverable. A stale/half-dead pooled FDB connection (e.g. from an FDB store restart) can otherwise be silently reused by a later, unrelated job and crash the whole worker process. Restarting the worker guarantees a fresh process with no leftover connection state. Occasional restarts (and the resulting k8s backoff) are accepted since these events are not expected to happen constantly.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Reverts the earlier leniency in
classify_mars_errorthat classifiedconnection reset by peer/socket read failedas recoverable. These(and any other unclassified
TCPExceptionwording, e.g. broken-pipewrite/read failures) now fall through to the default
unrecoverablebranch, which triggers the worker's existing graceful self-restart path.
Why
A stale/half-dead pooled FDB connection (e.g. from an FDB store restart)
can otherwise be silently reused by a later, unrelated job and crash the
whole worker process with an uncaught C++ exception. Restarting the
worker guarantees a fresh process with no leftover connection state.
We previously moved away from restarting on every TCP exception because
it could lead to Kubernetes restart backoff (CrashLoopBackOff) if it
happened repeatedly. We're accepting that tradeoff again: TCP exceptions
from FDB are not expected to happen constantly, so the occasional
restart-and-backoff cycle is preferable to a worker that stays broken.
Testing
mars-workerand deployed the digest-pinned image tolumi-test(marsandmars-heavypools).pytest -e lumi-test -k mars: 19 passed, 2 skipped (run twice).unrelated internal MARS/MIR error during testing; the pod went through
Kubernetes' restart backoff and recovered cleanly on its own.
🤖 Generated with pi