sqlalchemy-continuum: 1.3.5 update results in ResourceClosedError's in unit tests

Has the cloned connection functionality from 1.3.5 introduced additional constraints on what you’re allowed to do with connections? I had the unit tests in my Gitlab pipelines fail across multiple projects after the update to 1.3.5. ~~It didn’t reproduce when running the tests locally.~~ It also reproduces locally.

Traceback (most recent call last):
  File "/usr/local/lib/python3.5/site-packages/sqlalchemy/orm/session.py", line 2344, in _flush
    flush_context.execute()
  File "/usr/local/lib/python3.5/site-packages/sqlalchemy/orm/unitofwork.py", line 391, in execute
    rec.execute(self)
  File "/usr/local/lib/python3.5/site-packages/sqlalchemy/orm/unitofwork.py", line 556, in execute
    uow
  File "/usr/local/lib/python3.5/site-packages/sqlalchemy/orm/persistence.py", line 181, in save_obj
    mapper, table, insert)
  File "/usr/local/lib/python3.5/site-packages/sqlalchemy/orm/persistence.py", line 865, in _emit_insert_statements
    result = cached_connections[connection].\
  File "/usr/local/lib/python3.5/site-packages/sqlalchemy/util/_collections.py", line 729, in __missing__
    self[key] = val = self.creator(key)
  File "/usr/local/lib/python3.5/site-packages/sqlalchemy/orm/persistence.py", line 1268, in <lambda>
    compiled_cache=base_mapper._compiled_cache
  File "/usr/local/lib/python3.5/site-packages/sqlalchemy/engine/base.py", line 317, in execution_options
    self.dispatch.set_connection_execution_options(c, opt)
  File "/usr/local/lib/python3.5/site-packages/sqlalchemy/event/attr.py", line 284, in __call__
    fn(*args, **kw)
  File "/usr/local/lib/python3.5/site-packages/sqlalchemy_continuum/manager.py", line 412, in track_cloned_connections
    if connection.connection is c.connection:  # ConnectionFairy is the same - this is a clone
  File "/usr/local/lib/python3.5/site-packages/sqlalchemy/engine/base.py", line 354, in connection
    self._handle_dbapi_exception(e, None, None, None, None)
  File "/usr/local/lib/python3.5/site-packages/sqlalchemy/engine/base.py", line 1416, in _handle_dbapi_exception
    util.reraise(*exc_info)
  File "/usr/local/lib/python3.5/site-packages/sqlalchemy/util/compat.py", line 187, in reraise
    raise value
  File "/usr/local/lib/python3.5/site-packages/sqlalchemy/engine/base.py", line 352, in connection
    return self._revalidate_connection()
  File "/usr/local/lib/python3.5/site-packages/sqlalchemy/engine/base.py", line 432, in _revalidate_connection
    raise exc.ResourceClosedError("This Connection is closed")
sqlalchemy.exc.ResourceClosedError: This Connection is closed

The error seems to be happening while a pytest fixture is being setup, adding some objects to a session and committing them. I’ll try and spend some more time later to get a reproducible test case.

About this issue

Original URL
State: closed
Created 6 years ago
Reactions: 3
Comments: 15 (2 by maintainers)

Commits related to this issue

global: pin `sqlalchemy-continuum` and `urllib3` * `urllib3` is needed as latest `requests` library does not include version 1.23 yet * `sqlalchemy-continuum` is needed because there is an issue ... — committed to zzacharo/cds-videos by zzacharo 6 years ago
global: pin `sqlalchemy-continuum` * `sqlalchemy-continuum` is needed because there is an issue in the latest release(`https://github.com/kvesteri/sqlalchemy-continuum/issues/188`) — committed to zzacharo/cds-videos by zzacharo 6 years ago
global: pin `sqlalchemy-continuum` and `urllib3` * pin urllib until is compatible with `requests` latest version * `sqlalchemy-continuum` is needed because there is an issue in the latest release(... — committed to zzacharo/cds-videos by zzacharo 6 years ago
global: pin `sqlalchemy-continuum` and `urllib3` * pin urllib until is compatible with `requests` latest version * `sqlalchemy-continuum` is needed because there is an issue in the latest release(... — committed to CERNDocumentServer/cds-videos by zzacharo 6 years ago

Most upvoted comments

Having attempted a fix, I think I got the closing of transactions and connections mixed up. I had made the assumption that the connection that triggers the error got closed after the flush resulting from the count call, which would mean that the session_connection_map should be empty after that like in https://github.com/kvesteri/sqlalchemy-continuum/pull/194/commits/ddbf061ce5aea28cb1b0305d03bd382bfaab000a .

But this is not the case, it was just the transaction that got closed. The connection stays active after the count call and that’s fine. The actual closing happens in the test teardown, which continuum isn’t aware of at the moment. So I’ll look into either reacting to that or to simply purging closed connections from the session_connection_map if they’re detected.

mgeens on Jul 14, 2018

I’ve spent some time debugging this and while the tracking cloned connections change triggers the error, it doesn’t seem to be the underlying cause.

The exception occurs because VersioningManager.session_connection_map state persists across unit tests. There’s old closed connections in there that aren’t cleared out, and so when then the cloned connection tracking iterates over it the ResourceClosedError exception happens.

That state bleed-over also happens before 1.3.5. Continuum uses VersioningManager.clear to remove closed connections in response to after_commit and after_rollback events. However, this leaves at least one case out: commits (and subsequent connection closes) that happens as a part of transactions triggered by flushes.

I’ll give an example based on my test case above. What happens in the dummy fixture is the normal case: session.commit is called, which closes the connection, which is removed from session_connection_map by clear in response to an after_commit event.

The trouble occurs in test_succeeds. First session.delete dirties the session. When count is called, it flushes those changes to the database using a new connection (which gets registered in session_connection_map). It does that by calling commit on a SessionTransaction, which also closes said connection. The event associated with that is after_transaction_end, there’s no after_commit involved. As a result session_connection_map doesn’t get updated as it should.

So it looks like the fix would involve calling clear appropriately when after_transaction_end happens. At the moment I can’t tell yet what the impact of that would be and whether the behavior is the same for non-sqlite databases, so that would require further investigation.

mgeens on Jul 1, 2018