Commit 4c747b1b authored by Jim Fulton's avatar Jim Fulton

Fixed a serious bug:

  Packing a blob-enabled file storage in a ZEO server caused blob data
  to be lost.
parent 6dd1e1e1
...@@ -22,6 +22,15 @@ New Features ...@@ -22,6 +22,15 @@ New Features
XXX There are known issues with this implementation that need to be XXX There are known issues with this implementation that need to be
sorted out before it is "released". sorted out before it is "released".
3.9.0a11 (2009-02-17)
=====================
Bugs Fixed
----------
- Packing a blob-enabled file storage in a ZEO server caused blob data
to be lost.
3.9.0a10 (2009-01-05) 3.9.0a10 (2009-01-05)
===================== =====================
......
...@@ -267,7 +267,7 @@ class GC(FileStorageFormatter): ...@@ -267,7 +267,7 @@ class GC(FileStorageFormatter):
# special case, pack to before creation time # special case, pack to before creation time
continue continue
raise raise
reachable[oid] = pos reachable[oid] = pos
for oid in self.findrefs(pos): for oid in self.findrefs(pos):
if oid not in reachable: if oid not in reachable:
...@@ -343,7 +343,7 @@ class FileStoragePacker(FileStorageFormatter): ...@@ -343,7 +343,7 @@ class FileStoragePacker(FileStorageFormatter):
os.path.join(storage.blob_dir, '.removed'), 'w') os.path.join(storage.blob_dir, '.removed'), 'w')
else: else:
self.pack_blobs = False self.pack_blobs = False
path = storage._file.name path = storage._file.name
self._name = path self._name = path
# We open our own handle on the storage so that much of pack can # We open our own handle on the storage so that much of pack can
...@@ -501,12 +501,22 @@ class FileStoragePacker(FileStorageFormatter): ...@@ -501,12 +501,22 @@ class FileStoragePacker(FileStorageFormatter):
if data and ZODB.blob.is_blob_record(data): if data and ZODB.blob.is_blob_record(data):
# We need to remove the blob record. Maybe we # We need to remove the blob record. Maybe we
# need to remove oid: # need to remove oid:
if h.oid not in self.gc.reachable:
self.blob_removed.write(h.oid.encode('hex')+'\n') # But first, we need to make sure the record
else: # we're looking at isn't a dup of the current
self.blob_removed.write( # record. There's a bug in ZEO blob support that causes
(h.oid+h.tid).encode('hex')+'\n') # duplicate data records.
rpos = self.gc.reachable.get(h.oid)
is_dup = (rpos
and self._read_data_header(rpos).tid == h.tid)
if not is_dup:
if h.oid not in self.gc.reachable:
self.blob_removed.write(
h.oid.encode('hex')+'\n')
else:
self.blob_removed.write(
(h.oid+h.tid).encode('hex')+'\n')
pos += h.recordlen() pos += h.recordlen()
continue continue
......
...@@ -14,16 +14,18 @@ ...@@ -14,16 +14,18 @@
from zope.testing import doctest from zope.testing import doctest
import cPickle
import os import os
import time import time
import transaction import transaction
import unittest import unittest
import ZODB.blob
import ZODB.FileStorage import ZODB.FileStorage
import ZODB.tests.util import ZODB.tests.util
def pack_keep_old(): def pack_keep_old():
"""Should a copy of the database be kept? """Should a copy of the database be kept?
The pack_keep_old constructor argument controls whether a .old file (and .old directory for blobs is kept.) The pack_keep_old constructor argument controls whether a .old file (and .old directory for blobs is kept.)
>>> fs = ZODB.FileStorage.FileStorage('data.fs', blob_dir='blobs') >>> fs = ZODB.FileStorage.FileStorage('data.fs', blob_dir='blobs')
...@@ -61,7 +63,7 @@ The pack_keep_old constructor argument controls whether a .old file (and .old di ...@@ -61,7 +63,7 @@ The pack_keep_old constructor argument controls whether a .old file (and .old di
True True
>>> db.close() >>> db.close()
>>> fs = ZODB.FileStorage.FileStorage('data.fs', blob_dir='blobs', >>> fs = ZODB.FileStorage.FileStorage('data.fs', blob_dir='blobs',
... create=True, pack_keep_old=False) ... create=True, pack_keep_old=False)
>>> db = ZODB.DB(fs) >>> db = ZODB.DB(fs)
...@@ -85,10 +87,43 @@ The pack_keep_old constructor argument controls whether a .old file (and .old di ...@@ -85,10 +87,43 @@ The pack_keep_old constructor argument controls whether a .old file (and .old di
>>> os.path.exists('blobs.old') >>> os.path.exists('blobs.old')
False False
>>> db.close() >>> db.close()
""" """
def pack_with_repeated_blob_records():
"""
There is a bug in ZEO that causes duplicate bloc database records
to be written in a blob store operation. (Maybe this has been
fixed by the time you read this, but there might still be
transactions in the wild that have duplicate records.
>>> fs = ZODB.FileStorage.FileStorage('t', blob_dir='bobs')
>>> db = ZODB.DB(fs)
>>> conn = db.open()
>>> conn.root()[1] = ZODB.blob.Blob()
>>> transaction.commit()
>>> tm = transaction.TransactionManager()
>>> oid = conn.root()[1]._p_oid
>>> blob_record, oldserial = fs.load(oid)
Now, create a transaction with multiple saves:
>>> trans = tm.begin()
>>> fs.tpc_begin(trans)
>>> open('ablob', 'w').write('some data')
>>> _ = fs.store(oid, oldserial, blob_record, '', trans)
>>> _ = fs.storeBlob(oid, oldserial, blob_record, 'ablob', '', trans)
>>> fs.tpc_vote(trans)
>>> fs.tpc_finish(trans)
>>> time.sleep(.01)
>>> db.pack()
>>> conn.sync()
>>> conn.root()[1].open().read()
'some data'
>>> db.close()
"""
def test_suite(): def test_suite():
return unittest.TestSuite(( return unittest.TestSuite((
......
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