Commit cdc6eda0 authored by Kirill Smelkov's avatar Kirill Smelkov

bigfile/zodb: Fix thinko in ZBlk1.setblkdata()

ZBlk1.setblkdata() has logic to detect CHUNKSIZE change, and if so
recreate whole chunktab from scratch for simplicity. There was a thinko
however - len(chunk.data) == CHUNKSIZE is ok and actually very often
happens when data does not have zeroes.

Because of this off-by-1 comparison mistake, ZData objects were
constantly created and thrown out instead of being reused which led to
fast ZODB growth.

Fix it.

/reported-by @Tyagov
parent 9ae42085
...@@ -287,7 +287,7 @@ class ZBlk1(ZBlkBase): ...@@ -287,7 +287,7 @@ class ZBlk1(ZBlkBase):
# first make sure chunktab was previously written with the same CHUNKSIZE # first make sure chunktab was previously written with the same CHUNKSIZE
# (for simplicity we don't allow several chunk sizes to mix) # (for simplicity we don't allow several chunk sizes to mix)
for start, chunk in chunktab.items(): for start, chunk in chunktab.items():
if (start % CHUNKSIZE) or len(chunk.data) >= CHUNKSIZE: if (start % CHUNKSIZE) or len(chunk.data) > CHUNKSIZE:
chunktab.clear() chunktab.clear()
break break
......
...@@ -761,3 +761,50 @@ def test_bigfile_filezodb_fmt_change(): ...@@ -761,3 +761,50 @@ def test_bigfile_filezodb_fmt_change():
file_zodb.ZBlk_fmt_write = fmt_write_save file_zodb.ZBlk_fmt_write = fmt_write_save
dbclose(root) dbclose(root)
# test that ZData are reused for changed chunks in ZBlk1 format
def test_bigfile_zblk1_zdata_reuse():
# set ZBlk_fmt_write to ZBlk1 for this test
fmt_write_save = file_zodb.ZBlk_fmt_write
file_zodb.ZBlk_fmt_write = 'ZBlk1'
try:
_test_bigfile_zblk1_zdata_reuse()
finally:
file_zodb.ZBlk_fmt_write = fmt_write_save
def _test_bigfile_zblk1_zdata_reuse():
root = dbopen()
root['zfile6'] = f = ZBigFile(blksize)
transaction.commit()
fh = f.fileh_open() # TODO + ram
vma = fh.mmap(0, 1)
b = Blk(vma, 0)
# initially empty and zero
assert len(f.blktab) == 0
assert (b == 0).all()
# set all to 1 and save ZData instances
b[:] = 1
assert (b == 1).all()
transaction.commit()
assert len(f.blktab) == 1
zblk0_v1 = f.blktab[0]
assert len(zblk0_v1.chunktab) == blksize / file_zodb.ZBlk1.CHUNKSIZE
zdata_v1 = zblk0_v1.chunktab.values()
# set all to 2 and verify ZBlk/ZData instances were reused
b[:] = 2
assert (b == 2).all()
transaction.commit()
assert len(f.blktab) == 1
zblk0_v2 = f.blktab[0]
assert zblk0_v2 is zblk0_v1
assert len(zblk0_v2.chunktab) == blksize / file_zodb.ZBlk1.CHUNKSIZE
zdata_v2 = zblk0_v2.chunktab.values()
assert len(zdata_v1) == len(zdata_v2)
for i in range(len(zdata_v1)):
assert zdata_v1[i] is zdata_v2[i]
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