Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pickling Errors due to Window's spawn Multiprocessing start method #2

Closed
tsculpt opened this issue Sep 8, 2015 · 12 comments
Closed

Comments

@tsculpt
Copy link
Contributor

tsculpt commented Sep 8, 2015

Greetings Pietro,

Fantastic work on this framework! The best I've found. I am thrilled with your most recent inclusion of components.

I have found the framework breaks on Windows with Python 3.4 due to both the bot's logging and GNUTranslations objects being non-serializable to spawned processes. I have gotten around these pickling issues by rebuilding the objects on the processes after they've started. In the case of the logger, I will likely get garbled/intermixed messages if I don't implement a queue to handle concurrent logging events. I am not very knowledgeable regarding the best practices in dealing with these concurrency issues and differing process start methods between platforms. Perhaps you are able to provide a more elegant solution to this? I have learned quite a bit reading through your code!

Thank you.
Brad

By the way, a feature I plan to integrate into my bot relies on a scheduler for messaging. Perhaps this can be useful to the framework overall? It would basically be extending the runner to include a process that uses APScheduler or the like. I could see a scheduling piece being integrated into components as well. Cheers.

pietroalbini pushed a commit that referenced this issue Sep 9, 2015
This allows to run multiple bots in the same runner, and also allows workers to
process not only updates, but everything (will be useful later for timers).

I've tried to remove as much pickle as I can from the queues, avoiding to pass
Bot or TelegramAPI objects around. This was done because pickle likes to
complain loudly when it receives some objects it don't like. Hopefully this
improves the problem reported in #2.
@pietroalbini pietroalbini self-assigned this Sep 9, 2015
@pietroalbini
Copy link
Contributor

Hi Brad, thanks for the bug report (and the suggestions!).

The pickling problem is nasty, and I met it while implementing support for keyboards. I've just refactored the whole runner code, and with that done, most of the pickle problems should be fixed everywhere except for Windows. Maybe you can try now, using the latest commit, to see if everything works also in Windows, but I don't think so, because in Windows the Bot instances will be pickled anyway to start the new processes.

Now I'll try to implement an extra "frozen" (immutable) bot instance, so I can keep everything as before in the code you write, while using a frozen instance in the runner code. This should also protect users from adding hooks/processors at runtime, which even now creates a big mess.

For the logging thing, thanks for pointing me that. I'll think about a solution for it. And about timers and deferreds, those are on my features wishlist from day one :-)

Pietro.

@tsculpt
Copy link
Contributor Author

tsculpt commented Sep 9, 2015

Hi Pietro.

I have pulled down the new runner code, and as expected, the logging and language tables still cause problems for process creation. While the new runner requires more patching in more places, I did get it working!

Next, I will try the new logger to see if it makes it through the spawn process without having to be stripped and added back after process startup. I'll try!

@tsculpt
Copy link
Contributor Author

tsculpt commented Sep 10, 2015

FYI, logbook works great and passes into processes without the need to do anything. The only thing I am patching for now are the GNUTranslations objects held by the bots.

Cheers, Brad

pietroalbini pushed a commit that referenced this issue Sep 10, 2015
This should improve the situation of #2, if not fixing it.

After this commit, only frozen bot instances will be used in the runner or
while processing messages. This is useful both because frozen bots instances
prevents different state between workers, and because frozen instances can be
pickled/unpickled flawlessy.
@pietroalbini
Copy link
Contributor

Can you try with the latest commit? Frozen bot instances should be completly pickleable.

Pietro.

@tsculpt
Copy link
Contributor Author

tsculpt commented Sep 11, 2015

Had to change FrozenBot.__reduce__'s return to match reduce interface, where the returned tuple's second item is itself a tuple. With this fixed, I still have pickling problem:

  File "C:\Python34\Lib\multiprocessing\reduction.py", line 59, in dump
    ForkingPickler(file, protocol).dump(obj)
_pickle.PicklingError: Can't pickle <function DefaultComponent.help_command
x031423D8>: attribute lookup help_command on botogram.defaults failed

pietroalbini pushed a commit that referenced this issue Sep 11, 2015
That should hopefully fix the last bits of #2.
@pietroalbini
Copy link
Contributor

Ok, now on Linux the FrozenBot instance is pickled flawlessy (on Python 3.4). Can you try on Windows with the latest commit (2509be8)? Unfortunately I had to drop support for Python 3.3, since pickleing methods requires the pickle protocol version 4, implemented in Python 3.4.

And probably I shouldn't code at midnight... (the errors you found this morning).

Pietro.

@tsculpt
Copy link
Contributor Author

tsculpt commented Sep 11, 2015

Everything seems to pickle fine on the Windows side as well! Thank you.

As a side effect of freezing the bots, bot sub-classing no longer works, as attributes are now stripped during the freeze. I'm not sure this is a big deal with now having components, but it has forced me to rethink how I'm doing some things.

In my case, I'm looking to pass along a bot specific connection address for a database, such that a connection can be held per bot per worker process, and shared to the bot per job, thus allowing for concurrent database access. I'm sure there's a better pattern than my previous hack...

