flan: Overwriting an object doesn't persist the number of bytes written

[avborup]

Context

This issue applies to flan development, specifically on the jag/flan-overwrite branch.

This issue was originally described with this message from the Discord server. The issue specific to the number of bytes was discovered with this message. Additionally the original overwrite issue was shown to exist with this GitHub Gist.

Demo

Running the flan_test_overwrite test from jag/flan-overwrite twice with a fresh mkfs.flexalloced loopback device produces the following behaviour:

$ sudo FLAN_TEST_DEV=/dev/loop1 ./build/flan/flan_test_overwrite 4096
Before object write, read data (16 bytes):
After object write, read data (28 bytes): My favourite food is salad!
Closing, re-opening
Before object write, read data (28 bytes): My favourite food is salad!
After object write, read data (28 bytes): My favourite food is pizza!
Closing, re-opening
Before object write, read data (28 bytes): My favourite food is pizza!
After object write, read data (32 bytes): My favourite food is chocolate!

$ sudo FLAN_TEST_DEV=/dev/loop1 ./build/flan/flan_test_overwrite 4096
Before object write, read data (28 bytes): My favourite food is chocola
After object write, read data (28 bytes): My favourite food is salad!
Closing, re-opening
Before object write, read data (28 bytes): My favourite food is salad!
After object write, read data (28 bytes): My favourite food is pizza!
Closing, re-opening
Before object write, read data (28 bytes): My favourite food is pizza!
After object write, read data (32 bytes): My favourite food is chocolate!

On the second run, the initial print shows that the object has 28 bytes of data. However, it should have 32 as shown by the end of the first run. The actual data in the object seems to have been persisted, so it's only the number of bytes that seem to be the problem now.

[albe2669]

Hi. I've done quite a bit of testing now and think i found the issue, and a temporary fix, but not the root cause.

The issue

The last write updating to "Chocolate" is slightly longer than the previous writes which means it has to overwrite the size of the object metadata, which seems to never be done.

If one where to update the size of the object we're still faced with the problem of writing the metadata to disk again, this seems to be done in flan_md_write_root_obj (Called by flan_close->flan_md_fini). Most of this method body, in my limited testing, only runs once when the first object is added. That's because it checks for atleast one dirty object to exist, created by flan_md_mod_dirty. If i dirty the object it then fails to write with this error when trying to write the root object:

flexalloc ERR  ../src/flexalloc_mm.c(1247) fla_xne_lba_range_from_offset_nbytes()
flexalloc ERR  ../flan/flan_md.c(238) fla_object_write() src_nbytes : 136, len 4096
flexalloc ERR  ../flan/flan_md.c(383) flan_md_write()
flexalloc ERR  ../flan/flan_md.c(394) flan_md_fini()
flexalloc ERR  ../flan/flan.c(415) flan_md_fini() : Unhandled error on closing md

This seems to be caused by src_nbytes being set to 136 bytes. This number seems strange as the object itself is 40 bytes, which means it tries to write quite far after the end of the previously written root object.

A very temporary solution

Adding this to flan_object_write_ below the other if statement checking the offset and len, sets the new size and dirties the object

  if (offset + len != oinfo->size) {
    oinfo->size = offset + len;
    flan_md_mod_dirty(flanh->md, oinfo->name, oinfo, true);
  }

And this very temporary change in flan_md_write_root_obj forces the write in the "correct" position. I've not tested the further consequences of this change apart from the small test that Adrian used above, i'll test further soon.

  uint64_t src_write_nbytes = root_obj->end->write_offset_nbytes;
  // uint64_t src_write_nbytes = 0;

Combining these 2 changes seems to fix the problem, but i wouldn't call it a solution. There is still the issue of src_nbytes which is read from root_obj->end->write_offset_nbytes which is set in flan_md_forward_dirty_elements. Not sure if that's actually supposed to be 0 or if i'm misunderstanding something, but this is as far as my debugging has taken me.

[Joelgranados]

@albe2669 : thx for your comment. Could you please create a PR so we can get this in. And remember that I have updated jag/flanv2. So please create the PR against the new branch.

Thx for posting a possible fix!!!!