Claude/omnicorp worker pool error ytpcn2#119
Merged
Merged
Conversation
A single pool child dying abruptly -- almost always an OOM kill while
overlaying a very large message -- puts the shared ProcessPoolExecutor
into a permanently broken state. From then on every run_in_executor call
raises BrokenProcessPool ("A process in the process pool was terminated
abruptly while the future was running or pending"), not just the task
whose child died. Because poll_for_tasks reused one executor for the
worker's whole lifetime, one oversized query wedged the worker into
failing every subsequent task until the pod restarted.
Wrap the executor in a PoolManager that catches the breakage, tears the
dead pool down, and stands up a fresh one so the worker recovers on its
next task. Replacement is guarded so the burst of concurrent
BrokenProcessPool errors (every in-flight future fails at once) swaps the
pool only once. The triggering task still fails cleanly via
run_task_lifecycle; only the poisoning of later tasks is fixed.
Also add an opt-in OMNICORP_MAX_TASKS_PER_CHILD knob to recycle pool
children under memory pressure so peak resident size from a big message
is returned to the OS instead of accumulating across tasks. Left unset by
default since it forces the spawn start method.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TiiFCcToq8LgiEeaQhQM5e
aragorn_score and arax_rank had the same wedged-worker vulnerability just fixed in aragorn.omnicorp: poll_for_tasks created a single ProcessPoolExecutor reused for the worker's whole lifetime, so one child dying abruptly (a kernel OOM kill on a large message) put the executor into a permanently broken state and every subsequent task failed with "A process in the process pool was terminated abruptly while the future was running or pending" until the pod restarted. Extract the self-healing wrapper introduced for omnicorp into a shared shepherd_utils.process_pool.ProcessPoolManager and route all three CPU- bound workers through it. The manager catches BrokenProcessPool, tears the dead pool down, and stands up a fresh one so the worker recovers on its next task; the triggering task still fails cleanly via run_task_lifecycle. Surveyed the remaining workers: merge_message already recovers via its own holder + _recreate_executor path; score_paths and monitor/alerts use thread pools, which a task crash cannot permanently poison. No other worker is affected. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TiiFCcToq8LgiEeaQhQM5e
The callback endpoint buffers each subservice's posted TRAPI response into memory before decoding it. An oversized payload can OOM the server (and, once merged, the downstream CPU-bound workers). Reject bodies larger than a configurable limit with 413 before the whole body is read. _read_body_within_limit checks the declared Content-Length first (so well-behaved clients are rejected without buffering anything), then reads the stream in chunks and aborts the moment the running total crosses the limit, so a missing or dishonest Content-Length can't force an unbounded buffer. Enforced in the shared callback() so every ARA's /callback route is covered. The limit is set via callback_max_request_size (Kubernetes-style sizes like "100MB"/"512Mi" or a plain byte count), default 100MB; 0 disables it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TiiFCcToq8LgiEeaQhQM5e
A callback only leaves the per-query running set (the postgres callbacks table that get_running_callbacks reads) once merge_message has processed it. When /callback rejects an oversized payload with 413 it never reaches merge_message, so the callback stayed "running" and the lookup worker blocked waiting for it until its whole-query timeout. Call remove_callback_id on the 413 path so the rejected callback is dropped from the running set immediately and the lookup proceeds with whatever else came back, exactly as it already does when a lookup POST fails. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TiiFCcToq8LgiEeaQhQM5e
/callback enqueues one wake task per callback and adds the callback to the query's ready set. The first worker to grab the query lock becomes the holder and drains the whole ready set; every other wake task for that query is a loser. A loser used to re-enqueue itself after merge_contention_backoff (0.1s) whenever its callback was still pending, so during a single burst merge each contended callback bounced through the stream ~10x/second while the holder drained. The shared get_tasks helper logs "Doing task <payload>" at INFO on every redelivery, and re-enqueued wakes carry the same callback_id/query_id, so the logs filled with hundreds of near-identical "Doing task" lines for what was really one callback being retried -- no duplicate merging, just wake-task churn. Have losers simply ack and drop: the holder already drains the ready set to empty, so a loser has nothing to add. To cover the narrow race where a callback lands after the holder's final drain pass but before it releases the lock, the holder now re-checks the ready set once after unlocking and kicks a single wake if anything remains. Holder crashes are still recovered by reclaim (60s min-idle) once the 45s lock TTL lapses. Net effect: at most one re-enqueue per drain instead of a storm, and one "Doing task" per callback instead of hundreds. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TiiFCcToq8LgiEeaQhQM5e
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
2 participants
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.
No description provided.