Cleanup file utils

Reviewers: teon.banek, msantl

Reviewed By: teon.banek

Subscribers: pullbot

Differential Revision: https://phabricator.memgraph.io/D1861

src/database/distributed/distributed_graph_db.cpp

@@ -712,7 +712,7 @@ bool Master::MakeSnapshot(GraphDbAccessor &accessor) {
   // some tx_id visibility also exists on workers
   const bool status =
       durability::MakeSnapshot(*this, accessor, impl_->config_.worker_id,
-                               fs::path(impl_->config_.durability_directory),
+                               impl_->config_.durability_directory,
                                impl_->config_.snapshot_max_retained);
   if (status) {
     LOG(INFO) << "Snapshot created successfully.";
@@ -749,7 +749,7 @@ void Master::Start() {
   impl_->tx_engine_.StartTransactionalCacheCleanup();
 
   if (impl_->config_.durability_enabled)
-    utils::CheckDir(impl_->config_.durability_directory);
+    utils::EnsureDirOrDie(impl_->config_.durability_directory);
 
   // Durability recovery.
   {
@@ -1082,7 +1082,7 @@ bool Worker::MakeSnapshot(GraphDbAccessor &accessor) {
   // Makes a local snapshot from the visibility of accessor
   const bool status =
       durability::MakeSnapshot(*this, accessor, impl_->config_.worker_id,
-                               fs::path(impl_->config_.durability_directory),
+                               impl_->config_.durability_directory,
                                impl_->config_.snapshot_max_retained);
   if (status) {
     LOG(INFO) << "Snapshot created successfully.";
@@ -1130,7 +1130,7 @@ void Worker::Start() {
   impl_->tx_engine_.StartTransactionalCacheCleanup();
 
   if (impl_->config_.durability_enabled)
-    utils::CheckDir(impl_->config_.durability_directory);
+    utils::EnsureDirOrDie(impl_->config_.durability_directory);
 
   // Durability recovery. We need to check this flag for workers that are added
   // after the "main" cluster recovery.

src/database/single_node/graph_db.cpp

@@ -17,7 +17,8 @@
 namespace database {
 
 GraphDb::GraphDb(Config config) : config_(config) {
-  if (config_.durability_enabled) utils::CheckDir(config_.durability_directory);
+  if (config_.durability_enabled)
+    utils::EnsureDirOrDie(config_.durability_directory);
 
   // Durability recovery.
   if (config_.db_recover_on_startup) {
@@ -135,8 +136,8 @@ database::Counters &GraphDb::counters() { return counters_; }
 void GraphDb::CollectGarbage() { storage_gc_->CollectGarbage(); }
 
 bool GraphDb::MakeSnapshot(GraphDbAccessor &accessor) {
-  const bool status = durability::MakeSnapshot(
+  const bool status =
-      *this, accessor, fs::path(config_.durability_directory),
+      durability::MakeSnapshot(*this, accessor, config_.durability_directory,
                                config_.snapshot_max_retained);
   if (status) {
     LOG(INFO) << "Snapshot created successfully.";

src/database/single_node_ha/graph_db.cpp

@@ -15,7 +15,7 @@ namespace database {
 GraphDb::GraphDb(Config config) : config_(config) {}
 
 void GraphDb::Start() {
-  utils::CheckDir(config_.durability_directory);
+  utils::EnsureDirOrDie(config_.durability_directory);
   raft_server_.Start();
   CHECK(coordination_.Start()) << "Couldn't start coordination!";
 

src/distributed/cluster_discovery_master.cpp

@@ -26,9 +26,7 @@ ClusterDiscoveryMaster::ClusterDiscoveryMaster(
     io::network::Endpoint worker_endpoint(endpoint.address(), req.port);
 
     // Create and find out what is our durability directory.
-    CHECK(utils::EnsureDir(durability_directory_))
+    utils::EnsureDirOrDie(durability_directory_);
-        << "Couldn't create durability directory '" << durability_directory_
-        << "'!";
     auto full_durability_directory =
         std::experimental::filesystem::canonical(durability_directory_);
 

src/distributed/cluster_discovery_worker.cpp

@@ -22,9 +22,7 @@ ClusterDiscoveryWorker::ClusterDiscoveryWorker(WorkerCoordination *coordination)
 void ClusterDiscoveryWorker::RegisterWorker(
     int worker_id, const std::string &durability_directory) {
   // Create and find out what is our durability directory.
-  CHECK(utils::EnsureDir(durability_directory))
+  utils::EnsureDirOrDie(durability_directory);
-      << "Couldn't create durability directory '" << durability_directory
-      << "'!";
   auto full_durability_directory =
       std::experimental::filesystem::canonical(durability_directory);
 

src/durability/distributed/recovery.cpp

@@ -83,9 +83,9 @@ bool ContainsDurabilityFiles(const fs::path &durability_dir) {
 
 void MoveToBackup(const fs::path &durability_dir) {
   auto backup_dir = durability_dir / kBackupDir;
-  utils::CheckDir(backup_dir);
+  utils::EnsureDirOrDie(backup_dir);
-  utils::CheckDir(backup_dir / kSnapshotDir);
+  utils::EnsureDirOrDie(backup_dir / kSnapshotDir);
-  utils::CheckDir(backup_dir / kWalDir);
+  utils::EnsureDirOrDie(backup_dir / kWalDir);
   for (const auto &durability_type : {kSnapshotDir, kWalDir}) {
     auto recovery_dir = durability_dir / durability_type;
     if (!fs::exists(recovery_dir) || !fs::is_directory(recovery_dir)) continue;

src/durability/distributed/wal.cpp

@@ -27,7 +27,7 @@ WriteAheadLog::WriteAheadLog(
       durability_enabled_(durability_enabled),
       synchronous_commit_(synchronous_commit) {
   if (durability_enabled_) {
-    utils::CheckDir(durability_dir);
+    utils::EnsureDirOrDie(durability_dir);
   }
 }
 

src/durability/single_node/recovery.cpp

@@ -82,9 +82,9 @@ bool ContainsDurabilityFiles(const fs::path &durability_dir) {
 
 void MoveToBackup(const fs::path &durability_dir) {
   auto backup_dir = durability_dir / kBackupDir;
-  utils::CheckDir(backup_dir);
+  utils::EnsureDirOrDie(backup_dir);
-  utils::CheckDir(backup_dir / kSnapshotDir);
+  utils::EnsureDirOrDie(backup_dir / kSnapshotDir);
-  utils::CheckDir(backup_dir / kWalDir);
+  utils::EnsureDirOrDie(backup_dir / kWalDir);
   for (const auto &durability_type : {kSnapshotDir, kWalDir}) {
     auto recovery_dir = durability_dir / durability_type;
     if (!fs::exists(recovery_dir) || !fs::is_directory(recovery_dir)) continue;

src/durability/single_node/wal.cpp

@@ -28,7 +28,7 @@ WriteAheadLog::WriteAheadLog(
       durability_enabled_(durability_enabled),
       synchronous_commit_(synchronous_commit) {
   if (durability_enabled_) {
-    utils::CheckDir(durability_dir);
+    utils::EnsureDirOrDie(durability_dir);
   }
 }
 

src/storage/common/kvstore/kvstore.cpp

@@ -12,7 +12,8 @@ struct KVStore::impl {
   rocksdb::Options options;
 };
 
-KVStore::KVStore(fs::path storage) : pimpl_(std::make_unique<impl>()) {
+KVStore::KVStore(std::experimental::filesystem::path storage)
+    : pimpl_(std::make_unique<impl>()) {
   pimpl_->storage = storage;
   if (!utils::EnsureDir(pimpl_->storage))
     throw KVStoreError("Folder for the key-value store " +

src/storage/common/kvstore/kvstore_dummy.cpp

@@ -8,7 +8,7 @@ namespace storage {
 
 struct KVStore::impl {};
 
-KVStore::KVStore(fs::path storage) {}
+KVStore::KVStore(std::experimental::filesystem::path storage) {}
 
 KVStore::~KVStore() {}
 

src/utils/file.cpp

@@ -5,51 +5,19 @@
 #include <sys/types.h>
 #include <unistd.h>
 
-#include "fmt/format.h"
+#include <fstream>
-#include "fmt/ostream.h"
+
-#include "glog/logging.h"
+#include <glog/logging.h>
 
 namespace utils {
 
-std::vector<fs::path> LoadFilePaths(const fs::path &directory,
+std::vector<std::string> ReadLines(
-                                    const std::string &extension) {
+    const std::experimental::filesystem::path &path) noexcept {
-  // result container
-  std::vector<fs::path> file_paths;
-
-  for (auto &directory_entry : fs::recursive_directory_iterator(directory)) {
-    auto path = directory_entry.path().string();
-
-    // skip directories
-    if (!fs::is_regular_file(directory_entry)) continue;
-
-    // if extension isn't defined then put all file paths from the directory
-    // to the result set
-    if (!extension.empty()) {
-      // skip paths that don't have appropriate extension
-      auto file_extension = path.substr(path.find_last_of(".") + 1);
-      if (file_extension != extension) continue;
-    }
-
-    file_paths.emplace_back(path);
-
-    // skip paths that don't have appropriate extension
-    auto file_extension = path.substr(path.find_last_of(".") + 1);
-    if (file_extension != extension) continue;
-
-    // path has the right extension and can be placed in the result
-    // container
-    file_paths.emplace_back(path);
-  }
-
-  return file_paths;
-}
-
-std::vector<std::string> ReadLines(const fs::path &path) {
   std::vector<std::string> lines;
 
-  if (!fs::exists(path)) return lines;
-
   std::ifstream stream(path.c_str());
+  // We don't have to check the failed bit of the stream because `getline` won't
+  // read anything in that case and that is exactly the behavior that we want.
   std::string line;
   while (std::getline(stream, line)) {
     lines.emplace_back(line);
@@ -58,153 +26,187 @@ std::vector<std::string> ReadLines(const fs::path &path) {
   return lines;
 }
 
-void Write(const std::string &text, const fs::path &path) {
+bool EnsureDir(const std::experimental::filesystem::path &dir) noexcept {
-  std::ofstream stream;
+  std::error_code error_code;  // For exception suppression.
-  stream.open(path.c_str());
+  if (std::experimental::filesystem::exists(dir, error_code))
-  stream << text;
+    return std::experimental::filesystem::is_directory(dir, error_code);
-  stream.close();
+  return std::experimental::filesystem::create_directories(dir, error_code);
 }
 
-bool EnsureDir(const fs::path &dir) {
+void EnsureDirOrDie(const std::experimental::filesystem::path &dir) {
-  if (fs::exists(dir)) return true;
+  CHECK(EnsureDir(dir)) << "Couldn't create directory '" << dir
-  std::error_code error_code;  // Just for exception suppression.
+                        << "' due to a permission issue or the path exists and "
-  return fs::create_directories(dir, error_code);
+                           "isn't a directory!";
 }
 
-void CheckDir(const std::string &dir) {
+bool DeleteDir(const std::experimental::filesystem::path &dir) noexcept {
-  if (fs::exists(dir)) {
+  std::error_code error_code;  // For exception suppression.
-    CHECK(fs::is_directory(dir)) << "The directory path '" << dir
+  if (!std::experimental::filesystem::is_directory(dir, error_code))
-                                 << "' is not a directory!";
+    return false;
-  } else {
+  return std::experimental::filesystem::remove_all(dir, error_code) > 0;
-    bool success = EnsureDir(dir);
-    CHECK(success) << "Failed to create directory '" << dir << "'.";
-  }
 }
 
-bool DeleteDir(const fs::path &dir) {
+bool CopyFile(const std::experimental::filesystem::path &src,
-  if (!fs::exists(dir)) return true;
+              const std::experimental::filesystem::path &dst) noexcept {
-  std::error_code error_code;  // Just for exception suppression.
+  std::error_code error_code;  // For exception suppression.
-  return fs::remove_all(dir, error_code) > 0;
+  return std::experimental::filesystem::copy_file(src, dst, error_code);
 }
 
-bool CopyFile(const fs::path &src, const fs::path &dst) {
+LogFile::~LogFile() {
-  std::error_code error_code;  // Just for exception suppression.
+  if (IsOpen()) Close();
-  return fs::copy_file(src, dst);
 }
 
-File::File() : fd_(-1), path_() {}
+LogFile::LogFile(LogFile &&other)
-
+    : fd_(other.fd_),
-File::File(int fd, fs::path path) : fd_(fd), path_(std::move(path)) {}
+      written_since_last_sync_(other.written_since_last_sync_),
-
+      path_(other.path_) {
-File::~File() { Close(); }
+  other.fd_ = -1;
-
+  other.written_since_last_sync_ = 0;
-File::File(File &&rhs) : fd_(rhs.fd_), path_(rhs.path_) { rhs.Release(); }
+  other.path_ = "";
-
-File &File::operator=(File &&rhs) {
-  if (this != &rhs) {
-    fd_ = rhs.fd_;
-    path_ = rhs.path_;
-    rhs.Release();
 }
+
+LogFile &LogFile::operator=(LogFile &&other) {
+  if (IsOpen()) Close();
+
+  fd_ = other.fd_;
+  written_since_last_sync_ = other.written_since_last_sync_;
+  path_ = other.path_;
+
+  other.fd_ = -1;
+  other.written_since_last_sync_ = 0;
+  other.path_ = "";
+
   return *this;
 }
 
-fs::path File::Path() const { return path_; }
+void LogFile::Open(const std::experimental::filesystem::path &path) {
-int File::Handle() const { return fd_; }
+  CHECK(!IsOpen())
-bool File::Empty() const { return fd_ == -1; }
+      << "While trying to open " << path
+      << " for writing the database used a handle that already has " << path_
+      << " opened in it!";
+
+  path_ = path;
+  written_since_last_sync_ = 0;
+
+  while (true) {
+    // The permissions are set to ((rw-r-----) & ~umask)
+    fd_ = open(path_.c_str(), O_WRONLY | O_CLOEXEC | O_CREAT | O_APPEND, 0640);
+    if (fd_ == -1 && errno == EINTR) {
+      // The call was interrupted, try again...
+      continue;
+    } else {
+      // All other possible errors are fatal errors and are handled in the CHECK
+      // below.
+      break;
+    }
+  }
+
+  CHECK(fd_ != -1) << "While trying to open " << path_
+                   << " for writing an error occurred: " << strerror(errno)
+                   << " (" << errno << ").";
+}
+
+bool LogFile::IsOpen() const { return fd_ != -1; }
+
+const std::experimental::filesystem::path &LogFile::path() const {
+  return path_;
+}
+
+void LogFile::Write(const char *data, size_t size) {
+  while (size > 0) {
+    auto written = write(fd_, data, size);
+    if (written == -1 && errno == EINTR) {
+      continue;
+    }
+
+    CHECK(written > 0)
+        << "While trying to write to " << path_
+        << " an error occurred: " << strerror(errno) << " (" << errno
+        << "). Possibly " << size
+        << " bytes of data were lost from this call and possibly "
+        << written_since_last_sync_ << " bytes were lost from previous calls.";
+
+    size -= written;
+    data += written;
+    written_since_last_sync_ += written;
+  }
+}
+
+void LogFile::Write(const uint8_t *data, size_t size) {
+  Write(reinterpret_cast<const char *>(data), size);
+}
+void LogFile::Write(const std::string &data) {
+  Write(data.data(), data.size());
+}
+
+void LogFile::Sync() {
+  int ret = 0;
+  while (true) {
+    ret = fsync(fd_);
+    if (ret == -1 && errno == EINTR) {
+      // The call was interrupted, try again...
+      continue;
+    } else {
+      // All other possible errors are fatal errors and are handled in the CHECK
+      // below.
+      break;
+    }
+  }
+
+  // In this check we are extremely rigorous because any error except EINTR is
+  // treated as a fatal error that will crash the database. The errors that will
+  // mainly occur are EIO which indicates an I/O error on the physical device
+  // and ENOSPC (documented only in new kernels) which indicates that the
+  // physical device doesn't have any space left. If we don't succeed in
+  // syncing pending data to the physical device there is no mechanism to
+  // determine which parts of the `write` calls weren't synced. That is why
+  // we call this a fatal error and we don't continue further.
+  //
+  // A good description of issues with `fsync` can be seen here:
+  // https://stackoverflow.com/questions/42434872/writing-programs-to-cope-with-i-o-errors-causing-lost-writes-on-linux
+  //
+  // A discussion between PostgreSQL developers of what to do when `fsync`
+  // fails can be seen here:
+  // https://www.postgresql.org/message-id/flat/CAMsr%2BYE5Gs9iPqw2mQ6OHt1aC5Qk5EuBFCyG%2BvzHun1EqMxyQg%40mail.gmail.com#CAMsr+YE5Gs9iPqw2mQ6OHt1aC5Qk5EuBFCyG+vzHun1EqMxyQg@mail.gmail.com
+  //
+  // A brief of the `fsync` semantics can be seen here (part of the mailing list
+  // discussion linked above):
+  // https://www.postgresql.org/message-id/20180402185320.GM11627%40technoir
+  //
+  // The PostgreSQL developers decided to do the same thing (die) when such an
+  // error occurs:
+  // https://www.postgresql.org/message-id/20180427222842.in2e4mibx45zdth5@alap3.anarazel.de
+  CHECK(ret == 0) << "While trying to sync " << path_
+                  << " an error occurred: " << strerror(errno) << " (" << errno
+                  << "). Possibly " << written_since_last_sync_
+                  << " bytes from previous write calls were lost.";
+
+  // Reset the counter.
+  written_since_last_sync_ = 0;
+}
+
+void LogFile::Close() {
+  int ret = 0;
+  while (true) {
+    ret = close(fd_);
+    if (ret == -1 && errno == EINTR) {
+      // The call was interrupted, try again...
+      continue;
+    } else {
+      // All other possible errors are fatal errors and are handled in the CHECK
+      // below.
+      break;
+    }
+  }
+
+  CHECK(ret == 0) << "While trying to close " << path_
+                  << " an error occurred: " << strerror(errno) << " (" << errno
+                  << "). Possibly " << written_since_last_sync_
+                  << " bytes from previous write calls were lost.";
 
-void File::Release() {
   fd_ = -1;
-  path_ = fs::path();
+  written_since_last_sync_ = 0;
-}
+  path_ = "";
-
-void File::Close() {
-  if (fd_ == -1) {
-    return;
-  }
-  if (::close(fd_) == -1) {
-    throw std::system_error(errno, std::generic_category(),
-                            fmt::format("cannot close {}", path_));
-  }
-  Release();
-}
-
-File OpenFile(const fs::path &path, int flags, ::mode_t mode) {
-  int fd = ::open(path.c_str(), flags, mode);
-  if (fd == -1) {
-    throw std::system_error(errno, std::generic_category(),
-                            fmt::format("cannot open {}", path));
-  }
-
-  return File(fd, path);
-}
-
-std::experimental::optional<File> TryOpenFile(const fs::path &path, int flags,
-                                              ::mode_t mode) {
-  int fd = ::open(path.c_str(), flags, mode);
-  return fd == -1 ? std::experimental::nullopt
-                  : std::experimental::make_optional(File(fd, path));
-}
-
-File OpenFile(const File &dir, const fs::path &path, int flags, ::mode_t mode) {
-  int fd = ::openat(dir.Handle(), path.c_str(), flags, mode);
-  if (fd == -1) {
-    throw std::system_error(errno, std::generic_category(),
-                            fmt::format("cannot open {}", dir.Path() / path));
-  }
-  return File(fd, dir.Path() / path);
-}
-
-std::experimental::optional<File> TryOpenFile(const File &dir,
-                                              const fs::path &path, int flags,
-                                              ::mode_t mode) {
-  int fd = ::openat(dir.Handle(), path.c_str(), flags, mode);
-  return fd == -1
-             ? std::experimental::nullopt
-             : std::experimental::make_optional(File(fd, dir.Path() / path));
-}
-
-void Fsync(const File &file) {
-  if (::fsync(file.Handle()) == -1) {
-    throw std::system_error(errno, std::generic_category(),
-                            fmt::format("cannot fsync {}", file.Path()));
-  }
-}
-
-void Rename(const File &dir1, const fs::path &path1, const File &dir2,
-            const fs::path &path2) {
-  if (::renameat(dir1.Handle(), path1.c_str(), dir2.Handle(), path2.c_str()) !=
-      0) {
-    throw std::system_error(
-        errno, std::generic_category(),
-        fmt::format("cannot move {} to {}", dir1.Path() / path1,
-                    dir2.Path() / path2));
-  }
-}
-
-void SyncDir(const fs::path &path) {
-  File dir = OpenFile(path, O_DIRECTORY | O_RDONLY);
-  Fsync(dir);
-}
-
-File OpenDir(const fs::path &path) {
-  int res = ::mkdir(path.c_str(), 0777);
-  if (res == 0) {
-    if (path.has_parent_path()) {
-      SyncDir(path.parent_path());
-    }
-  } else if (errno != EEXIST) {
-    throw std::system_error(errno, std::generic_category(),
-                            fmt::format("cannot create directory {}", path));
-  }
-
-  int fd = ::open(path.c_str(), O_RDONLY | O_DIRECTORY);
-  if (fd == -1) {
-    throw std::system_error(errno, std::generic_category(),
-                            fmt::format("cannot open directory {}", path));
-  }
-
-  return File(fd, path);
 }
 
 }  // namespace utils

src/utils/file.hpp

@@ -2,222 +2,99 @@
  * @file
  *
  * This file contains utilities for operations with files. Other than utility
- * funtions, a `File` class is provided which wraps a system file handle.
+ * functions, a `File` class is provided which emulates a `fstream`.
  */
 #pragma once
 
 #include <experimental/filesystem>
-#include <experimental/optional>
-#include <fstream>
-
-namespace fs = std::experimental::filesystem;
 
 namespace utils {
 
-// Higher level utility operations follow.
+/// Reads all lines from the file specified by path. If the file doesn't exist
+/// or there is an access error the function returns an empty list.
+std::vector<std::string> ReadLines(
+    const std::experimental::filesystem::path &path) noexcept;
 
-/**
+/// Ensures that the given directory either exists after this call. If the
- * Loads all file paths in the specified directory. Optionally
+/// directory didn't exist prior to the call it is created, if it existed prior
- * the paths are filtered by extension.
+/// to the call it is left as is.
- *
+bool EnsureDir(const std::experimental::filesystem::path &dir) noexcept;
- * NOTE: the call isn't recursive
- *
- * @param directory a path to directory that will be scanned in order to find
- *                  all paths
- * @param extension paths will be filtered by this extension
- *
- * @return std::vector of paths founded in the directory
- */
-std::vector<fs::path> LoadFilePaths(const fs::path &directory,
-                                    const std::string &extension = "");
 
-// TODO: add error checking
+/// Calls `EnsureDir` and terminates the program if the call failed. It prints
-/**
+/// an error message for which directory the ensuring failed.
- * Reads all lines from the file specified by path.
+void EnsureDirOrDie(const std::experimental::filesystem::path &dir);
- *
- * @param path file path.
- * @return vector of all lines from the file.
- */
-std::vector<std::string> ReadLines(const fs::path &path);
 
-/**
+/// Deletes everything from the given directory including the directory.
- * Writes text into the file specified by path.
+bool DeleteDir(const std::experimental::filesystem::path &dir) noexcept;
- *
- * @param text content which will be written in the file.
- * @param path a path to the file.
- */
-void Write(const std::string &text, const fs::path &path);
 
-/**
+/// Copies the file from `src` to `dst`.
- * Esures that the given dir either exists or is succsefully created.
- */
-bool EnsureDir(const std::experimental::filesystem::path &dir);
-
-/**
- * Ensures the given directory exists and is ready for use. Creates
- * the directory if it doesn't exist.
- */
-void CheckDir(const std::string &dir);
-
-/**
- * Deletes everything from the given dir including the dir.
- */
-bool DeleteDir(const std::experimental::filesystem::path &dir);
-
-/**
- * Copies the file from src to dst.
- */
 bool CopyFile(const std::experimental::filesystem::path &src,
-              const std::experimental::filesystem::path &dst);
+              const std::experimental::filesystem::path &dst) noexcept;
 
-// End higher level operations.
+/// This class implements a file handler that is used for mission critical files
-
+/// that need to be written and synced to permanent storage. Typical usage for
-// Lower level wrappers around C system calls follow.
+/// this class is in implementation of write-ahead logging or anything similar
-
+/// that requires that data that is written *must* be stored in permanent
-/**
+/// storage.
- * Thin wrapper around system file handle.
+///
- */
+/// If any of the methods fails with a critical error *they will crash* the
-class File {
+/// whole program. The reasoning is that if you have some data that is mission
+/// critical to be written to permanent storage and you fail in doing so you
+/// aren't safe to continue your operation. The errors that can occur are mainly
+/// EIO (unrecoverable underlying storage error) or ENOSPC (the underlying
+/// storage has no more space).
+///
+/// The typical usage for this class when writing data to the file is that you
+/// call `Write` as many times as necessary to write one logical part of your
+/// data and only then you call `Sync`. For the write-ahead log example that
+/// would mean that you call `Write` until you write a whole single state delta
+/// and only after that you call `Sync` to ensure that the whole delta was
+/// written to permanent storage.
+///
+/// This class *isn't* thread safe. It is implemented as a wrapper around low
+/// level system calls used for file manipulation.
+class LogFile {
  public:
-  /** Constructs an empty file handle. */
+  LogFile() = default;
-  File();
+  ~LogFile();
 
-  /**
+  LogFile(const LogFile &) = delete;
-   * Take ownership of the given system file handle.
+  LogFile &operator=(const LogFile &) = delete;
-   *
-   * @param fd   System file handle.
-   * @param path Pathname naming the file, used only for error handling.
-   */
-  File(int fd, fs::path path);
 
-  File(File &&);
+  LogFile(LogFile &&other);
-  File &operator=(File &&);
+  LogFile &operator=(LogFile &&other);
 
-  File(const File &) = delete;
+  /// This method opens a new file used for writing. If the file doesn't exist
-  File &operator=(const File &) = delete;
+  /// it is created and if the file exists data is appended to the file to
+  /// ensure that no data is ever lost. Files are created with a restrictive
+  /// permission mask (0640). On failure and misuse it crashes the program.
+  void Open(const std::experimental::filesystem::path &path);
 
-  /**
+  /// Returns a boolean indicating whether a file is opened.
-   * Closes the underlying file handle.
+  bool IsOpen() const;
-   *
-   * @throws std::system_error
-   */
-  ~File();
 
-  /** Gets the path to the underlying file. */
+  /// Returns the path to the currently opened file. If a file isn't opened the
-  fs::path Path() const;
+  /// path is empty.
+  const std::experimental::filesystem::path &path() const;
 
-  /** Gets the underlying file handle. */
+  /// Writes data to the currently opened file. On failure and misuse it crashes
-  int Handle() const;
+  /// the program.
+  void Write(const char *data, size_t size);
+  void Write(const uint8_t *data, size_t size);
+  void Write(const std::string &data);
 
-  /** Checks if there's an underlying file handle. */
+  /// Syncs currently pending data to the currently opened file. On failure
-  bool Empty() const;
+  /// and misuse it crashes the program.
+  void Sync();
 
-  /**
+  /// Closes the currently opened file. It doesn't perform a `Sync` on the
-   * Closes the underlying file handle.
+  /// file. On failure and misuse it crashes the program.
-   *
-   * Wrapper around `close` system call (see `man 2 close`). File will
-   * be empty after the call returns. You may call Close multiple times, all
-   * calls after the first one are no-op.
-   *
-   * @throws std::system_error
-   */
   void Close();
 
  private:
-  int fd_;
+  int fd_{-1};
-  fs::path path_;
+  size_t written_since_last_sync_{0};
-
+  std::experimental::filesystem::path path_;
-  void Release();
 };
 
-/**
- * Opens a file descriptor.
- * Wrapper around `open` system call (see `man 2 open`).
- *
- * @param  path  Pathname naming the file.
- * @param  flags Opening flags.
- * @param  mode  File mode bits to apply for creation of new file.
- * @return File descriptor referring to the opened file.
- *
- * @throws std::system_error
- */
-File OpenFile(const fs::path &path, int flags, ::mode_t mode = 0666);
-
-/**
- * Same as OpenFile, but returns `nullopt` instead of throwing an exception.
- */
-std::experimental::optional<File> TryOpenFile(const fs::path &path, int flags,
-                                              ::mode_t mode = 0666);
-
-/** Calls File::Close */
-inline void Close(File &file) { file.Close(); }
-
-/**
- * Opens a file descriptor for a file inside a directory.
- * Wrapper around `openat` system call (see `man 2 openat`).
- *
- * @param  dir   Directory file descriptor.
- * @param  path  Pathname naming the file.
- * @param  flags Opening flags.
- * @param  mode  File mode bits to apply for creation of new file.
- * @return File descriptor referring to the opened file.
- *
- * @throws std::system_error
- */
-File OpenFile(const File &dir, const fs::path &path, int flags,
-              ::mode_t mode = 0666);
-
-/**
- * Same as OpenFile, but returns `nullopt` instead of throwing an exception.
- */
-std::experimental::optional<File> TryOpenFile(const File &dir,
-                                              const fs::path &path, int flags,
-                                              ::mode_t mode = 0666);
-
-/**
- * Synchronizes file with the underlying storage device.
- * Wrapper around `fsync` system call (see `man 2 fsync`).
- *
- * @param file File descriptor referring to the file to be synchronized.
- *
- * @throws std::system_error
- */
-void Fsync(const File &file);
-
-/**
- * Moves a file from one directory to another.
- *
- * Wrapper around `renameat` system call (see `man 2 renameat`).
- *
- * @param dir1  Source directory.
- * @param path1 Pathname naming the source file.
- * @param dir2  Destination directory.
- * @param path2 Pathname naming the destination file.
- *
- * @throws std::system_error
- */
-void Rename(const File &dir1, const fs::path &path1, const File &dir2,
-            const fs::path &path2);
-
-/**
- * Synchronizes directory with the underlying storage device.
- *
- * @param path Pathname naming the directory.
- *
- * @throws std::system_error
- */
-void SyncDir(const fs::path &path);
-
-/**
- * Opens a directory, creating it if it doesn't exist.
- *
- * @param  path Pathname naming the directory.
- * @return File descriptor referring to the opened directory.
- *
- * @throws std::system_error
- */
-File OpenDir(const fs::path &path);
-
-// End lower level wrappers.
-
 }  // namespace utils

tests/unit/CMakeLists.txt

@@ -332,6 +332,9 @@ target_link_libraries(${test_prefix}utils_exceptions mg-utils)
 add_unit_test(utils_executor.cpp)
 target_link_libraries(${test_prefix}utils_executor mg-utils)
 
+add_unit_test(utils_file.cpp)
+target_link_libraries(${test_prefix}utils_file mg-utils)
+
 add_unit_test(utils_math.cpp)
 target_link_libraries(${test_prefix}utils_math mg-utils)
 

tests/unit/distributed_coordination.cpp

@@ -18,6 +18,8 @@
 using namespace distributed;
 using namespace std::literals::chrono_literals;
 
+namespace fs = std::experimental::filesystem;
+
 const int kWorkerCount = 5;
 const std::string kLocal = "127.0.0.1";
 

tests/unit/utils_file.cpp

@@ -0,0 +1,277 @@
+#include <fstream>
+#include <map>
+#include <string>
+#include <vector>
+
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+#include "utils/file.hpp"
+#include "utils/string.hpp"
+
+namespace fs = std::experimental::filesystem;
+
+const std::vector<std::string> kDirsAll = {
+    "existing_dir_777", "existing_dir_770", "existing_dir_700",
+    "existing_dir_000", "symlink_dir_777",  "symlink_dir_770",
+    "symlink_dir_700",  "symlink_dir_000"};
+
+const std::vector<std::string> kFilesAll = {
+    "existing_file_666", "existing_file_660", "existing_file_600",
+    "existing_file_000", "symlink_file_666",  "symlink_file_660",
+    "symlink_file_600",  "symlink_file_000"};
+
+const std::map<std::string, fs::perms> kPermsAll = {
+    {"777",
+     fs::perms::owner_all | fs::perms::group_all | fs::perms::others_all},
+    {"770", fs::perms::owner_all | fs::perms::group_all},
+    {"700", fs::perms::owner_all},
+    {"666", fs::perms::owner_read | fs::perms::owner_write |
+                fs::perms::group_read | fs::perms::group_write |
+                fs::perms::others_read | fs::perms::others_write},
+    {"660", fs::perms::owner_read | fs::perms::owner_write |
+                fs::perms::group_read | fs::perms::group_write},
+    {"600", fs::perms::owner_read | fs::perms::owner_write},
+    {"000", fs::perms::none},
+};
+
+fs::perms GetPermsFromFilename(const std::string &name) {
+  auto split = utils::Split(name, "_");
+  return kPermsAll.at(split[split.size() - 1]);
+}
+
+void CreateFile(const fs::path &path) {
+  std::ofstream stream(path);
+  stream << "hai hai hai hai!" << std::endl << "nandare!!!" << std::endl;
+}
+
+void CreateFiles(const fs::path &path) {
+  CreateFile(path / "existing_file_666");
+  fs::create_symlink(path / "existing_file_666", path / "symlink_file_666");
+  fs::permissions(path / "existing_file_666",
+                  fs::perms::owner_read | fs::perms::owner_write |
+                      fs::perms::group_read | fs::perms::group_write |
+                      fs::perms::others_read | fs::perms::others_write);
+
+  CreateFile(path / "existing_file_660");
+  fs::create_symlink(path / "existing_file_660", path / "symlink_file_660");
+  fs::permissions(path / "existing_file_660",
+                  fs::perms::owner_read | fs::perms::owner_write |
+                      fs::perms::group_read | fs::perms::group_write);
+
+  CreateFile(path / "existing_file_600");
+  fs::create_symlink(path / "existing_file_600", path / "symlink_file_600");
+  fs::permissions(path / "existing_file_600",
+                  fs::perms::owner_read | fs::perms::owner_write);
+
+  CreateFile(path / "existing_file_000");
+  fs::create_symlink(path / "existing_file_000", path / "symlink_file_000");
+  fs::permissions(path / "existing_file_000", fs::perms::none);
+}
+
+class UtilsFileTest : public ::testing::Test {
+ public:
+  void SetUp() override {
+    Clear();
+    fs::create_directory(storage);
+
+    fs::create_directory(storage / "existing_dir_777");
+    fs::create_directory_symlink(storage / "existing_dir_777",
+                                 storage / "symlink_dir_777");
+    CreateFiles(storage / "existing_dir_777");
+
+    fs::create_directory(storage / "existing_dir_770");
+    fs::create_directory_symlink(storage / "existing_dir_770",
+                                 storage / "symlink_dir_770");
+    CreateFiles(storage / "existing_dir_770");
+
+    fs::create_directory(storage / "existing_dir_700");
+    fs::create_directory_symlink(storage / "existing_dir_700",
+                                 storage / "symlink_dir_700");
+    CreateFiles(storage / "existing_dir_700");
+
+    fs::create_directory(storage / "existing_dir_000");
+    fs::create_directory_symlink(storage / "existing_dir_000",
+                                 storage / "symlink_dir_000");
+    CreateFiles(storage / "existing_dir_000");
+
+    fs::permissions(storage / "existing_dir_777", fs::perms::owner_all |
+                                                      fs::perms::group_all |
+                                                      fs::perms::others_all);
+    fs::permissions(storage / "existing_dir_770",
+                    fs::perms::owner_all | fs::perms::group_all);
+    fs::permissions(storage / "existing_dir_700", fs::perms::owner_all);
+    fs::permissions(storage / "existing_dir_000", fs::perms::none);
+  }
+
+  void TearDown() override {
+    // Validate that the test structure was left intact.
+    for (const auto &dir : kDirsAll) {
+      {
+        ASSERT_TRUE(fs::exists(storage / dir));
+        auto dir_status = fs::symlink_status(storage / dir);
+        if (!utils::StartsWith(dir, "symlink")) {
+          ASSERT_EQ(dir_status.permissions() & fs::perms::all,
+                    GetPermsFromFilename(dir));
+        }
+        fs::permissions(storage / dir,
+                        fs::perms::add_perms | fs::perms::owner_all);
+      }
+      for (const auto &file : kFilesAll) {
+        ASSERT_TRUE(fs::exists(storage / dir / file));
+        auto file_status = fs::symlink_status(storage / dir / file);
+        if (!utils::StartsWith(file, "symlink")) {
+          ASSERT_EQ(file_status.permissions() & fs::perms::all,
+                    GetPermsFromFilename(file));
+        }
+      }
+    }
+
+    Clear();
+  }
+
+  fs::path storage{fs::temp_directory_path() / "MG_test_unit_utils_file"};
+
+ private:
+  void Clear() {
+    if (fs::exists(storage)) {
+      for (auto &file : fs::recursive_directory_iterator(storage)) {
+        std::error_code error_code;  // For exception suppression.
+        fs::permissions(file.path(),
+                        fs::perms::add_perms | fs::perms::owner_all,
+                        error_code);
+      }
+      fs::remove_all(storage);
+    }
+  }
+};
+
+TEST(UtilsFile, PermissionDenied) {
+  auto ret = utils::ReadLines("/root/.bashrc");
+  ASSERT_EQ(ret.size(), 0);
+}
+
+TEST_F(UtilsFileTest, ReadLines) {
+  for (const auto &dir : kDirsAll) {
+    for (const auto &file : kFilesAll) {
+      auto ret = utils::ReadLines(storage / dir / file);
+      if (utils::EndsWith(dir, "000") || utils::EndsWith(file, "000")) {
+        ASSERT_EQ(ret.size(), 0);
+      } else {
+        ASSERT_EQ(ret.size(), 2);
+      }
+    }
+  }
+}
+
+TEST_F(UtilsFileTest, EnsureDirAndDeleteDir) {
+  for (const auto &dir : kDirsAll) {
+    for (const auto &file : kFilesAll) {
+      ASSERT_FALSE(utils::EnsureDir(storage / dir / file));
+      ASSERT_FALSE(utils::DeleteDir(storage / dir / file));
+    }
+    auto path = storage / dir / "test";
+    auto ret = utils::EnsureDir(path);
+    if (utils::EndsWith(dir, "000")) {
+      ASSERT_FALSE(ret);
+      ASSERT_FALSE(utils::DeleteDir(path));
+    } else {
+      ASSERT_TRUE(ret);
+      ASSERT_TRUE(fs::exists(path));
+      ASSERT_TRUE(fs::is_directory(path));
+      CreateFile(path / "test");
+      ASSERT_TRUE(utils::DeleteDir(path));
+    }
+  }
+}
+
+TEST_F(UtilsFileTest, EnsureDirOrDie) {
+  for (const auto &dir : kDirsAll) {
+    for (const auto &file : kFilesAll) {
+      ASSERT_DEATH(utils::EnsureDirOrDie(storage / dir / file), "");
+    }
+    auto path = storage / dir / "test";
+    if (utils::EndsWith(dir, "000")) {
+      ASSERT_DEATH(utils::EnsureDirOrDie(path), "");
+    } else {
+      utils::EnsureDirOrDie(path);
+    }
+  }
+}
+
+TEST_F(UtilsFileTest, LogFileExisting) {
+  for (const auto &dir : kDirsAll) {
+    for (const auto &file : kFilesAll) {
+      utils::LogFile handle;
+      if (utils::EndsWith(dir, "000") || utils::EndsWith(file, "000")) {
+        ASSERT_DEATH(handle.Open(storage / dir / file), "");
+      } else {
+        handle.Open(storage / dir / file);
+        ASSERT_TRUE(handle.IsOpen());
+        ASSERT_EQ(handle.path(), storage / dir / file);
+        handle.Write("hello world!\n", 13);
+        handle.Sync();
+        handle.Close();
+      }
+    }
+  }
+}
+
+TEST_F(UtilsFileTest, LogFileNew) {
+  for (const auto &dir : kDirsAll) {
+    utils::LogFile handle;
+    auto path = storage / dir / "test";
+    if (utils::EndsWith(dir, "000")) {
+      ASSERT_DEATH(handle.Open(path), "");
+    } else {
+      handle.Open(path);
+      ASSERT_TRUE(handle.IsOpen());
+      ASSERT_EQ(handle.path(), path);
+      handle.Write("hello world!\n");
+      handle.Sync();
+      handle.Close();
+    }
+  }
+}
+
+TEST_F(UtilsFileTest, LogFileInvalidUsage) {
+  utils::LogFile handle;
+  ASSERT_DEATH(handle.Write("hello!"), "");
+  ASSERT_DEATH(handle.Sync(), "");
+  ASSERT_DEATH(handle.Close(), "");
+  handle.Open(storage / "existing_dir_777" / "existing_file_777");
+  ASSERT_DEATH(handle.Open(storage / "existing_dir_770" / "existing_file_770"),
+               "");
+  handle.Write("hello!");
+  handle.Sync();
+  handle.Close();
+}
+
+TEST_F(UtilsFileTest, LogFileMove) {
+  utils::LogFile original;
+  original.Open(storage / "existing_dir_777" / "existing_file_777");
+
+  utils::LogFile moved(std::move(original));
+
+  ASSERT_DEATH(original.Write("hello!"), "");
+  ASSERT_DEATH(original.Sync(), "");
+  ASSERT_DEATH(original.Close(), "");
+  ASSERT_EQ(original.path(), "");
+  ASSERT_FALSE(original.IsOpen());
+
+  ASSERT_TRUE(moved.IsOpen());
+  ASSERT_EQ(moved.path(), storage / "existing_dir_777" / "existing_file_777");
+  moved.Write("hello!");
+  moved.Sync();
+  moved.Close();
+
+  original.Open(storage / "existing_dir_770" / "existing_file_770");
+  original.Close();
+}
+
+TEST_F(UtilsFileTest, LogFileDescriptorLeackage) {
+  for (int i = 0; i < 100000; ++i) {
+    utils::LogFile handle;
+    handle.Open(storage / "existing_dir_777" / "existing_file_777");
+  }
+}