#200 closed #123, which caught runtime errors in the Worker and Engine and failed the corresponding requests with a 500-level exception instead of leaving the Worker process in an infinite error handling loop.
There are still some open questions and enhancements to the error handling setup.
Main Open Question: Failure Attribution
Right now, if a failure happens during forward or preprocess, all RIDs get failed.
- Blaming a whole batch is too blunt. When
forward/preprocess crashes, we
can't cleanly attribute it to one rid. Failing all of them is the easy answer and
probably the wrong one.
A retry path — re-queue the batch's requests, ideally re-isolated (e.g. retry singly / in smaller batches) — is likely better, but needs a retry bound so a genuinely bad request can't loop forever. This sub-problem is the crux of the issue; scope it
deliberately rather than defaulting to "fail the batch."
- The solution need not be complex; something as simple as "retry all requests sequentially" upon
preprocess or forward failure could be a good first step.
- Recoverable vs. fatal is hard to classify. A per-request bug should drop
just that request; a corrupted CUDA context or a wedged allocator should take
the worker down rather than silently dropping requests against a broken GPU.
There's no obvious clean rule. Candidate signals: exception type, or a
frequency/threshold heuristic (e.g. the same error N times in a row → treat as
fatal). Propose a policy rather than assuming one, and it is also OK to not tackle this point immediately.
Note: #200's _handle_main_loop_error comment already marks where, e.g., a retry path, would go.
Minor Issues
These are minor issues that were non-blocking for the original #200 PR, but should be fixed anyway:
-
In the KVCacheEngine, if one rid fails prepare_inputs and another rid in the same minibatch then raises AllocationFailedError, the alloc envelope in execute_batch builds a fresh NodeOutput(allocation_failed=True, ...) that doesn't carry prepared.failed_requests, so the first rid's error report is dropped for that round. It's not a hang as _handle_allocation_failure pushes back all batch.node_objects (the failed rid included, since _drop_failed_rids never ran), so it re-fails and gets reported on the next attempt. But the report is delayed by an alloc-backoff round, and in a persistently-OOM-thrashing worker it could starve. Carrying failed_requests into that NodeOutput is a one-liner, though it makes the worker requeue and fail the rid in the same iteration; the current drop-and-rediscover is arguably the simpler invariant. Either way, the interaction should be documented in the code.
-
The KVCacheEngine re-raise AllocationFailedError so the retryable alloc envelope keeps priority, but the stateless ones catch bare Exception. Currently, stateless engines have no allocator, but if a stateless submodule ever raises it, the retryable signal would be converted into a permanent request failure. A mirrored carve-out or a short comment stating the invariant would be more robust.
#200 closed #123, which caught runtime errors in the Worker and Engine and failed the corresponding requests with a 500-level exception instead of leaving the Worker process in an infinite error handling loop.
There are still some open questions and enhancements to the error handling setup.
Main Open Question: Failure Attribution
Right now, if a failure happens during
forwardorpreprocess, all RIDs get failed.forward/preprocesscrashes, wecan't cleanly attribute it to one rid. Failing all of them is the easy answer and
probably the wrong one.
A retry path — re-queue the batch's requests, ideally re-isolated (e.g. retry singly / in smaller batches) — is likely better, but needs a retry bound so a genuinely bad request can't loop forever. This sub-problem is the crux of the issue; scope it
deliberately rather than defaulting to "fail the batch."
preprocessorforwardfailure could be a good first step.just that request; a corrupted CUDA context or a wedged allocator should take
the worker down rather than silently dropping requests against a broken GPU.
There's no obvious clean rule. Candidate signals: exception type, or a
frequency/threshold heuristic (e.g. the same error N times in a row → treat as
fatal). Propose a policy rather than assuming one, and it is also OK to not tackle this point immediately.
Note: #200's
_handle_main_loop_errorcomment already marks where, e.g., a retry path, would go.Minor Issues
These are minor issues that were non-blocking for the original #200 PR, but should be fixed anyway:
In the
KVCacheEngine, if one rid failsprepare_inputsand another rid in the same minibatch then raisesAllocationFailedError, the alloc envelope inexecute_batchbuilds a freshNodeOutput(allocation_failed=True, ...)that doesn't carryprepared.failed_requests, so the first rid's error report is dropped for that round. It's not a hang as _handle_allocation_failure pushes back allbatch.node_objects(the failed rid included, since _drop_failed_rids never ran), so it re-fails and gets reported on the next attempt. But the report is delayed by an alloc-backoff round, and in a persistently-OOM-thrashing worker it could starve. Carryingfailed_requestsinto thatNodeOutputis a one-liner, though it makes the worker requeue and fail the rid in the same iteration; the current drop-and-rediscover is arguably the simpler invariant. Either way, the interaction should be documented in the code.The
KVCacheEnginere-raise AllocationFailedError so the retryable alloc envelope keeps priority, but the stateless ones catch bareException. Currently, stateless engines have no allocator, but if a stateless submodule ever raises it, the retryable signal would be converted into a permanent request failure. A mirrored carve-out or a short comment stating the invariant would be more robust.