[WIP] Start of work to redo the threading and forking split #44280
Conversation
|
!jimi_skip |
jimi-c
force-pushed
the
threading_and_forking_redux
branch
6 times, most recently
from
14250bc
to
9fb6066
Compare
|
A general comment, |
jimi-c
force-pushed
the
threading_and_forking_redux
branch
3 times, most recently
from
64e3454
to
6ef1796
Compare
jimi-c
force-pushed
the
threading_and_forking_redux
branch
from
6ef1796
to
a76a01b
Compare
|
Is there any chance of the plugins/loader.py patches landing for 2.8? They are fairly low impact, and useful elsewhere regardless of the remaining refactor Also I'm worried #45572 may need reverted (or locks added) with respect to this PR, it shares the Templar across threads:
|
jimi-c
force-pushed
the
threading_and_forking_redux
branch
3 times, most recently
from
593a756
to
1e4b019
Compare
jimi-c
force-pushed
the
threading_and_forking_redux
branch
from
1e4b019
to
f7273df
Compare
| __all__ = ["ProcessModelBase", "keyboard_interrupt_event", "ResultsSentinel"] | ||
|
|
||
|
|
||
| class ResultsSentinel: |
There was a problem hiding this comment.
FWIW, we have a sentinel available in ansible.utils.sentinel now. Not sure if it's worth using that instead.
| __all__ = ['ProcessModel',] | ||
|
|
||
|
|
||
| class ProcessModel(ProcessModelBase): |
There was a problem hiding this comment.
Not sure if it would be useful for something later, but should we have a class attr with the name of the plugin in it? Some plugin types do this others don't.
There was a problem hiding this comment.
_load_name gets appended by loader itself
jimi-c
force-pushed
the
threading_and_forking_redux
branch
3 times, most recently
from
f20c494
to
941d32e
Compare
ci_complete ci_coverage
* Also reverts the default model back to forking
This was in the original TQM use to protect against the possibility that /dev/shm was not available.
* Also fixes a bug in which the callback for tasks being started in threads was when the task data was put on the queue, not when it was popped by a worker and actually started.
A longer term solution should move the decision to run with threading or forking into the tests themselves, instead of being dependent on the configuration of the test environment. ci_complete
This reverts commit 4c66fa35cfa562480dbc223e0fa22be9e01f043d.
ci_complete
!ci_complete
jimi-c
force-pushed
the
threading_and_forking_redux
branch
from
941d32e
to
f5c6c1c
Compare
!ci_complete
jimi-c
force-pushed
the
threading_and_forking_redux
branch
from
f5c6c1c
to
d0be06b
Compare
|
Hi! |
|
@emousse not really. A few things popped up regarding non-thread-safe methods we use in python, and I cannot confidently say those will ever be addressed. As such I've turned my attention to alternative ways of improving the performance of the core engine. I'm going to go ahead and close this WIP now as it's quite out of date. |
This refactors the way we handle process management by introducing the ProcessModel* classes. Right now, these will remain hard-coded as there's no real plans to add others in the future beyond threading/forking.
As a WIP, this PR enables threading by default, however when merged forking will be the default as it is today. Threading will have to be enabled via a config change (environment variable or ansible.cfg).