diff --git a/doc/manual/rl-next/fsync-store-paths.md b/doc/manual/rl-next/fsync-store-paths.md new file mode 100644 index 000000000..0e9e7f7f2 --- /dev/null +++ b/doc/manual/rl-next/fsync-store-paths.md @@ -0,0 +1,9 @@ +--- +synopsis: Add setting `fsync-store-paths` +issues: [1218] +prs: [7126] +--- + +Nix now has a setting `fsync-store-paths` that ensures that new store paths are durably written to disk before they are registered as "valid" in Nix's database. This can prevent Nix store corruption if the system crashes or there is a power loss. This setting defaults to `false`. + +Author: [**@squalus**](https://github.com/squalus) diff --git a/src/libstore/globals.hh b/src/libstore/globals.hh index 65c7bf3bc..e5e7024cb 100644 --- a/src/libstore/globals.hh +++ b/src/libstore/globals.hh @@ -399,10 +399,18 @@ public: default is `true`. )"}; + Setting fsyncStorePaths{this, false, "fsync-store-paths", + R"( + Whether to call `fsync()` on store paths before registering them, to + flush them to disk. This improves robustness in case of system crashes, + but reduces performance. The default is `false`. + )"}; + Setting useSQLiteWAL{this, !isWSL1(), "use-sqlite-wal", "Whether SQLite should use WAL mode."}; #ifndef _WIN32 + // FIXME: remove this option, `fsync-store-paths` is faster. Setting syncBeforeRegistering{this, false, "sync-before-registering", "Whether to call `sync()` before registering a path as valid."}; #endif diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 819cee345..72927e3f0 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -1137,7 +1137,7 @@ void LocalStore::addToStore(const ValidPathInfo & info, Source & source, TeeSource wrapperSource { source, hashSink }; narRead = true; - restorePath(realPath, wrapperSource); + restorePath(realPath, wrapperSource, settings.fsyncStorePaths); auto hashResult = hashSink.finish(); @@ -1191,6 +1191,11 @@ void LocalStore::addToStore(const ValidPathInfo & info, Source & source, optimisePath(realPath, repair); // FIXME: combine with hashPath() + if (settings.fsyncStorePaths) { + recursiveSync(realPath); + syncParent(realPath); + } + registerValidPath(info); } @@ -1271,7 +1276,7 @@ StorePath LocalStore::addToStoreFromDump( delTempDir = std::make_unique(tempDir); tempPath = tempDir / "x"; - restorePath(tempPath.string(), bothSource, dumpMethod); + restorePath(tempPath.string(), bothSource, dumpMethod, settings.fsyncStorePaths); dumpBuffer.reset(); dump = {}; @@ -1318,7 +1323,7 @@ StorePath LocalStore::addToStoreFromDump( switch (fim) { case FileIngestionMethod::Flat: case FileIngestionMethod::NixArchive: - restorePath(realPath, dumpSource, (FileSerialisationMethod) fim); + restorePath(realPath, dumpSource, (FileSerialisationMethod) fim, settings.fsyncStorePaths); break; case FileIngestionMethod::Git: // doesn't correspond to serialization method, so @@ -1343,6 +1348,11 @@ StorePath LocalStore::addToStoreFromDump( optimisePath(realPath, repair); + if (settings.fsyncStorePaths) { + recursiveSync(realPath); + syncParent(realPath); + } + ValidPathInfo info { *this, name, diff --git a/src/libutil/archive.cc b/src/libutil/archive.cc index e2ebcda0c..9ed65be6a 100644 --- a/src/libutil/archive.cc +++ b/src/libutil/archive.cc @@ -294,9 +294,9 @@ void parseDump(FileSystemObjectSink & sink, Source & source) } -void restorePath(const std::filesystem::path & path, Source & source) +void restorePath(const std::filesystem::path & path, Source & source, bool startFsync) { - RestoreSink sink; + RestoreSink sink{startFsync}; sink.dstPath = path; parseDump(sink, source); } diff --git a/src/libutil/archive.hh b/src/libutil/archive.hh index 2da8f0cb1..c38fa8a46 100644 --- a/src/libutil/archive.hh +++ b/src/libutil/archive.hh @@ -75,7 +75,7 @@ void dumpString(std::string_view s, Sink & sink); void parseDump(FileSystemObjectSink & sink, Source & source); -void restorePath(const std::filesystem::path & path, Source & source); +void restorePath(const std::filesystem::path & path, Source & source, bool startFsync = false); /** * Read a NAR from 'source' and write it to 'sink'. diff --git a/src/libutil/file-content-address.cc b/src/libutil/file-content-address.cc index 86378dd67..69301d9c8 100644 --- a/src/libutil/file-content-address.cc +++ b/src/libutil/file-content-address.cc @@ -88,14 +88,15 @@ void dumpPath( void restorePath( const Path & path, Source & source, - FileSerialisationMethod method) + FileSerialisationMethod method, + bool startFsync) { switch (method) { case FileSerialisationMethod::Flat: - writeFile(path, source); + writeFile(path, source, 0666, startFsync); break; case FileSerialisationMethod::NixArchive: - restorePath(path, source); + restorePath(path, source, startFsync); break; } } diff --git a/src/libutil/file-content-address.hh b/src/libutil/file-content-address.hh index ec42d3d34..0c584ea8a 100644 --- a/src/libutil/file-content-address.hh +++ b/src/libutil/file-content-address.hh @@ -70,7 +70,8 @@ void dumpPath( void restorePath( const Path & path, Source & source, - FileSerialisationMethod method); + FileSerialisationMethod method, + bool startFsync = false); /** diff --git a/src/libutil/file-descriptor.cc b/src/libutil/file-descriptor.cc index 3bbfc50ee..4c5daf398 100644 --- a/src/libutil/file-descriptor.cc +++ b/src/libutil/file-descriptor.cc @@ -92,7 +92,7 @@ void AutoCloseFD::close() } } -void AutoCloseFD::fsync() +void AutoCloseFD::fsync() const { if (fd != INVALID_DESCRIPTOR) { int result; @@ -111,6 +111,18 @@ void AutoCloseFD::fsync() } + +void AutoCloseFD::startFsync() const +{ +#if __linux__ + if (fd != -1) { + /* Ignore failure, since fsync must be run later anyway. This is just a performance optimization. */ + ::sync_file_range(fd, 0, 0, SYNC_FILE_RANGE_WRITE); + } +#endif +} + + AutoCloseFD::operator bool() const { return fd != INVALID_DESCRIPTOR; diff --git a/src/libutil/file-descriptor.hh b/src/libutil/file-descriptor.hh index be61375f6..551e51adc 100644 --- a/src/libutil/file-descriptor.hh +++ b/src/libutil/file-descriptor.hh @@ -128,7 +128,18 @@ public: explicit operator bool() const; Descriptor release(); void close(); - void fsync(); + + /** + * Perform a blocking fsync operation. + */ + void fsync() const; + + /** + * Asynchronously flush to disk without blocking, if available on + * the platform. This is just a performance optimization, and + * fsync must be run later even if this is called. + */ + void startFsync() const; }; class Pipe diff --git a/src/libutil/file-system.cc b/src/libutil/file-system.cc index 536ae29ab..aa5f3670c 100644 --- a/src/libutil/file-system.cc +++ b/src/libutil/file-system.cc @@ -11,6 +11,7 @@ #include #include #include +#include #include #include @@ -318,6 +319,50 @@ void syncParent(const Path & path) } +void recursiveSync(const Path & path) +{ + /* If it's a file, just fsync and return. */ + auto st = lstat(path); + if (S_ISREG(st.st_mode)) { + AutoCloseFD fd = open(path.c_str(), O_RDONLY, 0); + if (!fd) + throw SysError("opening file '%1%'", path); + fd.fsync(); + return; + } + + /* Otherwise, perform a depth-first traversal of the directory and + fsync all the files. */ + std::deque dirsToEnumerate; + dirsToEnumerate.push_back(path); + std::vector dirsToFsync; + while (!dirsToEnumerate.empty()) { + auto currentDir = dirsToEnumerate.back(); + dirsToEnumerate.pop_back(); + for (auto & entry : std::filesystem::directory_iterator(currentDir)) { + auto st = entry.symlink_status(); + if (fs::is_directory(st)) { + dirsToEnumerate.emplace_back(entry.path()); + } else if (fs::is_regular_file(st)) { + AutoCloseFD fd = open(entry.path().c_str(), O_RDONLY, 0); + if (!fd) + throw SysError("opening file '%1%'", entry.path()); + fd.fsync(); + } + } + dirsToFsync.emplace_back(std::move(currentDir)); + } + + /* Fsync all the directories. */ + for (auto dir = dirsToFsync.rbegin(); dir != dirsToFsync.rend(); ++dir) { + AutoCloseFD fd = open(dir->c_str(), O_RDONLY, 0); + if (!fd) + throw SysError("opening directory '%1%'", *dir); + fd.fsync(); + } +} + + static void _deletePath(Descriptor parentfd, const fs::path & path, uint64_t & bytesFreed) { #ifndef _WIN32 diff --git a/src/libutil/file-system.hh b/src/libutil/file-system.hh index 1ae5fa136..0f406a2de 100644 --- a/src/libutil/file-system.hh +++ b/src/libutil/file-system.hh @@ -134,10 +134,15 @@ void writeFile(const Path & path, std::string_view s, mode_t mode = 0666, bool s void writeFile(const Path & path, Source & source, mode_t mode = 0666, bool sync = false); /** - * Flush a file's parent directory to disk + * Flush a path's parent directory to disk. */ void syncParent(const Path & path); +/** + * Flush a file or entire directory tree to disk. + */ +void recursiveSync(const Path & path); + /** * Delete a path; i.e., in the case of a directory, it is deleted * recursively. It's not an error if the path does not exist. The diff --git a/src/libutil/fs-sink.cc b/src/libutil/fs-sink.cc index f15324d0a..154346cee 100644 --- a/src/libutil/fs-sink.cc +++ b/src/libutil/fs-sink.cc @@ -76,6 +76,17 @@ void RestoreSink::createDirectory(const CanonPath & path) struct RestoreRegularFile : CreateRegularFileSink { AutoCloseFD fd; + bool startFsync = false; + + ~RestoreRegularFile() + { + /* Initiate an fsync operation without waiting for the + result. The real fsync should be run before registering a + store path, but this is a performance optimization to allow + the disk write to start early. */ + if (fd && startFsync) + fd.startFsync(); + } void operator () (std::string_view data) override; void isExecutable() override; @@ -95,6 +106,7 @@ void RestoreSink::createRegularFile(const CanonPath & path, std::function