fab-classic icon indicating copy to clipboard operation
fab-classic copied to clipboard

fix parallel mode under Python-3.8

Open ploxiln opened this issue 4 years ago • 14 comments

attempt to fix #27 (but probably only works for basic cases)

ploxiln avatar Mar 20 '20 05:03 ploxiln

Probably a better idea: add a thread mode as an alternative to the multiprocessing mode for parallel execution. env would be a thread-local variable. Most typical tasks would work with parallel-thread mode, but it would not be perfectly compatible with all existing tasks, which may depend on other user global state variables not being shared between parallel tasks, so thread-mode and multiprocessing-mode would both be options.

ploxiln avatar Mar 30 '20 19:03 ploxiln

I've tried

pip install https://github.com/ploxiln/fab-classic/archive/python38_parallel.tar.gz

with this fabfile:

from fabric.api import env, put, run, parallel

env.hosts = '[email protected]'

@parallel
def test():
    run('hostname')

and this is what I got:

(venv) MacBook-Pro:myproject mmalysh$ fab test
[[email protected]] Executing task 'test'
!!! Parallel execution exception under host '[email protected]':
Process [email protected]:
Traceback (most recent call last):
  File "/Users/mmalysh/.pyenv/versions/3.8.2/lib/python3.8/multiprocessing/process.py", line 315, in _bootstrap
    self.run()
  File "/Users/mmalysh/.pyenv/versions/3.8.2/lib/python3.8/multiprocessing/process.py", line 108, in run
    self._target(*self._args, **self._kwargs)
  File "/Users/mmalysh/Development/myproject/venv/lib/python3.8/site-packages/fabric/tasks.py", line 220, in _parallel_wrap
    queue.put({'name': name, 'result': task.run(*args, **kwargs)})
AttributeError: 'function' object has no attribute 'run'

Fatal error: One or more hosts failed while executing task 'test'

Underlying exception:
    'function' object has no attribute 'run'

Aborting.

maxmalysh avatar Apr 03 '20 13:04 maxmalysh

Yeah, this assumes that you decorate your task functions with @task

ploxiln avatar Apr 03 '20 14:04 ploxiln

It kind of works...

@task
@parallel
def test():
    run('hostname')

Output:

(venv) MacBook-Pro:myproject mmalysh$ fab test
[[email protected]] Executing task 'test'
[[email protected]] Executing task 'test'
[[email protected]] run: hostname
[[email protected]] run: hostname
[[email protected]] out: mesg: ttyname failed: Inappropriate ioctl for device
[[email protected]] out: sun.foobar.com
[[email protected]] out: 

[[email protected]] out: mesg: ttyname failed: Inappropriate ioctl for device
[[email protected]] out: moon.foobar.com
[[email protected]] out: 


Done.

Notice the error: mesg: ttyname failed: Inappropriate ioctl for device.

maxmalysh avatar Apr 03 '20 17:04 maxmalysh

I'm seeing the error as well (upgrading from fabric 1.4 to fab-classic 1.18 with python 2.x)

[[email protected]] out: mesg: ttyname failed: Inappropriate ioctl for device

gingerlime avatar Apr 23 '20 15:04 gingerlime

that's not related to this PR or the issue this PR attempts to solve (parallel under python-3.8). that only is related to getting a PTY

ploxiln avatar Apr 23 '20 16:04 ploxiln

Sorry just searched for the error and that’s the first thing that popped up so I thought it’s somehow related. Happy to open a separate issue

EDIT: reported #46

gingerlime avatar Apr 23 '20 16:04 gingerlime

Any update on this? We are seeing the same issue.

rafecolton avatar Aug 10 '20 22:08 rafecolton

Unfortunately this still struggles when running under parallel mode (in 3.8+), two problems being the with_settings decorator not applying to the tasks execute wraps, and more importantly, execute does not seem wait on completion:

File ~/host_finder.py:348, in HostDataCollector.host_report(self, datacenter)
    345 output = execute(get_kvm_data, roles=[datacenter])
    347 # Trim trash results, and log a warning why this happened
--> 348 for key, value in output.items():
    349     if not isinstance(value, collections.abc.MutableMapping):
    350         logger.warning("%s raised when retrieving data from %s", value, key)

RuntimeError: dictionary changed size during iteration

Edit: The above test was run on Python 3.8.6. I'd considered this might be related to https://github.com/python/cpython/issues/83541, but this still occurs under Python 3.8.13.

Stealthii avatar May 05 '22 15:05 Stealthii

Ignore the above - I incorrectly attributed this to dict updates after execute finished, this is not a bug in fab-classic, this was me being stupid when putting together a test.

However, there are still behavioural changes with 3.8 compared to 3.6/3.7 - one is around env context not getting applied down the chain (which explains env settings not applying even within the same method, and @with_settings decorator on the method calling execute not applying to tasks run underneath. @ploxiln I know you addressed this in your first comment on here.

Execution time of parallel jobs is also significantly higher - on the above test against 68 hosts (with parallel pool size set to 100), Python 3.7 will take ~8s to finish execution, with Python 3.8 taking ~45s for the same task.

Stealthii avatar May 05 '22 17:05 Stealthii

I've had great success with ensuring the default processing call uses fork() and not spawn() - pre 3.8 speed, behaviours and execution speed on macOS seem to be upheld:

diff --git a/fabric/tasks.py b/fabric/tasks.py
index 47a2b49f..dcb34d5f 100644
--- a/fabric/tasks.py
+++ b/fabric/tasks.py
@@ -329,6 +329,7 @@ def execute(task, *args, **kwargs):
         # if it can't.
         try:
             import multiprocessing
+            multiprocessing.set_start_method('fork')
         except ImportError:
             import traceback
             tb = traceback.format_exc()

@ploxiln it might be worth making this change for non-Windows (or all if we don't maintain Windows support), until such a time as the multi-thread method can be rewritten as you suggest.

Stealthii avatar May 05 '22 18:05 Stealthii

I'm pretty sure parallel mode never worked with windows anyway, so we could just switch to "fork" method now.

ploxiln avatar May 06 '22 13:05 ploxiln

Just came across your work, 1000% thank you for forking fab-classic.

As for this issue, after dealing with so many old / fragile py2 related dependencies, really like the looks of this ability to restore parallel tasks in current python. Mg.

boblozano avatar May 11 '22 18:05 boblozano

I know this is not a proper fix or anything, but one can use this workaround without doing any monkey-patching:

In my main fabfile I did this:

import multiprocessing
multiprocessing.set_start_method('fork')

And it seems to be working nicely.

I just realized this as I was stuck to python3.7 earlier and I'm trying to do an upgrade now.

Also @ploxiln - based on the docs for multiprocessing: https://docs.python.org/3/library/multiprocessing.html

Changed in version 3.8: On macOS, the spawn start method is now the default. The fork start method should be considered unsafe as it can lead to crashes of the subprocess as macOS system libraries may start threads. See bpo-33725.

I believe that this is the reason why the problem started happening for me. As in - it would have also started for earlier python versions if they would use spawn (though I didn't check that part myself, just trusted the docs). If that's the case, I think it's OK to use it with fabric (since this is what I was doing earlier without even realizing it).

ajakubo1 avatar Oct 05 '23 11:10 ajakubo1