Re: [ZODB-Dev] Storm/ZEO deadlocks (was Re: [Zope-dev] [announce] NEO 1.0 - scalable and redundant storage for ZODB)
On Thu, Aug 30, 2012 at 11:19:22AM -0600, Shane Hathaway wrote: > On 08/30/2012 10:14 AM, Marius Gedminas wrote: > >Here's the code to reproduce it: http://pastie.org/4617132 Updated version with more explicit logging and fewer unnecessary things: http://pastie.org/4630898 And here's the output: http://pastie.org/4631136 > >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. It looks like I mixed up the thread numbers when I was writing this up last night, i.e. in the above Thread #2 is the one running work1(), and Thread #1 is the one running work2(). Sorry about that. In my defense, that was the order in which they were printed out, which was the iteration order of the sys._current_frames() dict. > > 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. Thread 1 (i.e. work2) acquires the PostgreSQL lock by issuing that DELETE statement, at which point we haven't started the transaction commit yet, on either thread. On the other hand, the second thread (work1) tries to lock PostgreSQL in the UPDATE statement that happens during store.flush() that happens during tpc_begin() that happens after it's already holding the ZEO lock. So maybe it would be enough to make Storm's StoreDataManager sort before ZEO always, so we always take PostgrsSQL locks before ZEO locks. For the record, adding a store.flush() before transaction.commit() inside work1() makes this particular instance of the deadlock go away (and one of the transactions fail with a TransactionRollbackError: could not serialize access due to concurrent update). Marius Gedminas -- The worst thing about going out is that you're not in your house. -- Kimiko Ross signature.asc Description: Digital signature ___ For more information about ZODB, see http://zodb.org/ ZODB-Dev mailing list - ZODB-Dev@zope.org https://mail.zope.org/mailman/listinfo/zodb-dev
Re: [ZODB-Dev] Storm/ZEO deadlocks (was Re: [Zope-dev] [announce] NEO 1.0 - scalable and redundant storage for ZODB)
On 30 August 2012 19:19, Shane Hathaway 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 >>> 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 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" else: prefix = "aa" return "%s_store_%d" % (prefix, id(self)) http://bazaar.launchpad.net/~storm/storm/trunk/view/head:/storm/zope/zstorm.py#L320 (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) http://zope3.pov.lt/trac/browser/zope.sqlalchemy/trunk/src/zope/sqlalchemy/datamanager.py#L105 (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 early. 701 if self._server_addr is None: 702 raise ClientDisconnected 703 else: 704 return '%s:%s' % (self._storage, self._server_addr) http://zope3.pov.lt/trac/browser/ZODB/trunk/src/ZEO/ClientStorage.py#L698 (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 no
Re: [ZODB-Dev] Storm/ZEO deadlocks (was Re: [Zope-dev] [announce] NEO 1.0 - scalable and redundant storage for ZODB)
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 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 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. Shane ___ For more information about ZODB, see http://zodb.org/ ZODB-Dev mailing list - ZODB-Dev@zope.org https://mail.zope.org/mailman/listinfo/zodb-dev
[ZODB-Dev] Storm/ZEO deadlocks (was Re: [Zope-dev] [announce] NEO 1.0 - scalable and redundant storage for ZODB)
On Wed, Aug 29, 2012 at 06:30:50AM -0400, Jim Fulton wrote: > On Wed, Aug 29, 2012 at 2:29 AM, Marius Gedminas 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 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. I'm too fried right now to understand who's at fault here. Workarounds probably exist (use RelStorage instead of ZEO? Configure Storm to use a lower PostgreSQL transaction isolation level?). Maybe this problem would go away if Storm always went into tpc_begin() before ZEO. I've pinged the people in #storm on FreeNode about this, but haven't filed any bugs yet. Marius Gedminas -- Q: Wanting both frequent updates and stability/support is just wishing for a pony! A: Well, we're riding our ponies to the tune of several billion page views per month. Where's your pony? Oh, you didn't get one? -- http://meta.wikimedia.org/wiki/Wikimedia_Ubuntu_migration_FAQ signature.asc Description: Digital signature ___ For more information about ZODB, see http://zodb.org/ ZODB-Dev mailing list - ZODB-Dev@zope.org https://mail.zope.org/mailman/listinfo/zodb-dev
