Project

General

Profile

Bug #38407

Funny issues with python sub-interpreters

Added by Sebastian Wagner 6 months ago. Updated 6 months ago.

Status:
New
Priority:
Normal
Assignee:
-
Category:
python interface
Target version:
Start date:
02/20/2019
Due date:
% Done:

0%

Source:
Tags:
Backport:
Regression:
No
Severity:
3 - minor
Reviewed:
Affected Versions:
ceph-qa-suite:
Pull request ID:

Description

Orchestrator is now running into funny issues with python sub-interpreters.

https://modwsgi.readthedocs.io/en/develop/user-guides/application-issues.html#multiple-python-sub-interpreters

Stuff like:

Error EINVAL: Traceback (most recent call last):
  File "/home/sebastian/Repos/ceph/src/pybind/mgr/mgr_module.py", line 862, in _handle_command
    return CLICommand.COMMANDS[cmd['prefix']].call(self, cmd, inbuf)
  File "/home/sebastian/Repos/ceph/src/pybind/mgr/mgr_module.py", line 337, in call
    return self.func(mgr, **kwargs)
  File "/home/sebastian/Repos/ceph/src/pybind/mgr/orchestrator_cli/module.py", line 23, in handle_exception
    return func(*args, **kwargs)
  File "/home/sebastian/Repos/ceph/src/pybind/mgr/orchestrator_cli/module.py", line 93, in _list_devices
    completion.raise_if_exception()
  File "/home/sebastian/Repos/ceph/src/pybind/mgr/orchestrator.py", line 99, in raise_if_exception
    dumped_exception = pickle.dumps(self.exception)
  File "/usr/lib/python2.7/pickle.py", line 1380, in dumps
    Pickler(file, protocol).dump(obj)
  File "/usr/lib/python2.7/pickle.py", line 224, in dump
    self.save(obj)
  File "/usr/lib/python2.7/pickle.py", line 331, in save
    self.save_reduce(obj=obj, *rv)
  File "/usr/lib/python2.7/pickle.py", line 400, in save_reduce
    save(func)
  File "/usr/lib/python2.7/pickle.py", line 286, in save
    f(self, obj) # Call unbound method with explicit self
  File "/usr/lib/python2.7/pickle.py", line 748, in save_global
    __import__(module)
RuntimeError: cannot unmarshal code objects in restricted execution mode

or broken ininstance() calls within the orchestrator.

This is not going to be sustainable in the long term.


Related issues

Related to mgr - Bug #37472: Cython 0.29 removed support for subinterpreters: raises "ImportError: Interpreter change detected ..." Resolved 11/29/2018

History

#1 Updated by Tim Serong 6 months ago

I think the original intent was only to pass data around between modules, not things that could be executed (see the commit message in https://github.com/ceph/ceph/pull/22951/commits/f02316adb4baf4dceaab79cc0ef4c2acdb544f3e)

#2 Updated by Sebastian Wagner 6 months ago

Tim Serong wrote:

I think the original intent was only to pass data around between modules

yeah, you don't even need to execute code. passing objects is enough to get into the realm of funny things

#3 Updated by Tim Serong 6 months ago

Is that just for objects that contain code (i.e. methods), or objects that only contain data? In any case, I think we probably just can't pass objects around between modules, and need to stick to data only.

We can't fix this by removing subinterpreters and going back to a single interpreter that runs all mgr modules, because that will mean that various mgr module dependencies conflict with each other: think two mgr modules that both use cherrypy -- if these are run in a single interpreter, only one will work.

If we do remove subinterepters in future, I believe we will have to switch to an even more isolated structure (e.g. a separate process per module with some form of IPC (or shared memory? not sure)), which also wouldn't support passing objects around.

#4 Updated by Sebastian Wagner 6 months ago

Tim Serong wrote:

Is that just for objects that contain code (i.e. methods), or objects that only contain data? In any case, I think we probably just can't pass objects around between modules, and need to stick to data only.

there are actually at least two parts of this problem:

  1. Passing user defined types around is enough to cause problems with module identity. Anything that depends on module identity breaks: try: except, isinstance, issubclass, etc...
  2. Executing code outside of it's sub-interpreter context, e.g. all code called by remote, like all methods in deepsea.module.DeepSeaOrchestrator that implement methods of Prchestrator including wait: They run in that infamous "Reduced Execution Environment"

We can't fix this by removing subinterpreters and going back to a single interpreter that runs all mgr modules, because that will mean that various mgr module dependencies conflict with each other: think two mgr modules that both use cherrypy -- if these are run in a single interpreter, only one will work.

I've had some bad experience in having multiple sub-interpreters running HTTPS webservers using a common OpenSSL library. I don't know if we really want to have so many different web servers executing within the mgr.

If we do remove subinterepters in future, I believe we will have to switch to an even more isolated structure (e.g. a separate process per module with some form of IPC (or shared memory? not sure)), which also wouldn't support passing objects around.

Maybe that's already enough? Like don't try to pass any Python objects around and don't directly call any functions and instead perform some RPC between sub-interpreters.

#5 Updated by Tim Serong 6 months ago

Sebastian Wagner wrote:

We can't fix this by removing subinterpreters and going back to a single interpreter that runs all mgr modules, because that will mean that various mgr module dependencies conflict with each other: think two mgr modules that both use cherrypy -- if these are run in a single interpreter, only one will work.

I've had some bad experience in having multiple sub-interpreters running HTTPS webservers using a common OpenSSL library. I don't know if we really want to have so many different web servers executing within the mgr.

cherrypy was just an example - the general problem is that if two mgr modules have some python module as a dependency which in turn has some global state, it's going to break.

If we do remove subinterepters in future, I believe we will have to switch to an even more isolated structure (e.g. a separate process per module with some form of IPC (or shared memory? not sure)), which also wouldn't support passing objects around.

Maybe that's already enough? Like don't try to pass any Python objects around and don't directly call any functions and instead perform some RPC between sub-interpreters.

If we're going to develop an RPC solution, I'd suggest waiting until we get rid of subinterpreters and switch to multiple processes. In the meantime, I don't see the benefit of developing some sort of RPC system to use with subinterpreters, over, say, just passing simple data types around.

#6 Updated by Lenz Grimmer 6 months ago

  • Related to Cleanup #38467: Audit other functions in src/mgr/ActivePyModules.cc for thread safety in light of deadlock seen in #35985 added

#7 Updated by Lenz Grimmer 6 months ago

  • Related to Bug #35985: deadlock in standby ceph-mgr daemons added

#8 Updated by Sebastian Wagner 6 months ago

I just don't know if multiple processes are the best way. One big advantage by using one mgr process is the ability to have shared mgr data structures in the mgr memory that we don't need to replicate and synchronize between modules.

#9 Updated by Brad Hubbard 6 months ago

#35985 and #38467 are about thread deadlocks. What evidence do you have that they are related to the issue reported here?

#10 Updated by Tim Serong 6 months ago

Sebastian Wagner wrote:

I just don't know if multiple processes are the best way. One big advantage by using one mgr process is the ability to have shared mgr data structures in the mgr memory that we don't need to replicate and synchronize between modules.

I agree completely. But I suspect that eventually we'll have to ditch subinterpreters and split into separate processes, because subinterpreters are a relatively underused/undertested/underdocumented feature of python, and I don't want to have to do more weird things like https://github.com/ceph/ceph/pull/25585 to keep everything working :-)

#11 Updated by Sebastian Wagner 6 months ago

I don't want to have to do more weird things like https://github.com/ceph/ceph/pull/25585 to keep everything working :-)

And I also completely agree wit this :-)

#12 Updated by Sebastian Wagner 6 months ago

Brad Hubbard wrote:

#35985 and #38467 are about thread deadlocks. What evidence do you have that they are related to the issue reported here?

I don't think they are related.

#13 Updated by Sebastian Wagner 6 months ago

  • Related to Bug #37472: Cython 0.29 removed support for subinterpreters: raises "ImportError: Interpreter change detected ..." added

#14 Updated by Brad Hubbard 6 months ago

Sebastian Wagner wrote:

Brad Hubbard wrote:

#35985 and #38467 are about thread deadlocks. What evidence do you have that they are related to the issue reported here?

I don't think they are related.

But you attached them as "Related issues" ?

#15 Updated by Sebastian Wagner 6 months ago

  • Related to deleted (Cleanup #38467: Audit other functions in src/mgr/ActivePyModules.cc for thread safety in light of deadlock seen in #35985)

#16 Updated by Sebastian Wagner 6 months ago

  • Related to deleted (Bug #35985: deadlock in standby ceph-mgr daemons)

#17 Updated by Sebastian Wagner 6 months ago

removed unrelated issues.

Also available in: Atom PDF