Commit e6f505df authored by Tim Peters's avatar Tim Peters

Merge rev 29350 from ZODB 3.3 branch.

Port from ZODB 3.2.

Change FileStorage .restore() and .store() to update max oid in use.

This is the last of the checkins to fix critical bugs involving rare cases
where a FileStorage could end up reusing old oids for new objects.
parent ed9ebe85
...@@ -85,6 +85,18 @@ depending on platform, and should suffer far fewer (if any) intermittent ...@@ -85,6 +85,18 @@ depending on platform, and should suffer far fewer (if any) intermittent
FileStorage FileStorage
----------- -----------
- The ``.store()`` and ``.restore()`` methods didn't update the storage's
belief about the largest oid in use when passed an oid larger than the
largest oid the storage already knew about. Because ``.restore()`` in
particular is used by ``copyTransactionsFrom()``, and by the first stage
of ZRS recovery, a large database could be created that believed the only
oid in use was oid 0 (the special oid reserved for the root object). In
rare cases, it could go on from there assigning duplicate oids to new
objects, starting over from oid 1 again. This has been repaired. A
new ``set_max_oid()`` method was added to the ``BaseStorage`` class so
that derived storages can update the largest oid in use in a threadsafe
way.
- A FileStorage's index file tried to maintain the index's largest oid as a - A FileStorage's index file tried to maintain the index's largest oid as a
separate piece of data, incrementally updated over the storage's lifetime. separate piece of data, incrementally updated over the storage's lifetime.
This scheme was more complicated than necessary, so was also more brittle This scheme was more complicated than necessary, so was also more brittle
......
...@@ -157,6 +157,17 @@ class BaseStorage(UndoLogCompatible): ...@@ -157,6 +157,17 @@ class BaseStorage(UndoLogCompatible):
if d < 255: return last[:-1]+chr(d+1)+'\0'*(8-len(last)) if d < 255: return last[:-1]+chr(d+1)+'\0'*(8-len(last))
else: return self.new_oid(last[:-1]) else: return self.new_oid(last[:-1])
# Update the maximum oid in use, under protection of a lock. The
# maximum-in-use attribute is changed only if possible_new_max_oid is
# larger than its current value.
def set_max_oid(self, possible_new_max_oid):
self._lock_acquire()
try:
if possible_new_max_oid > self._oid:
self._oid = possible_new_max_oid
finally:
self._lock_release()
def registerDB(self, db, limit): def registerDB(self, db, limit):
pass # we don't care pass # we don't care
...@@ -371,10 +382,7 @@ class BaseStorage(UndoLogCompatible): ...@@ -371,10 +382,7 @@ class BaseStorage(UndoLogCompatible):
# using store(). However, if we use store, then # using store(). However, if we use store, then
# copyTransactionsFrom() may fail with VersionLockError or # copyTransactionsFrom() may fail with VersionLockError or
# ConflictError. # ConflictError.
if hasattr(self, 'restore'): restoring = hasattr(self, 'restore')
restoring = 1
else:
restoring = 0
fiter = other.iterator() fiter = other.iterator()
for transaction in fiter: for transaction in fiter:
tid=transaction.tid tid=transaction.tid
......
...@@ -632,6 +632,8 @@ class FileStorage(BaseStorage.BaseStorage, ...@@ -632,6 +632,8 @@ class FileStorage(BaseStorage.BaseStorage,
self._lock_acquire() self._lock_acquire()
try: try:
if oid > self._oid:
self.set_max_oid(oid)
old = self._index_get(oid, 0) old = self._index_get(oid, 0)
cached_tid = None cached_tid = None
pnv = None pnv = None
...@@ -758,6 +760,8 @@ class FileStorage(BaseStorage.BaseStorage, ...@@ -758,6 +760,8 @@ class FileStorage(BaseStorage.BaseStorage,
self._lock_acquire() self._lock_acquire()
try: try:
if oid > self._oid:
self.set_max_oid(oid)
prev_pos = 0 prev_pos = 0
if prev_txn is not None: if prev_txn is not None:
prev_txn_pos = self._txn_find(prev_txn, 0) prev_txn_pos = self._txn_find(prev_txn, 0)
......
...@@ -205,6 +205,39 @@ class FileStorageTests( ...@@ -205,6 +205,39 @@ class FileStorageTests(
self.failUnless(self._storage._records_before_save > 20) self.failUnless(self._storage._records_before_save > 20)
def checkStoreBumpsOid(self):
# If .store() is handed an oid bigger than the storage knows
# about already, it's crucial that the storage bump its notion
# of the largest oid in use.
t = transaction.Transaction()
self._storage.tpc_begin(t)
giant_oid = '\xee' * 8
# Store an object.
# oid, serial, data, version, transaction
r1 = self._storage.store(giant_oid, '\0'*8, 'data', '', t)
# Finish the transaction.
r2 = self._storage.tpc_vote(t)
self._storage.tpc_finish(t)
# Before ZODB 3.2.6, this failed, with ._oid == z64.
self.assertEqual(self._storage._oid, giant_oid)
def checkRestoreBumpsOid(self):
# As above, if .restore() is handed an oid bigger than the storage
# knows about already, it's crucial that the storage bump its notion
# of the largest oid in use. Because copyTransactionsFrom(), and
# ZRS recovery, use the .restore() method, this is plain critical.
t = transaction.Transaction()
self._storage.tpc_begin(t)
giant_oid = '\xee' * 8
# Store an object.
# oid, serial, data, version, prev_txn, transaction
r1 = self._storage.restore(giant_oid, '\0'*8, 'data', '', None, t)
# Finish the transaction.
r2 = self._storage.tpc_vote(t)
self._storage.tpc_finish(t)
# Before ZODB 3.2.6, this failed, with ._oid == z64.
self.assertEqual(self._storage._oid, giant_oid)
def checkCorruptionInPack(self): def checkCorruptionInPack(self):
# This sets up a corrupt .fs file, with a redundant transaction # This sets up a corrupt .fs file, with a redundant transaction
# length mismatch. The implementation of pack in many releases of # length mismatch. The implementation of pack in many releases of
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment