On 30 August 2012 19:19, Shane Hathaway <sh...@hathawaymix.org> wrote:
> On 08/30/2012 10:14 AM, Marius Gedminas wrote:
>> On Wed, Aug 29, 2012 at 06:30:50AM -0400, Jim Fulton wrote:
>>> On Wed, Aug 29, 2012 at 2:29 AM, Marius Gedminas <mar...@gedmin.as>
>>> wrote:
>>>> On Tue, Aug 28, 2012 at 06:31:05PM +0200, Vincent Pelletier wrote:
>>>>> On Tue, 28 Aug 2012 16:31:20 +0200,
>>>>> Martijn Pieters <m...@zopatista.com> wrote :
>>>>>> Anything else different? Did you make any performance comparisons
>>>>>> between RelStorage and NEO?
>>>>> I believe the main difference compared to all other ZODB Storage
>>>>> implementation is the finer-grained locking scheme: in all storage
>>>>> implementations I know, there is a database-level lock during the
>>>>> entire second phase of 2PC, whereas in NEO transactions are serialised
>>>>> only when they alter a common set of objects.
>>>> This could be a compelling point.  I've seen deadlocks in an app that
>>>> tried to use both ZEO and PostgreSQL via the Storm ORM.  (The thread
>>>> holding the ZEO commit lock was blocked waiting for the PostgreSQL
>>>> commit to finish, while the PostgreSQL server was waiting for some other
>>>> transaction to either commit or abort -- and that other transaction
>>>> couldn't proceed because it was waiting for the ZEO lock.)
>>> This sounds like an application/transaction configuration problem.
>> *shrug*
>> Here's the code to reproduce it: http://pastie.org/4617132
>>> To avoid this sort of deadlock, you need to always commit in a
>>> a consistent order.  You also need to configure ZEO (or NEO)
>>> to time-out transactions that take too long to finish the second phase.
>> The deadlock happens in tpc_begin() in both threads, which is the first
>> phase, AFAIU.
>> AFAICS Thread #2 first performs tpc_begin() for ClientStorage and takes
>> the ZEO commit lock.  Then it enters tpc_begin() for Storm's
>> StoreDataManager and blocks waiting for a response from PostgreSQL --
>> which is delayed because the PostgreSQL server is waiting to see if
>> the other thread, Thread #1, will commit or abort _its_ transaction, which
>> is conflicting with the one from Thread #2.
>> Meanwhile Thread #1 is blocked in ZODB's tpc_begin(), trying to acquire
>> the
>> ZEO commit lock held by Thread #2.
> So thread 1 acquires in this order:
> 1. PostgreSQL
> 2. ZEO
> Thread 2 acquires in this order:
> 1. ZEO
> 2. PostgreSQL
> SQL databases handle deadlocks by detecting and automatically rolling back
> transactions, while the "transaction" package expects all data managers to
> completely avoid deadlocks using the sortKey method.
> I haven't looked at the code, but I imagine Storm's StoreDataManager
> implements IDataManager.  I wonder if StoreDataManager provides a consistent
> sortKey.  The sortKey method must return a string (not an integer or other
> object) that is consistent yet different from all other participating data
> managers.

Storm's DataManager defines sortKey as:

    def sortKey(self):
        # Stores in TPC mode should be the last to be committed, this makes
        # it possible to have TPC behavior when there's only a single store
        # not in TPC mode.
        if self._store._tpc:
            prefix = "zz"
            prefix = "aa"
        return "%s_store_%d" % (prefix, id(self))


(By default self._store._tpc is set to False.)

This is essentially similar to zope.sqlalchemy's, the single phase
variant being:

105         def sortKey(self):
106             # Try to sort last, so that we vote last - we may commit
in tpc_vote(),
107             # which allows Zope to roll back its transaction if the RDBMS
108             # threw a conflict error.
109             return "~sqlalchemy:%d" % id(self.tx)


(The TPC variant simply omits the leading tilde as it is not required
to sort last - zope.sqlalchemy commits in tpc_vote() rather than
tpc_finish() when using single phase commit.)

ZEO's sortKey is:

698         def sortKey(self):
699             # If the client isn't connected to anything, it can't have a
700             # valid sortKey().  Raise an error to stop the transaction 
701             if self._server_addr is None:
702                 raise ClientDisconnected
703             else:
704                 return '%s:%s' % (self._storage, self._server_addr)


(self._storage defaults to the string '1'.)

This should mean that ZEO always gets a sortKey like '1:./zeosock' in
the example given whereas Storm gets a sortKey like 'aa_storm_12345'
(though the final number will vary per transaction.) Which should mean
a consistent sort order and ZEO always committing first.

It seems StormDataManager only commits in tpc_finish, doing nothing in
either of commit() or tpc_vote() stages when in 1PC mode. As ZEO sorts
first a failure to commit by Storm could never abort the ZEO server's
transaction. Postgres can raise the equivalent of a ConflictError too.
That situation would lead to transaction logging a critical error and
raising the exception. The ZEO transaction would already have finished
by then


p.s. It would be interesting to try a zope.sqlalchemy equivalent of
the storm deadlock maker to see whether it holds up any better.
For more information about ZODB, see http://zodb.org/

ZODB-Dev mailing list  -  ZODB-Dev@zope.org

Reply via email to