Commit d9d6adec authored by Kirill Smelkov's avatar Kirill Smelkov

bigfile/zodb: Resync _ZBigFileH to Connection.transaction_manager on every connection reopen

This continues c7c01ce4 (bigfile/zodb: ZODB.Connection can migrate
between threads on close/open and we have to care): Until now we were
retrieving zconn.transaction_manager on _ZBigFileH init, and further using
that transaction manager for every connection reopen. However that is
not correct because on every reopen connection can be given new
transaction manager.

We were not practically hitting the bug because until recently ZODB was,
by default, using the same ThreadTransactionManager manager instance as
Connection.transaction_manager for all connections, and not doing all
steps needed to keep _ZBigFileH.transaction_manager in sync to
Connection was forgiven - a particular transaction manager that was used
was TransactionManager instance implicitly associated with current
thread by global threading.Local transaction.manager . However starting
from ZODB 5.5.0 Connection code was changed to remember as
.transaction_manager the particular TransactionManager instance without
any threading.Local games:

    https://github.com/zopefoundation/ZODB/commit/b6ac40f153
    https://github.com/zopefoundation/ZODB/issues/208
    https://github.com/zopefoundation/ZODB/pull/226

Given that we were not syncing properly that broke wendelin.core tests, for
example:

    bigfile/tests/test_filezodb.py::test_bigfile_filezodb_vs_conn_migration Exception in thread Thread-1:
    Traceback (most recent call last):
      File "/usr/lib/python2.7/threading.py", line 801, in __bootstrap_inner
        self.run()
      File "/usr/lib/python2.7/threading.py", line 754, in run
        self.__target(*self.__args, **self.__kwargs)
      File "/home/kirr/src/wendelin/wendelin.core/bigfile/tests/test_filezodb.py", line 401, in T11
        transaction.commit()    # should be nothing
      File "/home/kirr/src/wendelin/venv/z-dev/local/lib/python2.7/site-packages/transaction/_manager.py", line 252, in commit
        return self.manager.commit()
      File "/home/kirr/src/wendelin/venv/z-dev/local/lib/python2.7/site-packages/transaction/_manager.py", line 131, in commit
        return self.get().commit()
      File "/home/kirr/src/wendelin/venv/z-dev/local/lib/python2.7/site-packages/transaction/_transaction.py", line 298, in commit
        self._synchronizers.map(lambda s: s.beforeCompletion(self))
      File "/home/kirr/src/wendelin/venv/z-dev/local/lib/python2.7/site-packages/transaction/weakset.py", line 61, in map
        f(elt)
      File "/home/kirr/src/wendelin/venv/z-dev/local/lib/python2.7/site-packages/transaction/_transaction.py", line 298, in <lambda>
        self._synchronizers.map(lambda s: s.beforeCompletion(self))
      File "/home/kirr/src/wendelin/wendelin.core/bigfile/file_zodb.py", line 671, in beforeCompletion
        assert txn is zconn.transaction_manager.get()
    AssertionError

What is happening here is that one thread used the connection and
ZBigFile/_ZBigFileH associated with it, then the connection was closed
and released to DB pool. Then the connection was reopened but by another
thread and thus with different TransactionManager instance and oops -
_ZBigFileH.transaction_manager is different because it is
TransactionManager instance that was used by the first thread.

Fix it by resyncing _ZBigFileH.transaction_manager on every
connection reopen. No new test as existing tests already cover the
problem when run with ZODB >= 5.5.0 .
parent 89fb8992
...@@ -596,9 +596,6 @@ class _ZBigFileH(object): ...@@ -596,9 +596,6 @@ class _ZBigFileH(object):
# FIXME zfile._p_jar could be None (ex. ZBigFile is newly created # FIXME zfile._p_jar could be None (ex. ZBigFile is newly created
# before first commit) # before first commit)
# IDataManager requires .transaction_manager
self.transaction_manager = zfile._p_jar.transaction_manager
# when connection will be reopened -> txn_manager.registerSynch(self) # when connection will be reopened -> txn_manager.registerSynch(self)
zfile._p_jar.onOpenCallback(self) # -> self.on_connection_open() zfile._p_jar.onOpenCallback(self) # -> self.on_connection_open()
...@@ -608,21 +605,41 @@ class _ZBigFileH(object): ...@@ -608,21 +605,41 @@ class _ZBigFileH(object):
def on_connection_open(self): def on_connection_open(self):
# IDataManager requires .transaction_manager
#
# resync txn manager every time a connection is (re)opened as even for
# the same connection, the data manager can be different for each reopen.
self.transaction_manager = self.zfile._p_jar.transaction_manager
# when connection is closed -> txn_manager.unregisterSynch(self) # when connection is closed -> txn_manager.unregisterSynch(self)
# NOTE close callbacks are fired once, and thus we have to re-register # NOTE close callbacks are fired once, and thus we have to re-register
# it on every open. # it on every open.
self.zfile._p_jar.onCloseCallback(self.on_connection_close) self.zfile._p_jar.onCloseCallback(self.on_connection_close)
# attach us to _current_ _thread_ TM (staying in sync with Connection): # attach us to Connection's transaction manager:
# #
# Hook into txn_manager so that we get a chance to run before # Hook into txn_manager so that we get a chance to run before
# transaction.commit(). (see .beforeCompletion() with more details) # transaction.commit(). (see .beforeCompletion() with more details)
#
# NOTE before ZODB < 5.5.0 Connection.transaction_manager is
# ThreadTransactionManager which implicitly uses separate
# TransactionManager for each thread. We are thus attaching to
# _current_ _thread_ TM and correctness depends on the fact that
# .transaction_manager is further used from the same thread only.
#
# Starting from ZODB >= 5.5.0 this issue has been fixed:
# https://github.com/zopefoundation/ZODB/commit/b6ac40f153
# https://github.com/zopefoundation/ZODB/issues/208
self.transaction_manager.registerSynch(self) self.transaction_manager.registerSynch(self)
def on_connection_close(self): def on_connection_close(self):
# detach us from _current_ _thread_ TM (staying in sync with Connection) # detach us from connection's transaction manager.
# (NOTE it is _current_ _thread_ TM for ZODB < 5.5.0: see notes ^^^)
#
# make sure we stay unlinked from txn manager until the connection is reopened.
self.transaction_manager.unregisterSynch(self) self.transaction_manager.unregisterSynch(self)
self.transaction_manager = None
# NOTE open callbacks are setup once and fire on every open - we don't # NOTE open callbacks are setup once and fire on every open - we don't
# need to resetup them here. # need to resetup them here.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment