[Checkins] SVN: relstorage/trunk/relstorage/adapters/ Fixed a MySQL deadlock and moved the open_for_pre_pack method to IConnectionManager.

Shane Hathaway shane at hathawaymix.org
Fri Sep 25 12:52:42 EDT 2009


Log message for revision 104539:
  Fixed a MySQL deadlock and moved the open_for_pre_pack method to IConnectionManager.
  
  The deadlock was caused by a new statement that accidentally
  acquired a share lock on the object_state table outside the
  control of the commit lock.  The statement was incidentally doing
  the wrong thing anyway.  The replacement does not acquire
  a share lock.
  

Changed:
  U   relstorage/trunk/relstorage/adapters/connmanager.py
  U   relstorage/trunk/relstorage/adapters/interfaces.py
  U   relstorage/trunk/relstorage/adapters/locker.py
  U   relstorage/trunk/relstorage/adapters/mysql.py
  U   relstorage/trunk/relstorage/adapters/packundo.py

-=-
Modified: relstorage/trunk/relstorage/adapters/connmanager.py
===================================================================
--- relstorage/trunk/relstorage/adapters/connmanager.py	2009-09-25 15:28:51 UTC (rev 104538)
+++ relstorage/trunk/relstorage/adapters/connmanager.py	2009-09-25 16:52:41 UTC (rev 104539)
@@ -106,3 +106,9 @@
                 self.on_store_opened(cursor, restart=True)
         except self.disconnected_exceptions, e:
             raise StorageError(e)
+
+    def open_for_pre_pack(self):
+        """Open a connection to be used for the pre-pack phase.
+        Returns (conn, cursor).
+        """
+        return self.open()

Modified: relstorage/trunk/relstorage/adapters/interfaces.py
===================================================================
--- relstorage/trunk/relstorage/adapters/interfaces.py	2009-09-25 15:28:51 UTC (rev 104538)
+++ relstorage/trunk/relstorage/adapters/interfaces.py	2009-09-25 16:52:41 UTC (rev 104539)
@@ -78,7 +78,13 @@
         Raise StorageError if the database has disconnected.
         """
 
+    def open_for_pre_pack():
+        """Open a connection to be used for the pre-pack phase.
 
+        Returns (conn, cursor).
+        """
+
+
 class IDatabaseIterator(Interface):
     """Iterate over the available data in the database"""
 
@@ -245,12 +251,6 @@
         May raise UndoError.
         """
 
-    def open_for_pre_pack():
-        """Open a connection to be used for the pre-pack phase.
-
-        Returns (conn, cursor).
-        """
-
     def fill_object_refs(conn, cursor, get_references):
         """Update the object_refs table by analyzing new transactions.
         """

Modified: relstorage/trunk/relstorage/adapters/locker.py
===================================================================
--- relstorage/trunk/relstorage/adapters/locker.py	2009-09-25 15:28:51 UTC (rev 104538)
+++ relstorage/trunk/relstorage/adapters/locker.py	2009-09-25 16:52:41 UTC (rev 104539)
@@ -103,14 +103,15 @@
     implements(ILocker)
 
     def hold_commit_lock(self, cursor, ensure_current=False):
-        cursor.execute("SELECT GET_LOCK(CONCAT(DATABASE(), '.commit'), %s)",
-            (commit_lock_timeout,))
+        stmt = "SELECT GET_LOCK(CONCAT(DATABASE(), '.commit'), %s)"
+        cursor.execute(stmt, (commit_lock_timeout,))
         locked = cursor.fetchone()[0]
         if not locked:
             raise StorageError("Unable to acquire commit lock")
 
     def release_commit_lock(self, cursor):
-        cursor.execute("SELECT RELEASE_LOCK(CONCAT(DATABASE(), '.commit'))")
+        stmt = "SELECT RELEASE_LOCK(CONCAT(DATABASE(), '.commit'))"
+        cursor.execute(stmt)
 
     def hold_pack_lock(self, cursor):
         """Try to acquire the pack lock.

Modified: relstorage/trunk/relstorage/adapters/mysql.py
===================================================================
--- relstorage/trunk/relstorage/adapters/mysql.py	2009-09-25 15:28:51 UTC (rev 104538)
+++ relstorage/trunk/relstorage/adapters/mysql.py	2009-09-25 16:52:41 UTC (rev 104539)
@@ -174,3 +174,18 @@
         """
         return self.open(self.isolation_repeatable_read)
 
+    def open_for_pre_pack(self):
+        """Open a connection to be used for the pre-pack phase.
+        Returns (conn, cursor).
+
+        This overrides a method.
+        """
+        conn, cursor = self.open(transaction_mode=None)
+        try:
+            # This phase of packing works best with transactions
+            # disabled.  It changes no user-facing data.
+            conn.autocommit(True)
+            return conn, cursor
+        except:
+            self.close(conn, cursor)
+            raise

Modified: relstorage/trunk/relstorage/adapters/packundo.py
===================================================================
--- relstorage/trunk/relstorage/adapters/packundo.py	2009-09-25 15:28:51 UTC (rev 104538)
+++ relstorage/trunk/relstorage/adapters/packundo.py	2009-09-25 16:52:41 UTC (rev 104539)
@@ -84,8 +84,10 @@
         """
         self.runner.run_script_stmt(cursor, stmt, {'tid': tid})
 
+        replace_rows = []
         add_rows = []  # [(from_oid, tid, to_oid)]
         for from_oid, state in cursor:
+            replace_rows.append((from_oid,))
             if hasattr(state, 'read'):
                 # Oracle
                 state = state.read()
@@ -102,15 +104,8 @@
                     add_rows.append((from_oid, tid, to_oid))
 
         if not self.keep_history:
-            stmt = """
-            DELETE FROM object_ref
-            WHERE zoid in (
-                SELECT zoid
-                FROM object_state
-                WHERE tid = %(tid)s
-                )
-            """
-            self.runner.run_script(cursor, stmt, {'tid': tid})
+            stmt = "DELETE FROM object_ref WHERE zoid = %s"
+            self.runner.run_many(cursor, stmt, replace_rows)
 
         stmt = """
         INSERT INTO object_ref (zoid, tid, to_zoid)
@@ -207,15 +202,7 @@
                 log.debug('pack: sleeping %.4g second(s)', delay)
                 sleep(delay)
 
-    def open_for_pre_pack(self):
-        """Open a connection to be used for the pre-pack phase.
-        Returns (conn, cursor).
 
-        Subclasses may override this.
-        """
-        return self.connmanager.open()
-
-
 class HistoryPreservingPackUndo(PackUndo):
     implements(IPackUndo)
 
@@ -444,7 +431,7 @@
         even if nothing refers to it.  Packing with pack_gc disabled can be
         much faster.
         """
-        conn, cursor = self.open_for_pre_pack()
+        conn, cursor = self.connmanager.open_for_pre_pack()
         try:
             try:
                 if options.pack_gc:
@@ -808,23 +795,7 @@
         WHERE transaction.empty = true
         """
 
-    def open_for_pre_pack(self):
-        """Open a connection to be used for the pre-pack phase.
-        Returns (conn, cursor).
 
-        This overrides a method.
-        """
-        conn, cursor = self.connmanager.open(transaction_mode=None)
-        try:
-            # This phase of packing works best with transactions
-            # disabled.  It changes no user-facing data.
-            conn.autocommit(True)
-            return conn, cursor
-        except:
-            self.connmanager.close(conn, cursor)
-            raise
-
-
 class OracleHistoryPreservingPackUndo(HistoryPreservingPackUndo):
 
     _script_choose_pack_transaction = """
@@ -908,7 +879,7 @@
                 "history-free storage, so doing nothing")
             return
 
-        conn, cursor = self.open_for_pre_pack()
+        conn, cursor = self.connmanager.open_for_pre_pack()
         try:
             try:
                 self._pre_pack_main(conn, cursor, get_references)



More information about the checkins mailing list