This PR makes a couple tweaks to the shelf opening code:
When opening a new shelf that does not exist, the PR adds an fsync after writing the magic. The idea being that OSes are a bit different wrt cached data when they crash: some OSes expand the file to the needed size and don't write anything, others don't expand the file. This kind of leads ti different behaviours that might be hard to investigate. Adding an fsync after writing the magic should be irrelevant runtime wise, but will ensure that the shelves at least have this baseline guarantee that the magic is there and correct. This will allow us to "repair" corrupted shelves by knowing for sure that they are at least surely a shelf and not some random data the user pointed to.
When reading the magic, the PR modifies the read so it doesn't look at a returns error, rather the number of bytes read. This feels wonky, but the ReadAt spec says that returning len(p), EOF is valid, so some OS/file implementations maybe (?), might (?), return EOF if opening an empty shelf, which would still be successful if the magic is correct. The PR ensures that such wonky cases are handled correctly.
A last change is in the openShelf, where the shelf sizes are all pre-generated first, before starting to du actual disk operations. The reason being that if the shelf size generator is invalid, there's no point to do file ops. This specifically hit on MacOS with the fsync where the test case that opened 4K shelves needed 4K fsyncs, which is 20s (5ms / fsync) :/.
Lastly, the PR adds a test for the empty-but-correct shelf case.
This PR makes a couple tweaks to the shelf opening code:
fsync
after writing the magic. The idea being that OSes are a bit different wrt cached data when they crash: some OSes expand the file to the needed size and don't write anything, others don't expand the file. This kind of leads ti different behaviours that might be hard to investigate. Adding an fsync after writing the magic should be irrelevant runtime wise, but will ensure that the shelves at least have this baseline guarantee that the magic is there and correct. This will allow us to "repair" corrupted shelves by knowing for sure that they are at least surely a shelf and not some random data the user pointed to.ReadAt
spec says that returninglen(p), EOF
is valid, so some OS/file implementations maybe (?), might (?), return EOF if opening an empty shelf, which would still be successful if the magic is correct. The PR ensures that such wonky cases are handled correctly.openShelf
, where the shelf sizes are all pre-generated first, before starting to du actual disk operations. The reason being that if the shelf size generator is invalid, there's no point to do file ops. This specifically hit on MacOS with the fsync where the test case that opened 4K shelves needed 4K fsyncs, which is 20s (5ms / fsync) :/.