[PATCH] refs: sync loose refs to disk before committing them

When writing loose refs, we first create a lockfile, write the new ref into that lockfile, close it and then rename the lockfile into place such that the actual update is atomic for that single ref. While this works as intended under normal circumstences, at GitLab we infrequently encounter corrupt loose refs in repositories after a machine encountered a hard reset. The corruption is always of the same type: the ref has been committed into place, but it is completely empty.

The root cause of this is likely that we don't sync contents of the lockfile to disk before renaming it into place. As a result, it's not guaranteed that the contents are properly persisted and one may observe weird in-between states on hard resets. Quoting ext4 documentation [1]:

    Many broken applications don't use fsync() when replacing existing
    files via patterns such as fd =
    open("foo.new")/write(fd,..)/close(fd)/ rename("foo.new", "foo"), or
    worse yet, fd = open("foo", O_TRUNC)/write(fd,..)/close(fd). If
    auto_da_alloc is enabled, ext4 will detect the replace-via-rename
    and replace-via-truncate patterns and force that any delayed
    allocation blocks are allocated such that at the next journal
    commit, in the default data=ordered mode, the data blocks of the new
    file are forced to disk before the rename() operation is committed.
    This provides roughly the same level of guarantees as ext3, and
    avoids the "zero-length" problem that can happen when a system
    crashes before the delayed allocation blocks are forced to disk.

This explicitly points out that one must call fsync(3P) before doing the rename(3P) call, or otherwise data may not be correctly persisted to disk.

Fix this by always flushing refs to disk before committing them into place to avoid this class of corruption.

[1]: https://www.kernel.org/doc/Documentation/filesystems/ext4.txt

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 refs/files-backend.c | 1 +
 1 file changed, 1 insertion(+)