FIX report a worker recycled for memory usage accurately and only once - #641
Open
larsoner wants to merge 5 commits into
Open
FIX report a worker recycled for memory usage accurately and only once#641larsoner wants to merge 5 commits into
larsoner wants to merge 5 commits into
Conversation
A worker that loky itself shuts down because its memory usage grew past _MAX_MEMORY_LEAK_SIZE is reported through the generic "A worker stopped while some jobs were given to the executor" warning. Since a recycling happens between two tasks, and callers that keep the executor fed always have pending work at that point, a long-running executor emits that warning over and over for behaviour it triggered on purpose. Send the recycled worker's pid back wrapped in _RecycledWorkerPid so the manager thread can tell the two cases apart, and give the recycling case its own message, emitted once per executor. The warning for a worker that stopped on its own, e.g. because of a too short timeout, is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The amount of memory growth tolerated before a worker is recycled was hardcoded to 300 MB. The reference it is compared against is measured once, after the worker's first task, and never updated, so a workload whose tasks differ a lot in memory footprint crosses that threshold without leaking anything and pays for a worker restart every time. Read it from LOKY_MAX_MEMORY_LEAK_SIZE, in the same style as the existing LOKY_MAX_DEPTH. Workers inherit the environment of the process that created the executor, and loky's `env` argument sets variables before any module is loaded in the worker, so both routes work. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Okay an even more important/useful change is to be able to set |
Contributor
Author
|
I think CIs are just red here because they need #463 |
Contributor
Author
|
I also hit a deadlock in MNE-BIDS-Pipeline tests locally because we treat warnings as errors, and the warning being raised as an error resulted in a deadlock. I've push a fix here for that, too. |
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.
Companion PR of joblib/joblib#1829, worked through with Claude Opus 5.
Claude Opus 5 summary
When a worker's memory grows past _MAX_MEMORY_LEAK_SIZE, loky shuts it down and starts a replacement. The manager thread reports that through the generic "A worker stopped while some jobs were given to the executor. This can be caused by a too short worker timeout or by a memory leak." warning.
A recycling always happens between two tasks, so any caller that keeps the executor fed has pending work at that moment and the warning fires on essentially every recycling. For a long-running executor that means the same warning repeated indefinitely for something loky decided to do on purpose and has already handled. The MNE-Python documentation build, which drives sphinx-gallery's Parallel(n_jobs=4, pre_dispatch="n_jobs") over examples whose workers grow well past 300 MB, emits it 24 times in a single build. joblib#883 is the long-standing report of this.
This sends the recycled worker's pid back wrapped in a _RecycledWorkerPid int subclass so the manager thread can tell a recycling it triggered itself from a worker that stopped on its own. The recycling case gets its own message naming the actual cause and is emitted once per executor. The warning for a worker that stopped on its own, for instance because of a too short timeout, is unchanged, as are the test_worker_timeout.py expectations.
test_memory_leak_protection keeps asserting the warning and now also asserts it is emitted exactly once across the repeated restarts it provokes.
Tested on macOS across the loky, spawn and forkserver executor suites plus test_reusable_executor.py, test_worker_timeout.py and test_resource_tracker.py: 239 passed, 8 skipped.