google / google/leveldb

Possible bug: fsync() required after calling rename()

Open
#195 8 comments 0 reactions 0 assignees View on GitHub
bug
Dominant language
C++
Stars
39.4k
Forks
8.2k
PR merge metrics
No merged PRs in 30d

Description

Original [issue 189](https://code.google.com/p/leveldb/issues/detail?id=189) created by madthanu on 2013-07-17T03:34:10.000Z:

Similar to issue 187, this bug is about what happens during a power-failure.

Also, this bug is not actually triggered atop file-systems that I usually use (ext3/ext4), so I haven't tried reproducing it on a real system.

The bug: When LevelDB opens a database, it compacts the current log files to a ".sst" file, creates a new MANIFEST, and updates the CURRENT file. Updating the CURRENT file is done with the usual atomic technique using rename(). It then deletes the old log files and old MANIFEST file.

In this entire sequence of operations, LevelDB does not explicitly ensure that the rename() (corresponding to the CURRENT file) actually happens before the unlink()s (of the old MANIFEST and log files). If the unlink()s happen before the rename, the database can be left corrupted (due to a missing MANIFEST) or wrong (due to a deleted, non-compacted log). I verified the corruption and wrong-behavior by simulating the unlink()s happening before the rename().

Saying that, considering Linux, neither ext3 nor ext4 never "re-order" calls like unlink() or rename(). Btrfs comes close to affecting LevelDB by doing re-ordering, but it seems to only re-order such that rename()s get pushed before unlink()s, and so seems to do no real harm.

Nevertheless, LevelDB should not depend on the unlink()s happening after the rename(). There are lots of operating systems and file-systems out there.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.