Commit 968efe37 authored by Tim Peters's avatar Tim Peters

Bugfix for pack(). Approximately forever, we've seen extremely rare

failures in the ZEO FileStorage pack-while-writing tests, where they die
with
    CorruptedError: ... transaction with checkpoint flag set

By coincidence, the same kind of death during pack() was reported on
zodb-dev today(!).  The new checkPackLotsWhileWriting test provokes this
on my box reliably (failed each time it was run).

The problem appears to be that pack's idea of where a FileStorage
ends was obtained merely by seeking to the end of the file.  But if
a transaction is in progress, there can be an extra "not really there
yet" transaction at the end of the file, and when either buildPackIndex()
or findReachableFromFuture() bumped into a such a thing, the CorruptedError
above got raised.  This is always rare, and was exceedingly rare before
because only one pack per test was tried.  The new test tries packing
repeatedly while a thread keeps hammering the database, so is much more
likely to provoke the problem.

The fix amounts to passing FileStorage._pos to the pack code, telling
the latter directly where the legit transactions in the file end.
parent ba8132de
...@@ -13,6 +13,16 @@ than the first call, data loss could occur. The bug was fixed by ...@@ -13,6 +13,16 @@ than the first call, data loss could occur. The bug was fixed by
causing the second call to raise a StorageError before performing any causing the second call to raise a StorageError before performing any
work. work.
Fixed a rare bug in pack: if a pack started during a small window of
time near the end of a concurrent transaction's commit, it was possible
for the pack attempt to raise a spurious
CorruptedError: ... transaction with checkpoint flag set
exception. This did no damage to the database, or to the transaction
in progress, but no pack was performed then.
ZEO ZEO
--- ---
......
...@@ -13,7 +13,7 @@ ...@@ -13,7 +13,7 @@
############################################################################## ##############################################################################
"""Storage implementation using a log written to a single file. """Storage implementation using a log written to a single file.
$Revision: 1.10 $ $Revision: 1.11 $
""" """
import base64 import base64
...@@ -1319,13 +1319,15 @@ class FileStorage(BaseStorage.BaseStorage, ...@@ -1319,13 +1319,15 @@ class FileStorage(BaseStorage.BaseStorage,
if self._pack_is_in_progress: if self._pack_is_in_progress:
raise FileStorageError, 'Already packing' raise FileStorageError, 'Already packing'
self._pack_is_in_progress = True self._pack_is_in_progress = True
current_size = self.getSize()
finally: finally:
self._lock_release() self._lock_release()
p = FileStoragePacker(self._file_name, stop, p = FileStoragePacker(self._file_name, stop,
self._lock_acquire, self._lock_release, self._lock_acquire, self._lock_release,
self._commit_lock_acquire, self._commit_lock_acquire,
self._commit_lock_release) self._commit_lock_release,
current_size)
try: try:
opos = p.pack() opos = p.pack()
if opos is None: if opos is None:
......
...@@ -411,14 +411,15 @@ class FileStoragePacker(FileStorageFormatter): ...@@ -411,14 +411,15 @@ class FileStoragePacker(FileStorageFormatter):
# stop is the pack time, as a TimeStamp. # stop is the pack time, as a TimeStamp.
# la and lr are the acquire() and release() methods of the storage's lock. # la and lr are the acquire() and release() methods of the storage's lock.
# cla and clr similarly, for the storage's commit lock. # cla and clr similarly, for the storage's commit lock.
def __init__(self, path, stop, la, lr, cla, clr): # current_size is the storage's _pos. All valid data at the start
# lives before that offset (there may be a checkpoint transaction in
# progress after it).
def __init__(self, path, stop, la, lr, cla, clr, current_size):
self._name = path self._name = path
self._file = open(path, "rb") self._file = open(path, "rb")
self._stop = stop self._stop = stop
self.locked = 0 self.locked = 0
self._file.seek(0, 2) self.file_end = current_size
self.file_end = self._file.tell()
self._file.seek(0)
self.gc = GC(self._file, self.file_end, self._stop) self.gc = GC(self._file, self.file_end, self._stop)
......
...@@ -260,8 +260,7 @@ class PackableStorage(PackableStorageBase): ...@@ -260,8 +260,7 @@ class PackableStorage(PackableStorageBase):
def checkPackNowWhileWriting(self): def checkPackNowWhileWriting(self):
self._PackWhileWriting(pack_now=True) self._PackWhileWriting(pack_now=True)
# XXX Disabled because it always fails now. def checkPackLotsWhileWriting(self):
def XXXcheckPackLotsWhileWriting(self):
# This is like the other pack-while-writing tests, except it packs # This is like the other pack-while-writing tests, except it packs
# repeatedly until the client thread is done. At the time it was # repeatedly until the client thread is done. At the time it was
# introduced, it reliably provoked # introduced, it reliably provoked
......
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