[Zodb-checkins] SVN: ZODB/trunk/src/ZODB/ Fixed a pack bug (or possibly a missfeature). POSKeyErrors were

[Jim Fulton]

Log message for revision 94128:
  Fixed a pack bug (or possibly a missfeature).  POSKeyErrors were
  ignored when doing GC.  That is, when traversing from the root, if a
  reference object wasn't found, it was simply ignored, covering up a
  potential problem.  Now raise a key error, which stops packing.
  

Changed:
  U   ZODB/trunk/src/ZODB/FileStorage/fspack.py
  U   ZODB/trunk/src/ZODB/tests/TransactionalUndoStorage.py

-=-
Modified: ZODB/trunk/src/ZODB/FileStorage/fspack.py
===================================================================
--- ZODB/trunk/src/ZODB/FileStorage/fspack.py	2008-12-16 15:24:08 UTC (rev 94127)
+++ ZODB/trunk/src/ZODB/FileStorage/fspack.py	2008-12-16 16:23:50 UTC (rev 94128)
@@ -248,27 +248,28 @@
 
     def findReachableAtPacktime(self, roots):
         """Mark all objects reachable from the oids in roots as reachable."""
+        reachable = self.reachable
+        oid2curpos = self.oid2curpos
+
         todo = list(roots)
         while todo:
             oid = todo.pop()
-            if self.reachable.has_key(oid):
+            if oid in reachable:
                 continue
 
-            L = []
+            try:
+                pos = oid2curpos[oid]
+            except KeyError:
+                if oid == z64 and len(oid2curpos) == 0:
+                    # special case, pack to before creation time
+                    continue
+                raise
+            
+            reachable[oid] = pos
+            for oid in self.findrefs(pos):
+                if oid not in reachable:
+                    todo.append(oid)
 
-            pos = self.oid2curpos.get(oid)
-            if pos is not None:
-                L.append(pos)
-                todo.extend(self.findrefs(pos))
-
-            if not L:
-                continue
-
-            pos = L.pop()
-            self.reachable[oid] = pos
-            if L:
-                self.reach_ex[oid] = L
-
     def findReachableFromFuture(self):
         # In this pass, the roots are positions of object revisions.
         # We add a pos to extra_roots when there is a backpointer to a

Modified: ZODB/trunk/src/ZODB/tests/TransactionalUndoStorage.py
===================================================================
--- ZODB/trunk/src/ZODB/tests/TransactionalUndoStorage.py	2008-12-16 15:24:08 UTC (rev 94127)
+++ ZODB/trunk/src/ZODB/tests/TransactionalUndoStorage.py	2008-12-16 16:23:50 UTC (rev 94128)
@@ -427,10 +427,23 @@
         self._iterate()
 
     def checkTransactionalUndoAfterPack(self):
-        eq = self.assertEqual
+        # bwarsaw Date: Thu Mar 28 21:04:43 2002 UTC
+        # This is a test which should provoke the underlying bug in
+        # transactionalUndo() on a standby storage.  If our hypothesis
+        # is correct, the bug is in FileStorage, and is caused by
+        # encoding the file position in the `id' field of the undoLog
+        # information.  Note that Full just encodes the tid, but this
+        # is a problem for FileStorage (we have a strategy for fixing
+        # this).
+
+        # So, basically, this makes sure that undo info doesn't depend
+        # on file positions.  We change the file positions in an undo
+        # record by packing.
+        
         # Add a few object revisions
-        oid = self._storage.new_oid()
-        revid1 = self._dostore(oid, data=MinPO(51))
+        oid = '\0'*8
+        revid0 = self._dostore(oid, data=MinPO(50))
+        revid1 = self._dostore(oid, revid=revid0, data=MinPO(51))
         snooze()
         packtime = time.time()
         snooze()                # time.time() now distinct from packtime
@@ -438,7 +451,7 @@
         self._dostore(oid, revid=revid2, data=MinPO(53))
         # Now get the undo log
         info = self._storage.undoInfo()
-        eq(len(info), 3)
+        self.assertEqual(len(info), 4)
         tid = info[0]['id']
         # Now pack just the initial revision of the object.  We need the
         # second revision otherwise we won't be able to undo the third
@@ -446,18 +459,18 @@
         self._storage.pack(packtime, referencesf)
         # Make some basic assertions about the undo information now
         info2 = self._storage.undoInfo()
-        eq(len(info2), 2)
+        self.assertEqual(len(info2), 2)
         # And now attempt to undo the last transaction
         t = Transaction()
         self._storage.tpc_begin(t)
         tid, oids = self._storage.undo(tid, t)
         self._storage.tpc_vote(t)
         self._storage.tpc_finish(t)
-        eq(len(oids), 1)
-        eq(oids[0], oid)
+        self.assertEqual(len(oids), 1)
+        self.assertEqual(oids[0], oid)
         data, revid = self._storage.load(oid, '')
         # The object must now be at the second state
-        eq(zodb_unpickle(data), MinPO(52))
+        self.assertEqual(zodb_unpickle(data), MinPO(52))
         self._iterate()
 
     def checkTransactionalUndoAfterPackWithObjectUnlinkFromRoot(self):