[Checkins] SVN: relstorage/trunk/ Fixed a bug that caused OIDs to be reused after importing objects.

Shane Hathaway shane at hathawaymix.org
Wed Feb 20 03:51:36 EST 2008


Log message for revision 84069:
  Fixed a bug that caused OIDs to be reused after importing objects.
  Added a corresponding test.
  Also renamed _zap() and zap() to zap_all(), making it a published
  interface with a more dangerous sounding name.
  

Changed:
  U   relstorage/trunk/CHANGELOG.txt
  U   relstorage/trunk/relstorage/adapters/mysql.py
  U   relstorage/trunk/relstorage/adapters/oracle.py
  U   relstorage/trunk/relstorage/adapters/postgresql.py
  U   relstorage/trunk/relstorage/relstorage.py
  U   relstorage/trunk/relstorage/tests/reltestbase.py
  U   relstorage/trunk/relstorage/tests/speedtest.py

-=-
Modified: relstorage/trunk/CHANGELOG.txt
===================================================================
--- relstorage/trunk/CHANGELOG.txt	2008-02-20 03:08:20 UTC (rev 84068)
+++ relstorage/trunk/CHANGELOG.txt	2008-02-20 08:51:35 UTC (rev 84069)
@@ -41,7 +41,10 @@
 - Implemented the storage iterator protocol, making it possible to copy
   transactions to and from FileStorage and other RelStorage instances.
 
+- Fixed a bug that caused OIDs to be reused after importing objects.
+  Added a corresponding test.
 
+
 PGStorage 0.4
 
 - Began using the PostgreSQL LISTEN and NOTIFY statements as a shortcut

Modified: relstorage/trunk/relstorage/adapters/mysql.py
===================================================================
--- relstorage/trunk/relstorage/adapters/mysql.py	2008-02-20 03:08:20 UTC (rev 84068)
+++ relstorage/trunk/relstorage/adapters/mysql.py	2008-02-20 08:51:35 UTC (rev 84069)
@@ -172,11 +172,8 @@
             self.close(conn, cursor)
 
 
-    def zap(self):
-        """Clear all data out of the database.
-
-        Used by the test suite.
-        """
+    def zap_all(self):
+        """Clear all data out of the database."""
         conn, cursor = self.open()
         try:
             try:
@@ -487,6 +484,10 @@
         WHERE tid = %s
         """, (tid,))
 
+    def set_min_oid(self, cursor, oid):
+        """Ensure the next OID is at least the given OID."""
+        cursor.execute("REPLACE INTO new_oid VALUES(%s)", (oid,))
+
     def commit_phase1(self, cursor, tid):
         """Begin a commit.  Returns the transaction name.
 

Modified: relstorage/trunk/relstorage/adapters/oracle.py
===================================================================
--- relstorage/trunk/relstorage/adapters/oracle.py	2008-02-20 03:08:20 UTC (rev 84068)
+++ relstorage/trunk/relstorage/adapters/oracle.py	2008-02-20 08:51:35 UTC (rev 84069)
@@ -226,11 +226,8 @@
             self.close(conn, cursor)
 
 
-    def zap(self):
-        """Clear all data out of the database.
-
-        Used by the test suite.
-        """
+    def zap_all(self):
+        """Clear all data out of the database."""
         conn, cursor = self.open()
         try:
             try:
@@ -580,6 +577,28 @@
         """
         cursor.execute(stmt, (tid,))
 
+    def set_min_oid(self, cursor, oid):
+        """Ensure the next OID is at least the given OID."""
+        next_oid = self.new_oid(cursor)
+        if next_oid < oid:
+            # Oracle provides no way modify the sequence value
+            # except through alter sequence or drop/create sequence,
+            # but either statement kills the current transaction.
+            # Therefore, open a temporary connection to make the
+            # alteration.
+            conn2, cursor2 = self.open()
+            try:
+                # Change the sequence by altering the increment.
+                # (this is safer than dropping and re-creating the sequence)
+                diff = oid - next_oid
+                cursor2.execute(
+                    "ALTER SEQUENCE zoid_seq INCREMENT BY %d" % diff)
+                cursor2.execute("SELECT zoid_seq.nextval FROM DUAL")
+                cursor2.execute("ALTER SEQUENCE zoid_seq INCREMENT BY 1")
+                conn2.commit()
+            finally:
+                self.close(conn2, cursor2)
+
     def commit_phase1(self, cursor, tid):
         """Begin a commit.  Returns the transaction name.
 

Modified: relstorage/trunk/relstorage/adapters/postgresql.py
===================================================================
--- relstorage/trunk/relstorage/adapters/postgresql.py	2008-02-20 03:08:20 UTC (rev 84068)
+++ relstorage/trunk/relstorage/adapters/postgresql.py	2008-02-20 08:51:35 UTC (rev 84069)
@@ -149,11 +149,8 @@
             self.close(conn, cursor)
 
 
-    def zap(self):
-        """Clear all data out of the database.
-
-        Used by the test suite.
-        """
+    def zap_all(self):
+        """Clear all data out of the database."""
         conn, cursor = self.open()
         try:
             try:
@@ -510,6 +507,15 @@
         )
         """, {'tid': tid})
 
+    def set_min_oid(self, cursor, oid):
+        """Ensure the next OID is at least the given OID."""
+        cursor.execute("""
+        SELECT CASE WHEN %s > nextval('zoid_seq')
+            THEN setval('zoid_seq', %s)
+            ELSE 0
+            END
+        """, (oid, oid))
+
     def commit_phase1(self, cursor, tid):
         """Begin a commit.  Returns the transaction name.
 

Modified: relstorage/trunk/relstorage/relstorage.py
===================================================================
--- relstorage/trunk/relstorage/relstorage.py	2008-02-20 03:08:20 UTC (rev 84068)
+++ relstorage/trunk/relstorage/relstorage.py	2008-02-20 08:51:35 UTC (rev 84069)
@@ -83,6 +83,14 @@
         # be inside a _lock_acquire()/_lock_release() block.
         self._closed = False
 
+        # _max_stored_oid is the highest OID stored by the current
+        # transaction
+        self._max_stored_oid = 0
+
+        # _max_new_oid is the highest OID provided by new_oid()
+        self._max_new_oid = 0
+
+
     def _open_load_connection(self):
         """Open the load connection to the database.  Return nothing."""
         conn, cursor = self._adapter.open_for_load()
@@ -112,12 +120,12 @@
             self._adapter.restart_load(self._load_cursor)
             self._load_transaction_open = True
 
-    def _zap(self):
-        """Clear all objects out of the database.
+    def zap_all(self):
+        """Clear all objects and transactions out of the database.
 
-        Used by the test suite.
+        Used by the test suite and migration scripts.
         """
-        self._adapter.zap()
+        self._adapter.zap_all()
         self._rollback_load_connection()
 
     def close(self):
@@ -263,6 +271,7 @@
 
         self._lock_acquire()
         try:
+            self._max_stored_oid = max(self._max_stored_oid, oid_int)
             # save the data in a temporary table
             adapter.store_temp(cursor, oid_int, prev_tid_int, md5sum, data)
             return None
@@ -297,6 +306,7 @@
 
         self._lock_acquire()
         try:
+            self._max_stored_oid = max(self._max_stored_oid, oid_int)
             # save the data.  Note that md5sum and data can be None.
             adapter.restore(cursor, oid_int, tid_int, md5sum, data)
         finally:
@@ -391,6 +401,7 @@
         # It is assumed that self._lock_acquire was called before this
         # method was called.
         self._prepared_txn = None
+        self._max_stored_oid = 0
 
 
     def _finish_store(self):
@@ -456,11 +467,15 @@
             # the vote phase has already completed
             return
 
-        self._prepare_tid()
-        tid_int = u64(self._tid)
         cursor = self._store_cursor
         assert cursor is not None
 
+        if self._max_stored_oid > self._max_new_oid:
+            self._adapter.set_min_oid(cursor, self._max_stored_oid + 1)
+
+        self._prepare_tid()
+        tid_int = u64(self._tid)
+
         serials = self._finish_store()
         self._adapter.update_current(cursor, tid_int)
         self._prepared_txn = self._adapter.commit_phase1(cursor, tid_int)
@@ -519,6 +534,7 @@
                 self._open_load_connection()
                 cursor = self._load_cursor
             oid_int = self._adapter.new_oid(cursor)
+            self._max_new_oid = max(self._max_new_oid, oid_int)
             return p64(oid_int)
         finally:
             self._lock_release()

Modified: relstorage/trunk/relstorage/tests/reltestbase.py
===================================================================
--- relstorage/trunk/relstorage/tests/reltestbase.py	2008-02-20 03:08:20 UTC (rev 84068)
+++ relstorage/trunk/relstorage/tests/reltestbase.py	2008-02-20 08:51:35 UTC (rev 84069)
@@ -44,7 +44,7 @@
 
     def setUp(self):
         self.open(create=1)
-        self._storage._zap()
+        self._storage.zap_all()
 
     def tearDown(self):
         self._storage.close()
@@ -203,6 +203,16 @@
         self.assertEqual(len(got), len(data))
         self.assertEqual(got, data)
 
+    def checkPreventOIDOverlap(self):
+        # Store an object with a particular OID, then verify that
+        # OID is not reused.
+        data = 'mydata'
+        oid1 = '\0' * 7 + '\x0f'
+        self._dostoreNP(oid1, data=data)
+        oid2 = self._storage.new_oid()
+        self.assert_(oid1 < oid2, 'old OID %r should be less than new OID %r'
+            % (oid1, oid2))
+
     def check16MObject(self):
         # Store 16 * 1024 * 1024 bytes in an object, then retrieve it
         data = 'a 16 byte string' * (1024 * 1024)
@@ -410,7 +420,7 @@
 class ToFileStorage(BaseRelStorageTests, RecoveryStorageSubset):
     def setUp(self):
         self.open(create=1)
-        self._storage._zap()
+        self._storage.zap_all()
         self._dst = FileStorage("Dest.fs", create=True)
 
     def tearDown(self):
@@ -426,7 +436,7 @@
 class FromFileStorage(BaseRelStorageTests, RecoveryStorageSubset):
     def setUp(self):
         self.open(create=1)
-        self._storage._zap()
+        self._storage.zap_all()
         self._dst = self._storage
         self._storage = FileStorage("Source.fs", create=True)
 

Modified: relstorage/trunk/relstorage/tests/speedtest.py
===================================================================
--- relstorage/trunk/relstorage/tests/speedtest.py	2008-02-20 03:08:20 UTC (rev 84068)
+++ relstorage/trunk/relstorage/tests/speedtest.py	2008-02-20 08:51:35 UTC (rev 84069)
@@ -196,7 +196,7 @@
     def postgres_test(self):
         from relstorage.adapters.postgresql import PostgreSQLAdapter
         adapter = PostgreSQLAdapter('dbname=relstoragetest')
-        adapter.zap()
+        adapter.zap_all()
         def make_storage():
             return RelStorage(adapter)
         return self.run_tests(make_storage)
@@ -206,7 +206,7 @@
         from relstorage.tests.testoracle import getOracleParams
         user, password, dsn = getOracleParams()
         adapter = OracleAdapter(user, password, dsn)
-        adapter.zap()
+        adapter.zap_all()
         def make_storage():
             return RelStorage(adapter)
         return self.run_tests(make_storage)
@@ -214,7 +214,7 @@
     def mysql_test(self):
         from relstorage.adapters.mysql import MySQLAdapter
         adapter = MySQLAdapter(db='relstoragetest')
-        adapter.zap()
+        adapter.zap_all()
         def make_storage():
             return RelStorage(adapter)
         return self.run_tests(make_storage)



More information about the Checkins mailing list