Maybe I need to extend Component instead of Bot, to hold a connection attribute, and subclass off it for those components that require db access. But then I'm still not sure how I'll assign to it the processes' connection. Perhaps something like a pass_db decorator can be used akin to pass_bot, for injecting the connection where it's needed. Just thinking out loud.

Brad

@pietroalbini
Copy link
Contributor

Yes, custom attributes on Bot instances won't work anymore. But that's the whole point of frozen bots, because anything stored in Bot's attributes creates just a big mess when you update the attribute's value (since the change applies only to one worker, and not all). And you can also expect a FrozenComponent in the future.

I'm currently working on a thing similar to the one you described (pass_db), but more generalized: shared memory. Each component will have an unique dict-like object (aka no conflicts between components), in which you can store everything (read: any pickleable thing) you want, and the dict will be synchronized between all the workers. Then you'll be able to get it using a @botogram.pass_shared decorator. I'm planning to use the multiprocessing's managers under the hood.

By the way, thanks for testing things in Windows :-)

Pietro.

@tsculpt
Copy link
Contributor Author

tsculpt commented Sep 11, 2015

Sounds great Pietro. I'm looking forward to it!

@pietroalbini
Copy link
Contributor

Hi @tsculpt! I've implemented the shared memory in the c338c21 commit. Do you mind testing if everything works correctly on Windows?

You can use the @botogram.pass_shared decorator, to get the component's own shared memory as first argument, just as the pass_bot decorator. Each component has its own, isolated shared memory, and there is also a default one for the hooks you attach directly to the bot.

@tsculpt
Copy link
Contributor Author

tsculpt commented Sep 18, 2015

Hi Pietro.
With attempting to run a stripped bot (just passing the api key), it may run but does not respond. Other times, setting one simple component or the about / owner bot attribute, I get any of the three errors pasted bellow:

...
\runner\__init__.py", line 85, in _boot_processes
    worker.start()
  File "C:\Python34\Lib\multiprocessing\process.py", line 105, in start
    self._popen = self._Popen(self)
  File "C:\Python34\Lib\multiprocessing\context.py", line 212, in _Popen
    return _default_context.get_context().Process._Popen(process_obj)
  File "C:\Python34\Lib\multiprocessing\context.py", line 313, in _Popen
    return Popen(process_obj)
  File "C:\Python34\Lib\multiprocessing\popen_spawn_win32.py", line 66, in __init__
    reduction.dump(process_obj, to_child)
  File "C:\Python34\Lib\multiprocessing\reduction.py", line 59, in dump
    ForkingPickler(file, protocol).dump(obj)
  File "C:\Python34\Lib\multiprocessing\connection.py", line 940, in reduce_pipe_connection
    dh = reduction.DupHandle(conn.fileno(), access)
  File "C:\Python34\Lib\multiprocessing\connection.py", line 170, in fileno
    self._check_closed()
  File "C:\Python34\Lib\multiprocessing\connection.py", line 136, in _check_closed
    raise OSError("handle is closed")
OSError: handle is closed
...
\runner\__init__.py", line 85, in _boot_processes
    worker.start()
  File "C:\Python34\Lib\multiprocessing\process.py", line 105, in start
    self._popen = self._Popen(self)
  File "C:\Python34\Lib\multiprocessing\context.py", line 212, in _Popen
    return _default_context.get_context().Process._Popen(process_obj)
  File "C:\Python34\Lib\multiprocessing\context.py", line 313, in _Popen
    return Popen(process_obj)
  File "C:\Python34\Lib\multiprocessing\popen_spawn_win32.py", line 66, in __init__
    reduction.dump(process_obj, to_child)
  File "C:\Python34\Lib\multiprocessing\reduction.py", line 59, in dump
    ForkingPickler(file, protocol).dump(obj)
_pickle.PicklingError: Can't pickle <class 'weakref'>: attribute lookup weakref
on builtins failed
...
\runner\__init__.py", line 85, in _boot_processes
    worker.start()
  File "C:\Python34\Lib\multiprocessing\process.py", line 105, in start
    self._popen = self._Popen(self)
  File "C:\Python34\Lib\multiprocessing\context.py", line 212, in _Popen
    return _default_context.get_context().Process._Popen(process_obj)
  File "C:\Python34\Lib\multiprocessing\context.py", line 313, in _Popen
    return Popen(process_obj)
  File "C:\Python34\Lib\multiprocessing\popen_spawn_win32.py", line 66, in __init__
    reduction.dump(process_obj, to_child)
  File "C:\Python34\Lib\multiprocessing\reduction.py", line 59, in dump
    ForkingPickler(file, protocol).dump(obj)
_pickle.PicklingError: Can't pickle <function BaseManager._finalize_manager at 0
x03214F18>: attribute lookup _finalize_manager on multiprocessing.managers failed

Hope this helps,
Brad

@pietroalbini
Copy link
Contributor

Does this also happen on the master branch?

Also, maybe it's better if you open another issue about this.

MarcoBuster pushed a commit to MarcoBuster/botogram that referenced this issue Jul 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants