Skip to content

Commit 49f5959

Browse files
committed
DemoStorage: Add support for deleteObject (IExternalGC)
So that it is generally possible to zodbrestore[1,2] zodbdumped[3] database/transactions into DemoStorage. Because without this patch, when dump contains deletion records, e.g. txn 0285cbacc06d3a4c " " user "" description "" extension "" obj 0000000000000007 delete it fails like this: Traceback (most recent call last): ... File "/home/kirr/src/wendelin/z/zodbtools/zodbtools/zodbrestore.py", line 94, in main zodbrestore(stor, asbinstream(sys.stdin), _) File "/home/kirr/src/wendelin/z/zodbtools/zodbtools/zodbrestore.py", line 43, in zodbrestore zodbcommit(stor, at, txn) File "/home/kirr/src/wendelin/z/zodbtools/zodbtools/zodbcommit.py", line 131, in zodbcommit _() File "/home/kirr/src/wendelin/z/zodbtools/zodbtools/zodbcommit.py", line 118, in _ stor.deleteObject(obj.oid, current_serial(obj.oid), txn) AttributeError: 'DemoStorage' object has no attribute 'deleteObject' demo_test.go:105: demo:(file:///tmp/demo470143633/δ0285cbac258bf266/base.fs)/(file:///tmp/demo470143633/δ0285cbac258bf266/δ.fs): zpyrestore: exit status 1 To be able to implement DemoStorage.deleteObject, we have to fix FileStorage.deleteObject first: currently FileStorage.deleteObject raises POSKeyError if an object is not present in the database, but for situation where - demo=base+δ, - object is present in base, and - object is not present in δ it creates a problem because there is no way to add whiteout record for the object into δ. This change should be generally good; let me quote https://lab.nexedi.com/kirr/neo/commit/543041a3 for why: ---- 8< ---- Even though currently it is not possible to create such data record via FileStorage(py).deleteObject (because it raises POSKeyError if there is no previous object revision), being able to use such data records is semantically useful in overlayed DemoStorage settings, where δ part marks an object that exists only in base with delete record whiteout. It is also generally meaningful to be able to create "delete" record even if object was not previously existing: "deleteObject" is actually similar to "store" (and so should be better named as "storeDelete"). If one wants to store deletion, there should not be a reason to reject it, because deleteObject already has seatbelt in the form of oldserial, and if the user calls deleteObject(oid, oldserial=z64), he/she is already telling that "I know that this object does not exist in this storage (oldserial=z64), but still please create a deletion record for it". Once again this is useful in overlayed DemoStorage settings described above. For the reference, such whiteout deletion records pass ZODB/scripts/fstest just fine. ---- 8< ---- DemoStorage.deleteObject implementation is straightforward, but builds on loadAt[4] as precondition. /cc @jimfulton, @jamadden, @perrinjerome [1] https://lab.nexedi.com/nexedi/zodbtools/blob/129afa67/zodbtools/zodbrestore.py [2] https://lab.nexedi.com/nexedi/zodbtools/merge_requests/19 [3] https://lab.nexedi.com/nexedi/zodbtools/blob/129afa67/zodbtools/zodbdump.py [4] zopefoundation#323
1 parent 55261f3 commit 49f5959

File tree

3 files changed

+52
-3
lines changed

3 files changed

+52
-3
lines changed

src/ZODB/DemoStorage.py

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
# -*- coding: utf-8 -*-
12
##############################################################################
23
#
34
# Copyright (c) Zope Corporation and Contributors.
@@ -108,11 +109,16 @@ def __init__(self, name=None, base=None, changes=None,
108109
if close_changes_on_close is None:
109110
close_changes_on_close = False
110111
else:
112+
self._temporary_changes = False
111113
if ZODB.interfaces.IBlobStorage.providedBy(changes):
112114
zope.interface.alsoProvides(self, ZODB.interfaces.IBlobStorage)
113115
if close_changes_on_close is None:
114116
close_changes_on_close = True
115117

118+
if ZODB.interfaces.IExternalGC.providedBy(changes):
119+
zope.interface.alsoProvides(self, ZODB.interfaces.IExternalGC)
120+
self.deleteObject = self._storeDelete
121+
116122
self.changes = changes
117123
self.close_changes_on_close = close_changes_on_close
118124

@@ -376,6 +382,40 @@ def store(self, oid, serial, data, version, transaction):
376382
else:
377383
self.changes.store(oid, serial, data, '', transaction)
378384

385+
# _storeDelete serves deleteObject when .changes implements IExternalGC.
386+
def _storeDelete(self, oid, oldserial, transaction):
387+
if transaction is not self._transaction:
388+
raise ZODB.POSException.StorageTransactionError(self, transaction)
389+
390+
# oldserial ∈ changes -> changes.deleteObject
391+
baseHead = self.base.lastTransaction()
392+
if oldserial > baseHead:
393+
self.changes.deleteObject(oid, oldserial, transaction)
394+
return
395+
396+
# oldserial ∈ base -> find out it is indeed the latest there and then
397+
# call changes.deleteObject(oldserial=z64)
398+
399+
_, serial = ZODB.utils.loadAt(self.changes, oid, self.changes.lastTransaction())
400+
if serial != ZODB.utils.z64:
401+
# object has data or deletion record in changes
402+
raise ZODB.POSException.ConflictError(oid=oid, serials=(serial, oldserial))
403+
404+
_, serial = ZODB.utils.loadAt(self.base, oid, baseHead)
405+
if serial != oldserial:
406+
raise ZODB.POSException.ConflictError(oid=oid, serials=(serial, oldserial))
407+
408+
# object has no data/deletion record in changes and its latest revision in base == oldserial
409+
# -> changes.deleteObject(oldserial=z64) + correct oldserial back on conflict.
410+
try:
411+
self.changes.deleteObject(oid, ZODB.utils.z64, transaction)
412+
except ZODB.POSException.ConflictError as e:
413+
assert len(e.serials) == 2
414+
assert e.serials[1] == ZODB.utils.z64
415+
e.serials = (e.serials[0], oldserial)
416+
raise
417+
418+
379419
def storeBlob(self, oid, oldserial, data, blobfilename, version,
380420
transaction):
381421
assert version=='', "versions aren't supported"

src/ZODB/FileStorage/FileStorage.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -661,9 +661,10 @@ def deleteObject(self, oid, oldserial, transaction):
661661
with self._lock:
662662
old = self._index_get(oid, 0)
663663
if not old:
664-
raise POSKeyError(oid)
665-
h = self._read_data_header(old, oid)
666-
committed_tid = h.tid
664+
committed_tid = z64
665+
else:
666+
h = self._read_data_header(old, oid)
667+
committed_tid = h.tid
667668

668669
if oldserial != committed_tid:
669670
raise ConflictError(

src/ZODB/tests/testDemoStorage.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -384,4 +384,12 @@ def test_suite():
384384
suite.addTest(unittest.makeSuite(DemoStorageWrappedAroundHexMappingStorage,
385385
'check'))
386386
suite.addTest(unittest.makeSuite(DemoStorageTests2, 'check'))
387+
388+
def demo_for_gctest():
389+
from ZODB.FileStorage import FileStorage
390+
base = ZODB.FileStorage.FileStorage('data.fs', blob_dir="data_blobs")
391+
changes = ZODB.FileStorage.FileStorage('changes.fs', blob_dir="changes_blobs", pack_gc=False)
392+
return ZODB.DemoStorage.DemoStorage(base=base, changes=changes)
393+
suite.addTest(PackableStorage.IExternalGC_suite(demo_for_gctest))
394+
387395
return suite

0 commit comments

Comments
 (0)