Pylons / pyramid_debugtoolbar

Pyramid debug toolbar
https://docs.pylonsproject.org/projects/pyramid-debugtoolbar/en/latest/
Other
95 stars 82 forks source link

Sqa panel code break for multiple threads #345

Open mei-li opened 6 years ago

mei-li commented 6 years ago

Sqa.py code is adding to the DB connection the attribute pdtb_start_timer here

Then it removes this attribute. When accessing the DB with multiple threads this code breaks if both threads are executing and then the first that reaches the after will remove it and the second will fail.

 in _execute_on_connection
    return connection._execute_clauseelement(self, multiparams, params)
  File "/home/meili/.pyenv/versions/3.6.4/envs/radar364/lib/python3.6/site-packages/sqlalchemy/engine/base.py", line 1010, in _execute_clauseelement
    compiled_sql, distilled_params
  File "/home/meili/.pyenv/versions/3.6.4/envs/radar364/lib/python3.6/site-packages/sqlalchemy/engine/base.py", line 1153, in _execute_context
    context.executemany)
  File "/home/meili/.pyenv/versions/3.6.4/envs/radar364/lib/python3.6/site-packages/sqlalchemy/event/attr.py", line 256, in __call__
    fn(*args, **kw)
  File "/home/meili/.pyenv/versions/3.6.4/envs/radar364/lib/python3.6/site-packages/pyramid_debugtoolbar/panels/sqla.py", line 44, in _after_cursor_execute
    delattr(conn, 'pdtb_start_timer')

Using latest version, 4.5

Unfortunately this code will still be executed, even is this panel is not configured. I tried to fix this issue without any luck so far. Maybe one way to start is to isolate this code so it can actually be switched off.

jvanasco commented 5 years ago

Do you connect to the database in your app's init()/main() phase (i.e. before the application forks or spawns threads)? This error looks like you do. SqlAlchemy connections are not safe to cross threads/forks/processes, and you need to call engine.dispose() before the "fork". Calling dispose will get rid of the engine's connection, so each process/thread will have it's own isolated connection after the "fork".

mei-li commented 5 years ago

Thank you for you response. We actually avoided using database calls inside the different threads as we call them only to parallelise some web requests. Though we used scoped session that is a thread local and each thread uses another session, which seems to be an appropriate use. so I wonder whether the connection should not be assumed to be used by one thread.

mei-li commented 4 years ago

Hi again, It is becoming hard to avoid having to do DB queries in the various threads. My understanding from reading about it is that we can initiate new sessions, but we would stick to the same connection and that would still be an issue with debug toolbar. What you mention about engine.dispose() I understand from sqlalchemy docs makes sense in a multiprocessing context and seems to be expensive too. In our case we spawn threads to parallelize processing for short time. Do you think there is a way to have the debugtoolbar depending on the session instead of the connection or sth else that would work?

mmerickel commented 4 years ago

Can you show some sort of snippet of what’s going on? Are you passing a session between threads? That’s not supported by SQLAlchemy at all. They are not threadsafe.