fixup! fixup! fixup! fixup! fixup! ZBigFile: Add ZBlk format option 'h' (heuristic) (4)

We were going on the edge of corrupting data. See added comment in
file_zodb.py about how that could close to happen and why the code is
correct. Add more corresponding asserts + cosmetics.

bigfile/file_zodb.py

@@ -593,6 +593,7 @@ class ZBigFile(LivePersistent):
         if zblk is None:
             old_data = b''
         else:
+            assert not zblk._p_changed
             old_data  = bytes(zblk.loadblkdata()).rstrip(b'\0')
         ndelta = memdelta(old_data, new_data)
 
@@ -604,7 +605,7 @@ class ZBigFile(LivePersistent):
         append_oldblk = ((blk == last_blk)   and (new_data[:len(old_data)] == old_data))
         append_newblk = ((blk == last_blk+1) and (len(old_data) == 0))
 
-        append = append_oldblk or append_newblk
+        append = (append_oldblk or append_newblk)
         small  = (ndelta < 0.5*self.blksize)
         filled = (len(new_data) == self.blksize)  # filled full with non-zeros at the end
 
@@ -620,14 +621,18 @@ class ZBigFile(LivePersistent):
         # For the implementation we rely on that zfileh.dirty_writeout()
         # invokes storeblk in ascending order of blk, so that when we are here,
         # we can be sure that if previous block is also modified, then .blktab
-        # already has corresponding entry for it.
+        # already has corresponding entry for it and the entry is changed.
         if append:
             if append_newblk:
                 zblk_prev = self.blktab.get(blk-1)
                 if zblk_prev is not None  and   \
                    zblk_prev._p_changed   and   \
                    type(zblk_prev) is not ZBlk0:
-                    self._setzblk(blk-1, zblk_prev, zblk_prev.loadblkdata(), ZBlk0)
+                    # gather data prepared for previous block
+                    # NOTE: loadblkdata throws away all changes inside zblk_prev
+                    zblk_prev_data = zblk_prev.loadblkdata()
+                    # but we re-save that updated data immediately after
+                    self._setzblk(blk-1, zblk_prev, zblk_prev_data, ZBlk0)
             return ZBlk1 if (small and not filled) else ZBlk0
 
         # all other changes - use ZBlk1 if the change is small and ZBlk0 otherwise

bigfile/tests/test_filezodb.py

@@ -700,7 +700,7 @@ def test_bigfile_zblk_fmt_auto():
     root = dbopen()
     defer(lambda: dbclose(root))
 
-    # set ZBlk_fmt_write to 'h' for this test
+    # set ZBlk_fmt_write to 'auto' for this test
     fmt_write_save = file_zodb.ZBlk_fmt_write
     file_zodb.ZBlk_fmt_write = 'auto'
     def _():

bigfile/virtmem.c

@@ -389,7 +389,7 @@ int fileh_dirty_writeout(BigFileH *fileh, enum WriteoutFlags flags)
     fileh->writeout_inprogress = 1;
 
     /* pages are stored (if stored) in sorted order
-     * NOTE ZBlk format 'auto' relies on this */
+     * NOTE writeout of ZBlk format 'auto' relies on this */
     if (flags & WRITEOUT_STORE)
         list_sort(&fileh->dirty_pages, hpage_indirty_cmp_bypgoffset, NULL);