Gentle introduction of log backend
The goal is to be able to write short logs to the pre-allocated
memory. To do that, we want to decouple file operations from logs
behind light abstraction.
There are a couple of TODO added. I hope to fix them with next iteration
where actual memory log backend will be implemented.
Change-Id: I65e80825b1e080375efc54f35b270df1ceb17a0d
Signed-off-by: Austin Schuh <austin.linux@gmail.com>
diff --git a/aos/events/logging/BUILD b/aos/events/logging/BUILD
index 1edc142..85d27b4 100644
--- a/aos/events/logging/BUILD
+++ b/aos/events/logging/BUILD
@@ -73,6 +73,34 @@
)
cc_library(
+ name = "log_backend",
+ srcs = ["log_backend.cc"],
+ hdrs = ["log_backend.h"],
+ target_compatible_with = ["@platforms//os:linux"],
+ visibility = ["//visibility:public"],
+ deps = [
+ "//aos/time",
+ "//aos/util:file",
+ "@com_github_google_glog//:glog",
+ "@com_google_absl//absl/strings",
+ "@com_google_absl//absl/types:span",
+ ],
+)
+
+cc_test(
+ name = "log_backend_test",
+ srcs = ["log_backend_test.cc"],
+ shard_count = 4,
+ target_compatible_with = ["@platforms//os:linux"],
+ deps = [
+ ":log_backend",
+ "//aos/testing:googletest",
+ "//aos/testing:tmpdir",
+ "@com_github_google_glog//:glog",
+ ],
+)
+
+cc_library(
name = "log_reader_utils",
srcs = [
"log_reader_utils.cc",
@@ -116,6 +144,7 @@
":snappy_encoder",
":buffer_encoder",
":logger_fbs",
+ ":log_backend",
"//aos:uuid",
"//aos:configuration",
"//aos:flatbuffer_merge",
diff --git a/aos/events/logging/log_backend.cc b/aos/events/logging/log_backend.cc
new file mode 100644
index 0000000..f47b16d
--- /dev/null
+++ b/aos/events/logging/log_backend.cc
@@ -0,0 +1,346 @@
+#include "aos/events/logging/log_backend.h"
+
+#include <dirent.h>
+
+#include <filesystem>
+
+#include "absl/strings/str_cat.h"
+#include "aos/util/file.h"
+#include "glog/logging.h"
+
+DEFINE_bool(direct, false,
+ "If true, write using O_DIRECT and write 512 byte aligned blocks "
+ "whenever possible.");
+
+namespace aos::logger {
+namespace {
+constexpr const char *kTempExtension = ".tmp";
+}
+
+FileHandler::FileHandler(std::string filename)
+ : filename_(std::move(filename)), supports_odirect_(FLAGS_direct) {}
+
+FileHandler::~FileHandler() { Close(); }
+
+WriteCode FileHandler::OpenForWrite() {
+ iovec_.reserve(10);
+ if (!aos::util::MkdirPIfSpace(filename_, 0777)) {
+ return WriteCode::kOutOfSpace;
+ } else {
+ fd_ = open(filename_.c_str(), O_RDWR | O_CLOEXEC | O_CREAT | O_EXCL, 0774);
+ if (fd_ == -1 && errno == ENOSPC) {
+ return WriteCode::kOutOfSpace;
+ } else {
+ PCHECK(fd_ != -1) << ": Failed to open " << filename_ << " for writing";
+ VLOG(1) << "Opened " << filename_ << " for writing";
+ }
+
+ flags_ = fcntl(fd_, F_GETFL, 0);
+ PCHECK(flags_ >= 0) << ": Failed to get flags for " << this->filename();
+
+ EnableDirect();
+
+ CHECK(std::filesystem::exists(filename_));
+
+ return WriteCode::kOk;
+ }
+}
+
+void FileHandler::EnableDirect() {
+ if (supports_odirect_ && !ODirectEnabled()) {
+ const int new_flags = flags_ | O_DIRECT;
+ // Track if we failed to set O_DIRECT. Note: Austin hasn't seen this call
+ // fail. The write call tends to fail instead.
+ if (fcntl(fd_, F_SETFL, new_flags) == -1) {
+ PLOG(WARNING) << "Failed to set O_DIRECT on " << filename();
+ supports_odirect_ = false;
+ } else {
+ VLOG(1) << "Enabled O_DIRECT on " << filename();
+ flags_ = new_flags;
+ }
+ }
+}
+
+void FileHandler::DisableDirect() {
+ if (supports_odirect_ && ODirectEnabled()) {
+ flags_ = flags_ & (~O_DIRECT);
+ PCHECK(fcntl(fd_, F_SETFL, flags_) != -1) << ": Failed to disable O_DIRECT";
+ VLOG(1) << "Disabled O_DIRECT on " << filename();
+ }
+}
+
+WriteResult FileHandler::Write(
+ const absl::Span<const absl::Span<const uint8_t>> &queue) {
+ iovec_.clear();
+ size_t iovec_size = std::min<size_t>(queue.size(), IOV_MAX);
+ iovec_.resize(iovec_size);
+ size_t counted_size = 0;
+
+ // Ok, we now need to figure out if we were aligned, and if we were, how much
+ // of the data we are being asked to write is aligned.
+ //
+ // The file is aligned if it is a multiple of kSector in length. The data is
+ // aligned if it's memory is kSector aligned, and the length is a multiple of
+ // kSector in length.
+ bool aligned = (total_write_bytes_ % kSector) == 0;
+ size_t write_index = 0;
+ for (size_t i = 0; i < iovec_size; ++i) {
+ iovec_[write_index].iov_base = const_cast<uint8_t *>(queue[i].data());
+
+ // Make sure the address is aligned, or give up. This should be uncommon,
+ // but is always possible.
+ if ((reinterpret_cast<size_t>(iovec_[write_index].iov_base) &
+ (kSector - 1)) != 0) {
+ aligned = false;
+ }
+
+ // Now, see if the length is a multiple of kSector. The goal is to figure
+ // out if/how much memory we can write out with O_DIRECT so that only the
+ // last little bit is done with non-direct IO to keep it fast.
+ iovec_[write_index].iov_len = queue[i].size();
+ if ((iovec_[write_index].iov_len % kSector) != 0) {
+ VLOG(1) << "Unaligned length on " << filename();
+ // If we've got over a sector of data to write, write it out with O_DIRECT
+ // and then continue writing the rest unaligned.
+ if (aligned && iovec_[write_index].iov_len > kSector) {
+ const size_t aligned_size =
+ iovec_[write_index].iov_len & (~(kSector - 1));
+ VLOG(1) << "Was aligned, writing last chunk rounded from "
+ << queue[i].size() << " to " << aligned_size;
+ iovec_[write_index].iov_len = aligned_size;
+
+ const auto code =
+ WriteV(iovec_.data(), i + 1, true, counted_size + aligned_size);
+ if (code == WriteCode::kOutOfSpace) {
+ return {
+ .code = code,
+ .messages_written = i,
+ };
+ }
+
+ // Now, everything before here has been written. Make an iovec out of
+ // the last bytes, and keep going.
+ // TODO (Alexei, Austin): is it safe to do here since it can be a
+ // situation when i >= iovec_size
+ iovec_size -= write_index;
+ iovec_.resize(iovec_size);
+ write_index = 0;
+ counted_size = 0;
+
+ iovec_[write_index].iov_base =
+ const_cast<uint8_t *>(queue[i].data() + aligned_size);
+ iovec_[write_index].iov_len = queue[i].size() - aligned_size;
+ }
+ aligned = false;
+ }
+ VLOG(1) << "Writing " << iovec_[write_index].iov_len << " to "
+ << filename();
+ counted_size += iovec_[write_index].iov_len;
+ ++write_index;
+ }
+
+ // Either write the aligned data if it is all aligned, or write the rest
+ // unaligned if we wrote aligned up above.
+ const auto code = WriteV(iovec_.data(), iovec_.size(), aligned, counted_size);
+ return {
+ .code = code,
+ .messages_written = iovec_size,
+ };
+}
+
+WriteCode FileHandler::WriteV(struct iovec *iovec_data, size_t iovec_size,
+ bool aligned, size_t counted_size) {
+ // Configure the file descriptor to match the mode we should be in. This is
+ // safe to over-call since it only does the syscall if needed.
+ if (aligned) {
+ EnableDirect();
+ } else {
+ DisableDirect();
+ }
+
+ const auto start = aos::monotonic_clock::now();
+ const ssize_t written = writev(fd_, iovec_data, iovec_size);
+
+ if (written > 0) {
+ // Flush asynchronously and force the data out of the cache.
+ sync_file_range(fd_, total_write_bytes_, written, SYNC_FILE_RANGE_WRITE);
+ if (last_synced_bytes_ != 0) {
+ // Per Linus' recommendation online on how to do fast file IO, do a
+ // blocking flush of the previous write chunk, and then tell the kernel to
+ // drop the pages from the cache. This makes sure we can't get too far
+ // ahead.
+ sync_file_range(fd_, last_synced_bytes_,
+ total_write_bytes_ - last_synced_bytes_,
+ SYNC_FILE_RANGE_WAIT_BEFORE | SYNC_FILE_RANGE_WRITE |
+ SYNC_FILE_RANGE_WAIT_AFTER);
+ posix_fadvise(fd_, last_synced_bytes_,
+ total_write_bytes_ - last_synced_bytes_,
+ POSIX_FADV_DONTNEED);
+
+ last_synced_bytes_ = total_write_bytes_;
+ }
+ }
+
+ const auto end = aos::monotonic_clock::now();
+ if (written == -1 && errno == ENOSPC) {
+ return WriteCode::kOutOfSpace;
+ }
+ PCHECK(written >= 0) << ": write failed, got " << written;
+ if (written < static_cast<ssize_t>(counted_size)) {
+ // Sometimes this happens instead of ENOSPC. On a real filesystem, this
+ // never seems to happen in any other case. If we ever want to log to a
+ // socket, this will happen more often. However, until we get there, we'll
+ // just assume it means we ran out of space.
+ return WriteCode::kOutOfSpace;
+ }
+
+ total_write_bytes_ += written;
+ write_stats_.UpdateStats(end - start, written, iovec_size);
+ return WriteCode::kOk;
+}
+
+WriteCode FileHandler::Close() {
+ if (!is_open()) {
+ return WriteCode::kOk;
+ }
+ bool ran_out_of_space = false;
+ if (close(fd_) == -1) {
+ if (errno == ENOSPC) {
+ ran_out_of_space = true;
+ } else {
+ PLOG(ERROR) << "Closing log file failed";
+ }
+ }
+ fd_ = -1;
+ VLOG(1) << "Closed " << filename_;
+ return ran_out_of_space ? WriteCode::kOutOfSpace : WriteCode::kOk;
+}
+
+FileBackend::FileBackend(std::string_view base_name)
+ : base_name_(base_name), separator_(base_name_.back() == '/' ? "" : "_") {}
+
+std::unique_ptr<FileHandler> FileBackend::RequestFile(std::string_view id) {
+ const std::string filename = absl::StrCat(base_name_, separator_, id);
+ return std::make_unique<FileHandler>(filename);
+}
+
+RenamableFileBackend::RenamableFileBackend(std::string_view base_name)
+ : base_name_(base_name), separator_(base_name_.back() == '/' ? "" : "_") {}
+
+std::unique_ptr<FileHandler> RenamableFileBackend::RequestFile(
+ std::string_view id) {
+ const std::string filename =
+ absl::StrCat(base_name_, separator_, id, temp_suffix_);
+ return std::make_unique<RenamableFileHandler>(this, filename);
+}
+
+void RenamableFileBackend::EnableTempFiles() {
+ use_temp_files_ = true;
+ temp_suffix_ = kTempExtension;
+}
+
+bool RenamableFileBackend::RenameLogBase(std::string_view new_base_name) {
+ if (new_base_name == base_name_) {
+ return true;
+ }
+ CHECK(old_base_name_.empty())
+ << "Only one change of base_name is supported. Was: " << old_base_name_;
+
+ std::string current_directory = base_name_;
+ std::string new_directory(new_base_name);
+
+ auto current_path_split = current_directory.rfind("/");
+ CHECK(current_path_split != std::string::npos)
+ << "Could not find / in the current directory path";
+ auto new_path_split = new_directory.rfind("/");
+ CHECK(new_path_split != std::string::npos)
+ << "Could not find / in the new directory path";
+
+ CHECK(new_base_name.substr(new_path_split) ==
+ current_directory.substr(current_path_split))
+ << "Rename of file base from " << current_directory << " to "
+ << new_directory << " is not supported.";
+
+ current_directory.resize(current_path_split);
+ new_directory.resize(new_path_split);
+ DIR *dir = opendir(current_directory.c_str());
+ if (dir) {
+ closedir(dir);
+ const int result = rename(current_directory.c_str(), new_directory.c_str());
+ if (result != 0) {
+ PLOG(ERROR) << "Unable to rename " << current_directory << " to "
+ << new_directory;
+ return false;
+ }
+ } else {
+ // Handle if directory was already renamed.
+ dir = opendir(new_directory.c_str());
+ if (!dir) {
+ LOG(ERROR) << "Old directory " << current_directory
+ << " missing and new directory " << new_directory
+ << " not present.";
+ return false;
+ }
+ closedir(dir);
+ }
+ old_base_name_ = base_name_;
+ base_name_ = std::string(new_base_name);
+ separator_ = base_name_.back() == '/' ? "" : "_";
+ return true;
+}
+
+WriteCode RenamableFileBackend::RenameFileAfterClose(
+ std::string_view filename) {
+ // Fast check that we can skip rename.
+ if (!use_temp_files_ && old_base_name_.empty()) {
+ return WriteCode::kOk;
+ }
+
+ std::string current_filename(filename);
+
+ // When changing the base name, we rename the log folder while there active
+ // buffer writers. Therefore, the name of that active buffer may still refer
+ // to the old file location rather than the new one.
+ if (!old_base_name_.empty()) {
+ auto offset = current_filename.find(old_base_name_);
+ if (offset != std::string::npos) {
+ current_filename.replace(offset, old_base_name_.length(), base_name_);
+ }
+ }
+
+ std::string final_filename = current_filename;
+ if (use_temp_files_) {
+ CHECK(current_filename.size() > temp_suffix_.size());
+ final_filename = current_filename.substr(
+ 0, current_filename.size() - temp_suffix_.size());
+ }
+
+ int result = rename(current_filename.c_str(), final_filename.c_str());
+
+ bool ran_out_of_space = false;
+ if (result != 0) {
+ if (errno == ENOSPC) {
+ ran_out_of_space = true;
+ } else {
+ PLOG(FATAL) << "Renaming " << current_filename << " to " << final_filename
+ << " failed";
+ }
+ } else {
+ VLOG(1) << "Renamed " << current_filename << " -> " << final_filename;
+ }
+ return ran_out_of_space ? WriteCode::kOutOfSpace : WriteCode::kOk;
+}
+
+WriteCode RenamableFileBackend::RenamableFileHandler::Close() {
+ if (!is_open()) {
+ return WriteCode::kOk;
+ }
+ if (FileHandler::Close() == WriteCode::kOutOfSpace) {
+ return WriteCode::kOutOfSpace;
+ }
+ if (owner_->RenameFileAfterClose(filename()) == WriteCode::kOutOfSpace) {
+ return WriteCode::kOutOfSpace;
+ }
+ return WriteCode::kOk;
+}
+} // namespace aos::logger
\ No newline at end of file
diff --git a/aos/events/logging/log_backend.h b/aos/events/logging/log_backend.h
new file mode 100644
index 0000000..4dd393e
--- /dev/null
+++ b/aos/events/logging/log_backend.h
@@ -0,0 +1,255 @@
+#ifndef AOS_EVENTS_LOGGING_LOG_BACKEND_H_
+#define AOS_EVENTS_LOGGING_LOG_BACKEND_H_
+
+#include <fcntl.h>
+#include <sys/types.h>
+#include <sys/uio.h>
+
+#include <memory>
+#include <string>
+#include <vector>
+
+#include "absl/types/span.h"
+#include "aos/time/time.h"
+
+namespace aos::logger {
+
+class WriteStats {
+ public:
+ // The maximum time for a single write call, or 0 if none have been performed.
+ std::chrono::nanoseconds max_write_time() const { return max_write_time_; }
+ // The number of bytes in the longest write call, or -1 if none have been
+ // performed.
+ int max_write_time_bytes() const { return max_write_time_bytes_; }
+ // The number of buffers in the longest write call, or -1 if none have been
+ // performed.
+ int max_write_time_messages() const { return max_write_time_messages_; }
+ // The total time spent in write calls.
+ std::chrono::nanoseconds total_write_time() const {
+ return total_write_time_;
+ }
+ // The total number of writes which have been performed.
+ int total_write_count() const { return total_write_count_; }
+ // The total number of messages which have been written.
+ int total_write_messages() const { return total_write_messages_; }
+ // The total number of bytes which have been written.
+ int total_write_bytes() const { return total_write_bytes_; }
+
+ void ResetStats() {
+ max_write_time_ = std::chrono::nanoseconds::zero();
+ max_write_time_bytes_ = -1;
+ max_write_time_messages_ = -1;
+ total_write_time_ = std::chrono::nanoseconds::zero();
+ total_write_count_ = 0;
+ total_write_messages_ = 0;
+ total_write_bytes_ = 0;
+ }
+
+ void UpdateStats(aos::monotonic_clock::duration duration, ssize_t written,
+ int iovec_size) {
+ if (duration > max_write_time_) {
+ max_write_time_ = duration;
+ max_write_time_bytes_ = written;
+ max_write_time_messages_ = iovec_size;
+ }
+ total_write_time_ += duration;
+ ++total_write_count_;
+ total_write_messages_ += iovec_size;
+ total_write_bytes_ += written;
+ }
+
+ private:
+ std::chrono::nanoseconds max_write_time_ = std::chrono::nanoseconds::zero();
+ int max_write_time_bytes_ = -1;
+ int max_write_time_messages_ = -1;
+ std::chrono::nanoseconds total_write_time_ = std::chrono::nanoseconds::zero();
+ int total_write_count_ = 0;
+ int total_write_messages_ = 0;
+ int total_write_bytes_ = 0;
+};
+
+// Currently, all write operations only cares about out-of-space error. This is
+// a simple representation of write result.
+enum class WriteCode { kOk, kOutOfSpace };
+
+struct WriteResult {
+ WriteCode code = WriteCode::kOk;
+ size_t messages_written = 0;
+};
+
+// FileHandler is a replacement for bare filename in log writing and reading
+// operations.
+//
+// There are a couple over-arching constraints on writing to keep track of.
+// 1) The kernel is both faster and more efficient at writing large, aligned
+// chunks with O_DIRECT set on the file. The alignment needed is specified
+// by kSector and is file system dependent.
+// 2) Not all encoders support generating round multiples of kSector of data.
+// Rather than burden the API for detecting when that is the case, we want
+// DetachedBufferWriter to be as efficient as it can at writing what given.
+// 3) Some files are small and not updated frequently. They need to be
+// flushed or we will lose data on power off. It is most efficient to write
+// as much as we can aligned by kSector and then fall back to the non direct
+// method when it has been flushed.
+// 4) Not all filesystems support O_DIRECT, and different sizes may be optimal
+// for different machines. The defaults should work decently anywhere and
+// be tunable for faster systems.
+// TODO (Alexei): need 2 variations, to support systems with and without
+// O_DIRECT
+class FileHandler {
+ public:
+ // Size of an aligned sector used to detect when the data is aligned enough to
+ // use O_DIRECT instead.
+ static constexpr size_t kSector = 512u;
+
+ explicit FileHandler(std::string filename);
+ virtual ~FileHandler();
+
+ FileHandler(const FileHandler &) = delete;
+ FileHandler &operator=(const FileHandler &) = delete;
+
+ // Try to open file. App will crash if there are other than out-of-space
+ // problems with backend media.
+ virtual WriteCode OpenForWrite();
+
+ // Close the file handler.
+ virtual WriteCode Close();
+
+ // This will be true until Close() is called, unless the file couldn't be
+ // created due to running out of space.
+ bool is_open() const { return fd_ != -1; }
+
+ // Peeks messages from queue and writes it to file. Returns code when
+ // out-of-space problem occurred along with number of messages from queue that
+ // was written.
+ virtual WriteResult Write(
+ const absl::Span<const absl::Span<const uint8_t>> &queue);
+
+ // TODO (Alexei): it is rather leaked abstraction.
+ // Path to the concrete log file.
+ std::string_view filename() const { return filename_; }
+
+ int fd() const { return fd_; }
+
+ // Get access to statistics related to the write operations.
+ WriteStats *WriteStatistics() { return &write_stats_; }
+
+ private:
+ // Enables O_DIRECT on the open file if it is supported. Cheap to call if it
+ // is already enabled.
+ void EnableDirect();
+ // Disables O_DIRECT on the open file if it is supported. Cheap to call if it
+ // is already disabld.
+ void DisableDirect();
+
+ bool ODirectEnabled() const { return !!(flags_ & O_DIRECT); }
+
+ // Writes a chunk of iovecs. aligned is true if all the data is kSector byte
+ // aligned and multiples of it in length, and counted_size is the sum of the
+ // sizes of all the chunks of data.
+ WriteCode WriteV(struct iovec *iovec_data, size_t iovec_size, bool aligned,
+ size_t counted_size);
+
+ const std::string filename_;
+
+ int fd_ = -1;
+
+ // List of iovecs to use with writev. This is a member variable to avoid
+ // churn.
+ std::vector<struct iovec> iovec_;
+
+ int total_write_bytes_ = 0;
+ int last_synced_bytes_ = 0;
+
+ bool supports_odirect_ = true;
+ int flags_ = 0;
+
+ WriteStats write_stats_;
+};
+
+// Class that decouples log writing and media (file system or memory). It is
+// handy to use for tests.
+class LogBackend {
+ public:
+ virtual ~LogBackend() = default;
+
+ // Request file-like object from the log backend. It maybe a file on a disk or
+ // in memory. id is usually generated by log namer and looks like name of the
+ // file within a log folder.
+ virtual std::unique_ptr<FileHandler> RequestFile(std::string_view id) = 0;
+};
+
+// Implements requests log files from file system.
+class FileBackend : public LogBackend {
+ public:
+ // base_name is the path to the folder where log files are.
+ explicit FileBackend(std::string_view base_name);
+ ~FileBackend() override = default;
+
+ // Request file from a file system. It is not open yet.
+ std::unique_ptr<FileHandler> RequestFile(std::string_view id) override;
+
+ private:
+ const std::string base_name_;
+ const std::string_view separator_;
+};
+
+// Provides a file backend that supports renaming of the base log folder and
+// temporary files.
+class RenamableFileBackend : public LogBackend {
+ public:
+ // Adds call to rename, when closed.
+ class RenamableFileHandler final : public FileHandler {
+ public:
+ RenamableFileHandler(RenamableFileBackend *owner, std::string filename)
+ : FileHandler(std::move(filename)), owner_(owner) {}
+ ~RenamableFileHandler() final = default;
+
+ // Returns false if not enough memory, true otherwise.
+ WriteCode Close() final;
+
+ private:
+ RenamableFileBackend *owner_;
+ };
+
+ explicit RenamableFileBackend(std::string_view base_name);
+ ~RenamableFileBackend() = default;
+
+ // Request file from a file system. It is not open yet.
+ std::unique_ptr<FileHandler> RequestFile(std::string_view id) override;
+
+ // TODO (Alexei): it is called by Logger, and left here for compatibility.
+ // Logger should not call it.
+ std::string_view base_name() { return base_name_; }
+
+ // If temp files are enabled, then this will write files with the .tmp
+ // suffix, and then rename them to the desired name after they are fully
+ // written.
+ //
+ // This is useful to enable incremental copying of the log files.
+ //
+ // Defaults to writing directly to the final filename.
+ void EnableTempFiles();
+
+ // Moves the current log location to the new name. Returns true if a change
+ // was made, false otherwise.
+ // Only renaming the folder is supported, not the file base name.
+ bool RenameLogBase(std::string_view new_base_name);
+
+ private:
+ // This function called after file closed, to adjust file names in case of
+ // base name was changed or temp files enabled.
+ WriteCode RenameFileAfterClose(std::string_view filename);
+
+ std::string base_name_;
+ std::string_view separator_;
+
+ bool use_temp_files_ = false;
+ std::string_view temp_suffix_;
+
+ std::string old_base_name_;
+};
+
+} // namespace aos::logger
+
+#endif // AOS_EVENTS_LOGGING_LOG_BACKEND_H_
diff --git a/aos/events/logging/log_backend_test.cc b/aos/events/logging/log_backend_test.cc
new file mode 100644
index 0000000..452592f
--- /dev/null
+++ b/aos/events/logging/log_backend_test.cc
@@ -0,0 +1,87 @@
+#include "aos/events/logging/log_backend.h"
+
+#include <filesystem>
+
+#include "aos/testing/tmpdir.h"
+#include "gtest/gtest.h"
+
+namespace aos::logger::testing {
+TEST(LogBackendTest, CreateSimpleFile) {
+ const std::string logevent = aos::testing::TestTmpDir() + "/logevent/";
+ FileBackend backend(logevent);
+ auto file = backend.RequestFile("test.log");
+ ASSERT_EQ(file->OpenForWrite(), WriteCode::kOk);
+ auto result = write(file->fd(), "test", 4);
+ EXPECT_GT(result, 0);
+ EXPECT_EQ(file->Close(), WriteCode::kOk);
+ EXPECT_TRUE(std::filesystem::exists(logevent + "test.log"));
+}
+
+TEST(LogBackendTest, CreateRenamableFile) {
+ const std::string logevent = aos::testing::TestTmpDir() + "/logevent/";
+ RenamableFileBackend backend(logevent);
+ auto file = backend.RequestFile("test.log");
+ ASSERT_EQ(file->OpenForWrite(), WriteCode::kOk);
+ auto result = write(file->fd(), "testtest", 8);
+ EXPECT_GT(result, 0);
+ EXPECT_EQ(file->Close(), WriteCode::kOk);
+ EXPECT_TRUE(std::filesystem::exists(logevent + "test.log"));
+}
+
+TEST(LogBackendTest, UseTempRenamableFile) {
+ const std::string logevent = aos::testing::TestTmpDir() + "/logevent/";
+ RenamableFileBackend backend(logevent);
+ backend.EnableTempFiles();
+ auto file = backend.RequestFile("test.log");
+ ASSERT_EQ(file->OpenForWrite(), WriteCode::kOk);
+ auto result = write(file->fd(), "testtest", 8);
+ EXPECT_GT(result, 0);
+ EXPECT_TRUE(std::filesystem::exists(logevent + "test.log.tmp"));
+
+ EXPECT_EQ(file->Close(), WriteCode::kOk);
+ // Check that file is renamed.
+ EXPECT_TRUE(std::filesystem::exists(logevent + "test.log"));
+}
+
+TEST(LogBackendTest, RenameBaseAfterWrite) {
+ const std::string logevent = aos::testing::TestTmpDir() + "/logevent/";
+ RenamableFileBackend backend(logevent);
+ auto file = backend.RequestFile("test.log");
+ ASSERT_EQ(file->OpenForWrite(), WriteCode::kOk);
+ auto result = write(file->fd(), "testtest", 8);
+ EXPECT_GT(result, 0);
+ EXPECT_TRUE(std::filesystem::exists(logevent + "test.log"));
+
+ std::string renamed = aos::testing::TestTmpDir() + "/renamed/";
+ backend.RenameLogBase(renamed);
+
+ EXPECT_FALSE(std::filesystem::exists(logevent + "test.log"));
+ EXPECT_TRUE(std::filesystem::exists(renamed + "test.log"));
+
+ EXPECT_EQ(file->Close(), WriteCode::kOk);
+ // Check that file is renamed.
+ EXPECT_TRUE(std::filesystem::exists(renamed + "test.log"));
+}
+
+TEST(LogBackendTest, UseTestAndRenameBaseAfterWrite) {
+ const std::string logevent = aos::testing::TestTmpDir() + "/logevent/";
+ RenamableFileBackend backend(logevent);
+ backend.EnableTempFiles();
+ auto file = backend.RequestFile("test.log");
+ ASSERT_EQ(file->OpenForWrite(), WriteCode::kOk);
+ auto result = write(file->fd(), "testtest", 8);
+ EXPECT_GT(result, 0);
+ EXPECT_TRUE(std::filesystem::exists(logevent + "test.log.tmp"));
+
+ std::string renamed = aos::testing::TestTmpDir() + "/renamed/";
+ backend.RenameLogBase(renamed);
+
+ EXPECT_FALSE(std::filesystem::exists(logevent + "test.log.tmp"));
+ EXPECT_TRUE(std::filesystem::exists(renamed + "test.log.tmp"));
+
+ EXPECT_EQ(file->Close(), WriteCode::kOk);
+ // Check that file is renamed.
+ EXPECT_TRUE(std::filesystem::exists(renamed + "test.log"));
+}
+
+} // namespace aos::logger::testing
\ No newline at end of file
diff --git a/aos/events/logging/log_edit.cc b/aos/events/logging/log_edit.cc
index dca56bb..55b7666 100644
--- a/aos/events/logging/log_edit.cc
+++ b/aos/events/logging/log_edit.cc
@@ -48,7 +48,7 @@
aos::logger::SpanReader span_reader(orig_path);
CHECK(!span_reader.ReadMessage().empty()) << ": Empty header, aborting";
- aos::logger::DetachedBufferWriter buffer_writer(
+ aos::logger::DetachedBufferFileWriter buffer_writer(
FLAGS_logfile,
std::make_unique<aos::logger::DummyEncoder>(FLAGS_max_message_size));
{
diff --git a/aos/events/logging/log_namer.cc b/aos/events/logging/log_namer.cc
index 86d813f..8ec1e70 100644
--- a/aos/events/logging/log_namer.cc
+++ b/aos/events/logging/log_namer.cc
@@ -560,17 +560,16 @@
return result;
}
-MultiNodeLogNamer::MultiNodeLogNamer(std::string_view base_name,
- EventLoop *event_loop)
- : MultiNodeLogNamer(base_name, event_loop->configuration(), event_loop,
- event_loop->node()) {}
+MultiNodeLogNamer::MultiNodeLogNamer(
+ std::unique_ptr<RenamableFileBackend> log_backend, EventLoop *event_loop)
+ : MultiNodeLogNamer(std::move(log_backend), event_loop->configuration(),
+ event_loop, event_loop->node()) {}
-MultiNodeLogNamer::MultiNodeLogNamer(std::string_view base_name,
- const Configuration *configuration,
- EventLoop *event_loop, const Node *node)
+MultiNodeLogNamer::MultiNodeLogNamer(
+ std::unique_ptr<RenamableFileBackend> log_backend,
+ const Configuration *configuration, EventLoop *event_loop, const Node *node)
: LogNamer(configuration, event_loop, node),
- base_name_(base_name),
- old_base_name_(),
+ log_backend_(std::move(log_backend)),
encoder_factory_([](size_t max_message_size) {
// TODO(austin): For slow channels, can we allocate less memory?
return std::make_unique<DummyEncoder>(max_message_size,
@@ -606,21 +605,19 @@
return;
}
- const std::string_view separator = base_name_.back() == '/' ? "" : "_";
- const std::string filename = absl::StrCat(
- base_name_, separator, config_sha256, ".bfbs", extension_, temp_suffix_);
-
+ const std::string filename = absl::StrCat(config_sha256, ".bfbs", extension_);
+ auto file_handle = log_backend_->RequestFile(filename);
std::unique_ptr<DetachedBufferWriter> writer =
std::make_unique<DetachedBufferWriter>(
- filename, encoder_factory_(header->span().size()));
+ std::move(file_handle), encoder_factory_(header->span().size()));
DataEncoder::SpanCopier coppier(header->span());
writer->CopyMessage(&coppier, aos::monotonic_clock::now());
if (!writer->ran_out_of_space()) {
- all_filenames_.emplace_back(
- absl::StrCat(config_sha256, ".bfbs", extension_));
+ all_filenames_.emplace_back(filename);
}
+ // Close the file and maybe rename it too.
CloseWriter(&writer);
}
@@ -646,8 +643,8 @@
if (!data_writer_) {
MakeDataWriter();
}
- data_writer_->UpdateMaxMessageSize(PackMessageSize(
- LogType::kLogRemoteMessage, channel->max_size()));
+ data_writer_->UpdateMaxMessageSize(
+ PackMessageSize(LogType::kLogRemoteMessage, channel->max_size()));
return data_writer_.get();
}
@@ -726,10 +723,10 @@
for (std::pair<const Channel *const, NewDataWriter> &data_writer :
data_writers_) {
if (!data_writer.second.writer) continue;
- data_writer.second.writer->ResetStatistics();
+ data_writer.second.writer->WriteStatistics()->ResetStats();
}
if (data_writer_) {
- data_writer_->writer->ResetStatistics();
+ data_writer_->writer->WriteStatistics()->ResetStats();
}
max_write_time_ = std::chrono::nanoseconds::zero();
max_write_time_bytes_ = -1;
@@ -789,7 +786,7 @@
// Refuse to open any new files, which might skip data. Any existing files
// are in the same folder, which means they're on the same filesystem, which
// means they're probably going to run out of space and get stuck too.
- if (!destination->get()) {
+ if (!(*destination)) {
// But avoid leaving a nullptr writer if we're out of space when
// attempting to open the first file.
*destination = std::make_unique<DetachedBufferWriter>(
@@ -797,102 +794,54 @@
}
return;
}
- const std::string_view separator = base_name_.back() == '/' ? "" : "_";
- const std::string filename =
- absl::StrCat(base_name_, separator, path, temp_suffix_);
- if (!destination->get()) {
+
+ // Let's check that we need to close and replace current driver.
+ if (*destination) {
+ // Let's close the current writer.
+ CloseWriter(destination);
+ // Are we out of space now?
if (ran_out_of_space_) {
*destination = std::make_unique<DetachedBufferWriter>(
DetachedBufferWriter::already_out_of_space_t());
return;
}
- *destination = std::make_unique<DetachedBufferWriter>(
- filename, encoder_factory_(max_message_size));
- if (!destination->get()->ran_out_of_space()) {
- all_filenames_.emplace_back(path);
- }
- return;
}
- CloseWriter(destination);
- if (ran_out_of_space_) {
- *destination->get() =
- DetachedBufferWriter(DetachedBufferWriter::already_out_of_space_t());
- return;
- }
-
- *destination->get() =
- DetachedBufferWriter(filename, encoder_factory_(max_message_size));
- if (!destination->get()->ran_out_of_space()) {
+ const std::string filename(path);
+ *destination = std::make_unique<DetachedBufferWriter>(
+ log_backend_->RequestFile(filename), encoder_factory_(max_message_size));
+ if (!(*destination)->ran_out_of_space()) {
all_filenames_.emplace_back(path);
}
}
-void MultiNodeLogNamer::RenameTempFile(DetachedBufferWriter *destination) {
- if (temp_suffix_.empty()) {
- return;
- }
- std::string current_filename = std::string(destination->filename());
- CHECK(current_filename.size() > temp_suffix_.size());
- std::string final_filename =
- current_filename.substr(0, current_filename.size() - temp_suffix_.size());
- int result = rename(current_filename.c_str(), final_filename.c_str());
-
- // When changing the base name, we rename the log folder while there active
- // buffer writers. Therefore, the name of that active buffer may still refer
- // to the old file location rather than the new one. This minimized changes to
- // existing code.
- if (result != 0 && errno != ENOSPC && !old_base_name_.empty()) {
- auto offset = current_filename.find(old_base_name_);
- if (offset != std::string::npos) {
- current_filename.replace(offset, old_base_name_.length(), base_name_);
- }
- offset = final_filename.find(old_base_name_);
- if (offset != std::string::npos) {
- final_filename.replace(offset, old_base_name_.length(), base_name_);
- }
- result = rename(current_filename.c_str(), final_filename.c_str());
- }
-
- if (result != 0) {
- if (errno == ENOSPC) {
- ran_out_of_space_ = true;
- return;
- } else {
- PLOG(FATAL) << "Renaming " << current_filename << " to " << final_filename
- << " failed";
- }
- } else {
- VLOG(1) << "Renamed " << current_filename << " -> " << final_filename;
- }
-}
-
void MultiNodeLogNamer::CloseWriter(
std::unique_ptr<DetachedBufferWriter> *writer_pointer) {
- DetachedBufferWriter *const writer = writer_pointer->get();
- if (!writer) {
+ CHECK_NOTNULL(writer_pointer);
+ if (!(*writer_pointer)) {
return;
}
+ DetachedBufferWriter *const writer = writer_pointer->get();
const bool was_open = writer->is_open();
writer->Close();
- if (writer->max_write_time() > max_write_time_) {
- max_write_time_ = writer->max_write_time();
- max_write_time_bytes_ = writer->max_write_time_bytes();
- max_write_time_messages_ = writer->max_write_time_messages();
+ const auto *stats = writer->WriteStatistics();
+ if (stats->max_write_time() > max_write_time_) {
+ max_write_time_ = stats->max_write_time();
+ max_write_time_bytes_ = stats->max_write_time_bytes();
+ max_write_time_messages_ = stats->max_write_time_messages();
}
- total_write_time_ += writer->total_write_time();
- total_write_count_ += writer->total_write_count();
- total_write_messages_ += writer->total_write_messages();
- total_write_bytes_ += writer->total_write_bytes();
+ total_write_time_ += stats->total_write_time();
+ total_write_count_ += stats->total_write_count();
+ total_write_messages_ += stats->total_write_messages();
+ total_write_bytes_ += stats->total_write_bytes();
if (writer->ran_out_of_space()) {
ran_out_of_space_ = true;
writer->acknowledge_out_of_space();
}
- if (was_open) {
- RenameTempFile(writer);
- } else {
+
+ if (!was_open) {
CHECK(access(std::string(writer->filename()).c_str(), F_OK) == -1)
<< ": File should not exist: " << writer->filename();
}
diff --git a/aos/events/logging/log_namer.h b/aos/events/logging/log_namer.h
index 07edeac..3b54025 100644
--- a/aos/events/logging/log_namer.h
+++ b/aos/events/logging/log_namer.h
@@ -174,7 +174,7 @@
logger_node_index_(configuration::GetNodeIndex(configuration_, node_)) {
nodes_.emplace_back(node_);
}
- virtual ~LogNamer() {}
+ virtual ~LogNamer() = default;
virtual std::string_view base_name() const = 0;
@@ -183,6 +183,8 @@
// Rotate is called by Logger::RenameLogBase, which is currently the only user
// of this method.
// Only renaming the folder is supported, not the file base name.
+ // TODO (Alexei): it should not be in interface, since it is not applied to
+ // files.
virtual void set_base_name(std::string_view base_name) = 0;
// Returns a writer for writing data from messages on this channel (on the
@@ -297,32 +299,30 @@
aos::SizePrefixedFlatbufferDetachedBuffer<LogFileHeader>::Empty();
};
-// Log namer which uses a config and a base name to name a bunch of files.
+// Log namer which uses a config to name a bunch of files.
class MultiNodeLogNamer : public LogNamer {
public:
- MultiNodeLogNamer(std::string_view base_name, EventLoop *event_loop);
- MultiNodeLogNamer(std::string_view base_name,
+ MultiNodeLogNamer(std::unique_ptr<RenamableFileBackend> log_backend,
+ EventLoop *event_loop);
+ MultiNodeLogNamer(std::unique_ptr<RenamableFileBackend> log_backend,
const Configuration *configuration, EventLoop *event_loop,
const Node *node);
~MultiNodeLogNamer() override;
- std::string_view base_name() const final { return base_name_; }
+ std::string_view base_name() const final { return log_backend_->base_name(); }
void set_base_name(std::string_view base_name) final {
- old_base_name_ = base_name_;
- base_name_ = base_name;
+ log_backend_->RenameLogBase(base_name);
}
- // If temp_suffix is set, then this will write files under names beginning
- // with the specified suffix, and then rename them to the desired name after
+ // When enabled, this will write files under names beginning
+ // with the .tmp suffix, and then rename them to the desired name after
// they are fully written.
//
// This is useful to enable incremental copying of the log files.
//
// Defaults to writing directly to the final filename.
- void set_temp_suffix(std::string_view temp_suffix) {
- temp_suffix_ = temp_suffix;
- }
+ void EnableTempFiles() { log_backend_->EnableTempFiles(); }
// Sets the function for creating encoders. The argument is the max message
// size (including headers) that will be written into this encoder.
@@ -396,7 +396,8 @@
return accumulate_data_writers(
max_write_time_,
[](std::chrono::nanoseconds x, const NewDataWriter &data_writer) {
- return std::max(x, data_writer.writer->max_write_time());
+ return std::max(
+ x, data_writer.writer->WriteStatistics()->max_write_time());
});
}
int max_write_time_bytes() const {
@@ -404,9 +405,11 @@
std::make_tuple(max_write_time_bytes_, max_write_time_),
[](std::tuple<int, std::chrono::nanoseconds> x,
const NewDataWriter &data_writer) {
- if (data_writer.writer->max_write_time() > std::get<1>(x)) {
- return std::make_tuple(data_writer.writer->max_write_time_bytes(),
- data_writer.writer->max_write_time());
+ if (data_writer.writer->WriteStatistics()->max_write_time() >
+ std::get<1>(x)) {
+ return std::make_tuple(
+ data_writer.writer->WriteStatistics()->max_write_time_bytes(),
+ data_writer.writer->WriteStatistics()->max_write_time());
}
return x;
}));
@@ -416,10 +419,12 @@
std::make_tuple(max_write_time_messages_, max_write_time_),
[](std::tuple<int, std::chrono::nanoseconds> x,
const NewDataWriter &data_writer) {
- if (data_writer.writer->max_write_time() > std::get<1>(x)) {
+ if (data_writer.writer->WriteStatistics()->max_write_time() >
+ std::get<1>(x)) {
return std::make_tuple(
- data_writer.writer->max_write_time_messages(),
- data_writer.writer->max_write_time());
+ data_writer.writer->WriteStatistics()
+ ->max_write_time_messages(),
+ data_writer.writer->WriteStatistics()->max_write_time());
}
return x;
}));
@@ -428,25 +433,26 @@
return accumulate_data_writers(
total_write_time_,
[](std::chrono::nanoseconds x, const NewDataWriter &data_writer) {
- return x + data_writer.writer->total_write_time();
+ return x + data_writer.writer->WriteStatistics()->total_write_time();
});
}
int total_write_count() const {
return accumulate_data_writers(
total_write_count_, [](int x, const NewDataWriter &data_writer) {
- return x + data_writer.writer->total_write_count();
+ return x + data_writer.writer->WriteStatistics()->total_write_count();
});
}
int total_write_messages() const {
return accumulate_data_writers(
total_write_messages_, [](int x, const NewDataWriter &data_writer) {
- return x + data_writer.writer->total_write_messages();
+ return x +
+ data_writer.writer->WriteStatistics()->total_write_messages();
});
}
int total_write_bytes() const {
return accumulate_data_writers(
total_write_bytes_, [](int x, const NewDataWriter &data_writer) {
- return x + data_writer.writer->total_write_bytes();
+ return x + data_writer.writer->WriteStatistics()->total_write_bytes();
});
}
@@ -466,8 +472,6 @@
void CreateBufferWriter(std::string_view path, size_t max_message_size,
std::unique_ptr<DetachedBufferWriter> *destination);
- void RenameTempFile(DetachedBufferWriter *destination);
-
void CloseWriter(std::unique_ptr<DetachedBufferWriter> *writer_pointer);
// A version of std::accumulate which operates over all of our DataWriters.
@@ -484,13 +488,11 @@
return t;
}
- std::string base_name_;
- std::string old_base_name_;
+ std::unique_ptr<RenamableFileBackend> log_backend_;
bool ran_out_of_space_ = false;
std::vector<std::string> all_filenames_;
- std::string temp_suffix_;
std::function<std::unique_ptr<DataEncoder>(size_t)> encoder_factory_;
std::string extension_;
@@ -509,6 +511,21 @@
std::map<const Channel *, NewDataWriter> data_writers_;
};
+// This is specialized log namer that deals with directory centric log events.
+class MultiNodeFilesLogNamer : public MultiNodeLogNamer {
+ public:
+ MultiNodeFilesLogNamer(std::string_view base_name, EventLoop *event_loop)
+ : MultiNodeLogNamer(std::make_unique<RenamableFileBackend>(base_name),
+ event_loop) {}
+
+ MultiNodeFilesLogNamer(std::string_view base_name,
+ const Configuration *configuration,
+ EventLoop *event_loop, const Node *node)
+ : MultiNodeLogNamer(std::make_unique<RenamableFileBackend>(base_name),
+ configuration, event_loop, node) {}
+ ~MultiNodeFilesLogNamer() override = default;
+};
+
} // namespace logger
} // namespace aos
diff --git a/aos/events/logging/log_writer.cc b/aos/events/logging/log_writer.cc
index 84f7503..1a333e7 100644
--- a/aos/events/logging/log_writer.cc
+++ b/aos/events/logging/log_writer.cc
@@ -236,44 +236,6 @@
if (new_base_name == CHECK_NOTNULL(log_namer_)->base_name()) {
return true;
}
- std::string current_directory = std::string(log_namer_->base_name());
- std::string new_directory = new_base_name;
-
- auto current_path_split = current_directory.rfind("/");
- CHECK(current_path_split != std::string::npos)
- << "Could not find / in the current directory path";
- auto new_path_split = new_directory.rfind("/");
- CHECK(new_path_split != std::string::npos)
- << "Could not find / in the new directory path";
-
- CHECK(new_base_name.substr(new_path_split) ==
- current_directory.substr(current_path_split))
- << "Rename of file base from " << current_directory << " to "
- << new_directory << " is not supported.";
-
- current_directory.resize(current_path_split);
- new_directory.resize(new_path_split);
- DIR *dir = opendir(current_directory.c_str());
- if (dir) {
- closedir(dir);
- const int result = rename(current_directory.c_str(), new_directory.c_str());
- if (result != 0) {
- PLOG(ERROR) << "Unable to rename " << current_directory << " to "
- << new_directory;
- return false;
- }
- } else {
- // Handle if directory was already renamed.
- dir = opendir(new_directory.c_str());
- if (!dir) {
- LOG(ERROR) << "Old directory " << current_directory
- << " missing and new directory " << new_directory
- << " not present.";
- return false;
- }
- closedir(dir);
- }
-
log_namer_->set_base_name(new_base_name);
Rotate();
return true;
diff --git a/aos/events/logging/log_writer.h b/aos/events/logging/log_writer.h
index 3e1832e..0063c9b 100644
--- a/aos/events/logging/log_writer.h
+++ b/aos/events/logging/log_writer.h
@@ -156,11 +156,11 @@
// Returns whether a log is currently being written.
bool is_started() const { return static_cast<bool>(log_namer_); }
- // Shortcut to call StartLogging with a MultiNodeLogNamer when event
+ // Shortcut to call StartLogging with a MultiNodeFilesLogNamer when event
// processing starts.
void StartLoggingOnRun(std::string base_name) {
event_loop_->OnRun([this, base_name]() {
- StartLogging(std::make_unique<MultiNodeLogNamer>(
+ StartLogging(std::make_unique<MultiNodeFilesLogNamer>(
base_name, configuration_, event_loop_, node_));
});
}
diff --git a/aos/events/logging/logfile_utils.cc b/aos/events/logging/logfile_utils.cc
index 02a9fdd..0338057 100644
--- a/aos/events/logging/logfile_utils.cc
+++ b/aos/events/logging/logfile_utils.cc
@@ -7,6 +7,7 @@
#include <algorithm>
#include <climits>
+#include <filesystem>
#include "absl/strings/escaping.h"
#include "aos/configuration.h"
@@ -64,10 +65,6 @@
"corrupt message found by MessageReader be silently ignored, "
"providing access to all uncorrupted messages in a logfile.");
-DEFINE_bool(direct, false,
- "If true, write using O_DIRECT and write 512 byte aligned blocks "
- "whenever possible.");
-
namespace aos::logger {
namespace {
@@ -83,51 +80,14 @@
}
} // namespace
-DetachedBufferWriter::DetachedBufferWriter(std::string_view filename,
- std::unique_ptr<DataEncoder> encoder)
- : filename_(filename),
- encoder_(std::move(encoder)),
- supports_odirect_(FLAGS_direct) {
- iovec_.reserve(10);
- if (!util::MkdirPIfSpace(filename, 0777)) {
- ran_out_of_space_ = true;
- } else {
- fd_ = open(filename_.c_str(), O_RDWR | O_CLOEXEC | O_CREAT | O_EXCL, 0774);
- if (fd_ == -1 && errno == ENOSPC) {
- ran_out_of_space_ = true;
- } else {
- PCHECK(fd_ != -1) << ": Failed to open " << this->filename()
- << " for writing";
- VLOG(1) << "Opened " << this->filename() << " for writing";
-
- flags_ = fcntl(fd_, F_GETFL, 0);
- PCHECK(flags_ >= 0) << ": Failed to get flags for " << this->filename();
-
- EnableDirect();
- }
- }
-}
-
-void DetachedBufferWriter::EnableDirect() {
- if (supports_odirect_ && !ODirectEnabled()) {
- const int new_flags = flags_ | O_DIRECT;
- // Track if we failed to set O_DIRECT. Note: Austin hasn't seen this call
- // fail. The write call tends to fail instead.
- if (fcntl(fd_, F_SETFL, new_flags) == -1) {
- PLOG(WARNING) << "Failed to set O_DIRECT on " << filename();
- supports_odirect_ = false;
- } else {
- VLOG(1) << "Enabled O_DIRECT on " << filename();
- flags_ = new_flags;
- }
- }
-}
-
-void DetachedBufferWriter::DisableDirect() {
- if (supports_odirect_ && ODirectEnabled()) {
- flags_ = flags_ & (~O_DIRECT);
- PCHECK(fcntl(fd_, F_SETFL, flags_) != -1) << ": Failed to disable O_DIRECT";
- VLOG(1) << "Disabled O_DIRECT on " << filename();
+DetachedBufferWriter::DetachedBufferWriter(
+ std::unique_ptr<FileHandler> file_handler,
+ std::unique_ptr<DataEncoder> encoder)
+ : file_handler_(std::move(file_handler)), encoder_(std::move(encoder)) {
+ CHECK(file_handler_);
+ ran_out_of_space_ = file_handler_->OpenForWrite() == WriteCode::kOutOfSpace;
+ if (ran_out_of_space_) {
+ LOG(WARNING) << "And we are out of space";
}
}
@@ -148,22 +108,10 @@
// (because that data will then be its data).
DetachedBufferWriter &DetachedBufferWriter::operator=(
DetachedBufferWriter &&other) {
- std::swap(filename_, other.filename_);
+ std::swap(file_handler_, other.file_handler_);
std::swap(encoder_, other.encoder_);
- std::swap(fd_, other.fd_);
std::swap(ran_out_of_space_, other.ran_out_of_space_);
std::swap(acknowledge_ran_out_of_space_, other.acknowledge_ran_out_of_space_);
- std::swap(iovec_, other.iovec_);
- std::swap(max_write_time_, other.max_write_time_);
- std::swap(max_write_time_bytes_, other.max_write_time_bytes_);
- std::swap(max_write_time_messages_, other.max_write_time_messages_);
- std::swap(total_write_time_, other.total_write_time_);
- std::swap(total_write_count_, other.total_write_count_);
- std::swap(total_write_messages_, other.total_write_messages_);
- std::swap(total_write_bytes_, other.total_write_bytes_);
- std::swap(last_synced_bytes_, other.last_synced_bytes_);
- std::swap(supports_odirect_, other.supports_odirect_);
- std::swap(flags_, other.flags_);
std::swap(last_flush_time_, other.last_flush_time_);
return *this;
}
@@ -183,7 +131,8 @@
// Keep writing chunks until we've written it all. If we end up with a
// partial write, this means we need to flush to disk.
do {
- const size_t bytes_written = encoder_->Encode(copier, overall_bytes_written);
+ const size_t bytes_written =
+ encoder_->Encode(copier, overall_bytes_written);
CHECK(bytes_written != 0);
overall_bytes_written += bytes_written;
@@ -198,22 +147,14 @@
}
void DetachedBufferWriter::Close() {
- if (fd_ == -1) {
+ if (!file_handler_->is_open()) {
return;
}
encoder_->Finish();
while (encoder_->queue_size() > 0) {
Flush(monotonic_clock::max_time);
}
- if (close(fd_) == -1) {
- if (errno == ENOSPC) {
- ran_out_of_space_ = true;
- } else {
- PLOG(ERROR) << "Closing log file failed";
- }
- }
- fd_ = -1;
- VLOG(1) << "Closed " << filename();
+ ran_out_of_space_ = file_handler_->Close() == WriteCode::kOutOfSpace;
}
void DetachedBufferWriter::Flush(aos::monotonic_clock::time_point now) {
@@ -236,141 +177,9 @@
return;
}
- iovec_.clear();
- size_t iovec_size = std::min<size_t>(queue.size(), IOV_MAX);
- iovec_.resize(iovec_size);
- size_t counted_size = 0;
-
- // Ok, we now need to figure out if we were aligned, and if we were, how much
- // of the data we are being asked to write is aligned.
- //
- // The file is aligned if it is a multiple of kSector in length. The data is
- // aligned if it's memory is kSector aligned, and the length is a multiple of
- // kSector in length.
- bool aligned = (total_write_bytes_ % kSector) == 0;
- size_t write_index = 0;
- for (size_t i = 0; i < iovec_size; ++i) {
- iovec_[write_index].iov_base = const_cast<uint8_t *>(queue[i].data());
-
- // Make sure the address is aligned, or give up. This should be uncommon,
- // but is always possible.
- if ((reinterpret_cast<size_t>(iovec_[write_index].iov_base) &
- (kSector - 1)) != 0) {
- aligned = false;
- }
-
- // Now, see if the length is a multiple of kSector. The goal is to figure
- // out if/how much memory we can write out with O_DIRECT so that only the
- // last little bit is done with non-direct IO to keep it fast.
- iovec_[write_index].iov_len = queue[i].size();
- if ((iovec_[write_index].iov_len % kSector) != 0) {
- VLOG(1) << "Unaligned length on " << filename();
- // If we've got over a sector of data to write, write it out with O_DIRECT
- // and then continue writing the rest unaligned.
- if (aligned && iovec_[write_index].iov_len > kSector) {
- const size_t aligned_size =
- iovec_[write_index].iov_len & (~(kSector - 1));
- VLOG(1) << "Was aligned, writing last chunk rounded from "
- << queue[i].size() << " to " << aligned_size;
- iovec_[write_index].iov_len = aligned_size;
-
- WriteV(iovec_.data(), i + 1, true, counted_size + aligned_size);
-
- // Now, everything before here has been written. Make an iovec out of
- // the last bytes, and keep going.
- iovec_size -= write_index;
- iovec_.resize(iovec_size);
- write_index = 0;
- counted_size = 0;
-
- iovec_[write_index].iov_base =
- const_cast<uint8_t *>(queue[i].data() + aligned_size);
- iovec_[write_index].iov_len = queue[i].size() - aligned_size;
- }
- aligned = false;
- }
- VLOG(1) << "Writing " << iovec_[write_index].iov_len << " to "
- << filename();
- counted_size += iovec_[write_index].iov_len;
- ++write_index;
- }
-
- // Either write the aligned data if it is all aligned, or write the rest
- // unaligned if we wrote aligned up above.
- WriteV(iovec_.data(), iovec_.size(), aligned, counted_size);
-
- encoder_->Clear(iovec_size);
-}
-
-size_t DetachedBufferWriter::WriteV(struct iovec *iovec_data, size_t iovec_size,
- bool aligned, size_t counted_size) {
- // Configure the file descriptor to match the mode we should be in. This is
- // safe to over-call since it only does the syscall if needed.
- if (aligned) {
- EnableDirect();
- } else {
- DisableDirect();
- }
-
- const auto start = aos::monotonic_clock::now();
- const ssize_t written = writev(fd_, iovec_data, iovec_size);
-
- if (written > 0) {
- // Flush asynchronously and force the data out of the cache.
- sync_file_range(fd_, total_write_bytes_, written, SYNC_FILE_RANGE_WRITE);
- if (last_synced_bytes_ != 0) {
- // Per Linus' recommendation online on how to do fast file IO, do a
- // blocking flush of the previous write chunk, and then tell the kernel to
- // drop the pages from the cache. This makes sure we can't get too far
- // ahead.
- sync_file_range(fd_, last_synced_bytes_,
- total_write_bytes_ - last_synced_bytes_,
- SYNC_FILE_RANGE_WAIT_BEFORE | SYNC_FILE_RANGE_WRITE |
- SYNC_FILE_RANGE_WAIT_AFTER);
- posix_fadvise(fd_, last_synced_bytes_,
- total_write_bytes_ - last_synced_bytes_,
- POSIX_FADV_DONTNEED);
-
- last_synced_bytes_ = total_write_bytes_;
- }
- }
-
- const auto end = aos::monotonic_clock::now();
- HandleWriteReturn(written, counted_size);
-
- UpdateStatsForWrite(end - start, written, iovec_size);
-
- return written;
-}
-
-void DetachedBufferWriter::HandleWriteReturn(ssize_t write_return,
- size_t write_size) {
- if (write_return == -1 && errno == ENOSPC) {
- ran_out_of_space_ = true;
- return;
- }
- PCHECK(write_return >= 0) << ": write failed, got " << write_return;
- if (write_return < static_cast<ssize_t>(write_size)) {
- // Sometimes this happens instead of ENOSPC. On a real filesystem, this
- // never seems to happen in any other case. If we ever want to log to a
- // socket, this will happen more often. However, until we get there, we'll
- // just assume it means we ran out of space.
- ran_out_of_space_ = true;
- return;
- }
-}
-
-void DetachedBufferWriter::UpdateStatsForWrite(
- aos::monotonic_clock::duration duration, ssize_t written, int iovec_size) {
- if (duration > max_write_time_) {
- max_write_time_ = duration;
- max_write_time_bytes_ = written;
- max_write_time_messages_ = iovec_size;
- }
- total_write_time_ += duration;
- ++total_write_count_;
- total_write_messages_ += iovec_size;
- total_write_bytes_ += written;
+ const WriteResult result = file_handler_->Write(queue);
+ encoder_->Clear(result.messages_written);
+ ran_out_of_space_ = result.code == WriteCode::kOutOfSpace;
}
void DetachedBufferWriter::FlushAtThreshold(
diff --git a/aos/events/logging/logfile_utils.h b/aos/events/logging/logfile_utils.h
index 50f6b40..c75c8fb 100644
--- a/aos/events/logging/logfile_utils.h
+++ b/aos/events/logging/logfile_utils.h
@@ -20,6 +20,7 @@
#include "aos/events/event_loop.h"
#include "aos/events/logging/boot_timestamp.h"
#include "aos/events/logging/buffer_encoder.h"
+#include "aos/events/logging/log_backend.h"
#include "aos/events/logging/logfile_sorting.h"
#include "aos/events/logging/logger_generated.h"
#include "aos/flatbuffers.h"
@@ -44,31 +45,13 @@
// This class manages efficiently writing a sequence of detached buffers to a
// file. It encodes them, queues them up, and batches the write operation.
-//
-// There are a couple over-arching constraints on writing to keep track of.
-// 1) The kernel is both faster and more efficient at writing large, aligned
-// chunks with O_DIRECT set on the file. The alignment needed is specified
-// by kSector and is file system dependent.
-// 2) Not all encoders support generating round multiples of kSector of data.
-// Rather than burden the API for detecting when that is the case, we want
-// DetachedBufferWriter to be as efficient as it can at writing what given.
-// 3) Some files are small and not updated frequently. They need to be
-// flushed or we will lose data on power off. It is most efficient to write
-// as much as we can aligned by kSector and then fall back to the non direct
-// method when it has been flushed.
-// 4) Not all filesystems support O_DIRECT, and different sizes may be optimal
-// for different machines. The defaults should work decently anywhere and
-// be tuneable for faster systems.
+
class DetachedBufferWriter {
public:
// Marker struct for one of our constructor overloads.
struct already_out_of_space_t {};
- // Size of an aligned sector used to detect when the data is aligned enough to
- // use O_DIRECT instead.
- static constexpr size_t kSector = 512u;
-
- DetachedBufferWriter(std::string_view filename,
+ DetachedBufferWriter(std::unique_ptr<FileHandler> file_handler,
std::unique_ptr<DataEncoder> encoder);
// Creates a dummy instance which won't even open a file. It will act as if
// opening the file ran out of space immediately.
@@ -81,11 +64,11 @@
DetachedBufferWriter &operator=(DetachedBufferWriter &&other);
DetachedBufferWriter &operator=(const DetachedBufferWriter &) = delete;
- std::string_view filename() const { return filename_; }
+ std::string_view filename() const { return file_handler_->filename(); }
// This will be true until Close() is called, unless the file couldn't be
// created due to running out of space.
- bool is_open() const { return fd_ != -1; }
+ bool is_open() const { return file_handler_->is_open(); }
// Queues up a finished FlatBufferBuilder to be encoded and written.
//
@@ -123,33 +106,7 @@
return encoder_->total_bytes();
}
- // The maximum time for a single write call, or 0 if none have been performed.
- std::chrono::nanoseconds max_write_time() const { return max_write_time_; }
- // The number of bytes in the longest write call, or -1 if none have been
- // performed.
- int max_write_time_bytes() const { return max_write_time_bytes_; }
- // The number of buffers in the longest write call, or -1 if none have been
- // performed.
- int max_write_time_messages() const { return max_write_time_messages_; }
- // The total time spent in write calls.
- std::chrono::nanoseconds total_write_time() const {
- return total_write_time_;
- }
- // The total number of writes which have been performed.
- int total_write_count() const { return total_write_count_; }
- // The total number of messages which have been written.
- int total_write_messages() const { return total_write_messages_; }
- // The total number of bytes which have been written.
- int total_write_bytes() const { return total_write_bytes_; }
- void ResetStatistics() {
- max_write_time_ = std::chrono::nanoseconds::zero();
- max_write_time_bytes_ = -1;
- max_write_time_messages_ = -1;
- total_write_time_ = std::chrono::nanoseconds::zero();
- total_write_count_ = 0;
- total_write_messages_ = 0;
- total_write_bytes_ = 0;
- }
+ WriteStats* WriteStatistics() const { return file_handler_->WriteStatistics(); }
private:
// Performs a single writev call with as much of the data we have queued up as
@@ -161,61 +118,33 @@
// all of it.
void Flush(aos::monotonic_clock::time_point now);
- // write_return is what write(2) or writev(2) returned. write_size is the
- // number of bytes we expected it to write.
- void HandleWriteReturn(ssize_t write_return, size_t write_size);
-
- void UpdateStatsForWrite(aos::monotonic_clock::duration duration,
- ssize_t written, int iovec_size);
-
// Flushes data if we've reached the threshold to do that as part of normal
// operation either due to the outstanding queued data, or because we have
// passed our flush period. now is the current time to save some CPU grabbing
// the current time. It just needs to be close.
void FlushAtThreshold(aos::monotonic_clock::time_point now);
- // Enables O_DIRECT on the open file if it is supported. Cheap to call if it
- // is already enabled.
- void EnableDirect();
- // Disables O_DIRECT on the open file if it is supported. Cheap to call if it
- // is already disabld.
- void DisableDirect();
-
- // Writes a chunk of iovecs. aligned is true if all the data is kSector byte
- // aligned and multiples of it in length, and counted_size is the sum of the
- // sizes of all the chunks of data. Returns the size of data written.
- size_t WriteV(struct iovec *iovec_data, size_t iovec_size, bool aligned,
- size_t counted_size);
-
- bool ODirectEnabled() { return !!(flags_ & O_DIRECT); }
-
- std::string filename_;
+ std::unique_ptr<FileHandler> file_handler_;
std::unique_ptr<DataEncoder> encoder_;
- int fd_ = -1;
bool ran_out_of_space_ = false;
bool acknowledge_ran_out_of_space_ = false;
- // List of iovecs to use with writev. This is a member variable to avoid
- // churn.
- std::vector<struct iovec> iovec_;
-
- std::chrono::nanoseconds max_write_time_ = std::chrono::nanoseconds::zero();
- int max_write_time_bytes_ = -1;
- int max_write_time_messages_ = -1;
- std::chrono::nanoseconds total_write_time_ = std::chrono::nanoseconds::zero();
- int total_write_count_ = 0;
- int total_write_messages_ = 0;
- int total_write_bytes_ = 0;
- int last_synced_bytes_ = 0;
-
- bool supports_odirect_ = true;
- int flags_ = 0;
-
aos::monotonic_clock::time_point last_flush_time_ =
aos::monotonic_clock::min_time;
};
+// Specialized writer to single file
+class DetachedBufferFileWriter : public FileBackend,
+ public DetachedBufferWriter {
+ public:
+ DetachedBufferFileWriter(std::string_view filename,
+ std::unique_ptr<DataEncoder> encoder)
+ : FileBackend("/"),
+ DetachedBufferWriter(FileBackend::RequestFile(filename),
+ std::move(encoder)) {}
+};
+
// Repacks the provided RemoteMessage into fbb.
flatbuffers::Offset<MessageHeader> PackRemoteMessage(
flatbuffers::FlatBufferBuilder *fbb,
diff --git a/aos/events/logging/logfile_utils_out_of_space_test_runner.cc b/aos/events/logging/logfile_utils_out_of_space_test_runner.cc
index 719eb6e..03bfd0a 100644
--- a/aos/events/logging/logfile_utils_out_of_space_test_runner.cc
+++ b/aos/events/logging/logfile_utils_out_of_space_test_runner.cc
@@ -18,7 +18,7 @@
std::array<uint8_t, 10240> data;
data.fill(0);
- aos::logger::DetachedBufferWriter writer(
+ aos::logger::DetachedBufferFileWriter writer(
FLAGS_tmpfs + "/file",
std::make_unique<aos::logger::DummyEncoder>(data.size()));
for (int i = 0; i < 8; ++i) {
diff --git a/aos/events/logging/logfile_utils_test.cc b/aos/events/logging/logfile_utils_test.cc
index ec84b58..e2dc8aa 100644
--- a/aos/events/logging/logfile_utils_test.cc
+++ b/aos/events/logging/logfile_utils_test.cc
@@ -1,6 +1,7 @@
#include "aos/events/logging/logfile_utils.h"
#include <chrono>
+#include <filesystem>
#include <random>
#include <string>
@@ -29,13 +30,13 @@
// Adapter class to make it easy to test DetachedBufferWriter without adding
// test only boilerplate to DetachedBufferWriter.
-class TestDetachedBufferWriter : public DetachedBufferWriter {
+class TestDetachedBufferWriter : public DetachedBufferFileWriter {
public:
// Pick a max size that is rather conservative.
static constexpr size_t kMaxMessageSize = 128 * 1024;
TestDetachedBufferWriter(std::string_view filename)
- : DetachedBufferWriter(filename,
- std::make_unique<DummyEncoder>(kMaxMessageSize)) {}
+ : DetachedBufferFileWriter(
+ filename, std::make_unique<DummyEncoder>(kMaxMessageSize)) {}
void WriteSizedFlatbuffer(flatbuffers::DetachedBuffer &&buffer) {
QueueSpan(absl::Span<const uint8_t>(buffer.data(), buffer.size()));
}
@@ -154,6 +155,7 @@
writer.QueueSpan(m2.span());
writer.QueueSpan(m3.span());
}
+ ASSERT_TRUE(std::filesystem::exists(logfile0)) << logfile0;
const std::vector<LogFile> parts = SortParts({logfile0});
diff --git a/aos/events/logging/logger_main.cc b/aos/events/logging/logger_main.cc
index f0582dc..0dbfc8d 100644
--- a/aos/events/logging/logger_main.cc
+++ b/aos/events/logging/logger_main.cc
@@ -48,8 +48,7 @@
aos::ShmEventLoop event_loop(&config.message());
- std::unique_ptr<aos::logger::MultiNodeLogNamer> log_namer;
- log_namer = std::make_unique<aos::logger::MultiNodeLogNamer>(
+ auto log_namer = std::make_unique<aos::logger::MultiNodeFilesLogNamer>(
absl::StrCat(aos::logging::GetLogName("fbs_log"), "/"), &event_loop);
if (FLAGS_snappy_compress) {
diff --git a/aos/events/logging/logger_test.cc b/aos/events/logging/logger_test.cc
index 80b3a4a..fcbd77b 100644
--- a/aos/events/logging/logger_test.cc
+++ b/aos/events/logging/logger_test.cc
@@ -1,5 +1,7 @@
#include <sys/stat.h>
+#include <filesystem>
+
#include "absl/strings/str_format.h"
#include "aos/events/event_loop.h"
#include "aos/events/logging/log_reader.h"
@@ -88,6 +90,8 @@
event_loop_factory_.RunFor(chrono::milliseconds(20000));
}
+ ASSERT_TRUE(std::filesystem::exists(logfile));
+
// Even though it doesn't make any difference here, exercise the logic for
// passing in a separate config.
LogReader reader(logfile, &config_.message());
@@ -152,16 +156,16 @@
Logger logger(logger_event_loop.get());
logger.set_polling_period(std::chrono::milliseconds(100));
- logger_event_loop->OnRun(
- [base_name1, base_name2, &logger_event_loop, &logger]() {
- logger.StartLogging(std::make_unique<MultiNodeLogNamer>(
- base_name1, logger_event_loop->configuration(),
- logger_event_loop.get(), logger_event_loop->node()));
- EXPECT_DEATH(logger.StartLogging(std::make_unique<MultiNodeLogNamer>(
- base_name2, logger_event_loop->configuration(),
- logger_event_loop.get(), logger_event_loop->node())),
- "Already logging");
- });
+ logger_event_loop->OnRun([base_name1, base_name2, &logger_event_loop,
+ &logger]() {
+ logger.StartLogging(std::make_unique<MultiNodeFilesLogNamer>(
+ base_name1, logger_event_loop->configuration(),
+ logger_event_loop.get(), logger_event_loop->node()));
+ EXPECT_DEATH(logger.StartLogging(std::make_unique<MultiNodeFilesLogNamer>(
+ base_name2, logger_event_loop->configuration(),
+ logger_event_loop.get(), logger_event_loop->node())),
+ "Already logging");
+ });
event_loop_factory_.RunFor(chrono::milliseconds(20000));
}
}
@@ -229,7 +233,7 @@
logger.set_separate_config(false);
logger.set_polling_period(std::chrono::milliseconds(100));
logger_event_loop->OnRun([base_name, &logger_event_loop, &logger]() {
- logger.StartLogging(std::make_unique<MultiNodeLogNamer>(
+ logger.StartLogging(std::make_unique<MultiNodeFilesLogNamer>(
base_name, logger_event_loop->configuration(),
logger_event_loop.get(), logger_event_loop->node()));
logger.StopLogging(aos::monotonic_clock::min_time);
@@ -267,13 +271,13 @@
Logger logger(logger_event_loop.get());
logger.set_separate_config(false);
logger.set_polling_period(std::chrono::milliseconds(100));
- logger.StartLogging(std::make_unique<MultiNodeLogNamer>(
+ logger.StartLogging(std::make_unique<MultiNodeFilesLogNamer>(
base_name1, logger_event_loop->configuration(), logger_event_loop.get(),
logger_event_loop->node()));
event_loop_factory_.RunFor(chrono::milliseconds(10000));
logger.StopLogging(logger_event_loop->monotonic_now());
event_loop_factory_.RunFor(chrono::milliseconds(10000));
- logger.StartLogging(std::make_unique<MultiNodeLogNamer>(
+ logger.StartLogging(std::make_unique<MultiNodeFilesLogNamer>(
base_name2, logger_event_loop->configuration(), logger_event_loop.get(),
logger_event_loop->node()));
event_loop_factory_.RunFor(chrono::milliseconds(10000));
diff --git a/aos/events/logging/multinode_logger_test_lib.cc b/aos/events/logging/multinode_logger_test_lib.cc
index 0f588c6..3ffb091 100644
--- a/aos/events/logging/multinode_logger_test_lib.cc
+++ b/aos/events/logging/multinode_logger_test_lib.cc
@@ -41,8 +41,8 @@
logger->set_logger_version(
absl::StrCat("logger_version_", event_loop->node()->name()->str()));
event_loop->OnRun([this, logfile_base]() {
- std::unique_ptr<MultiNodeLogNamer> namer =
- std::make_unique<MultiNodeLogNamer>(logfile_base, configuration,
+ std::unique_ptr<MultiNodeFilesLogNamer> namer =
+ std::make_unique<MultiNodeFilesLogNamer>(logfile_base, configuration,
event_loop.get(), node);
namer->set_extension(params.extension);
namer->set_encoder_factory(params.encoder_factory);
diff --git a/aos/events/logging/realtime_replay_test.cc b/aos/events/logging/realtime_replay_test.cc
index 2a5703a..2d02766 100644
--- a/aos/events/logging/realtime_replay_test.cc
+++ b/aos/events/logging/realtime_replay_test.cc
@@ -191,8 +191,8 @@
logger.set_separate_config(false);
logger.set_polling_period(std::chrono::milliseconds(100));
- std::unique_ptr<MultiNodeLogNamer> namer =
- std::make_unique<MultiNodeLogNamer>(
+ std::unique_ptr<MultiNodeFilesLogNamer> namer =
+ std::make_unique<MultiNodeFilesLogNamer>(
base_name_, &config_.message(), logger_event_loop.get(),
configuration::GetNode(&config_.message(), "pi1"));
@@ -236,8 +236,8 @@
logger.set_separate_config(false);
logger.set_polling_period(std::chrono::milliseconds(100));
- std::unique_ptr<MultiNodeLogNamer> namer =
- std::make_unique<MultiNodeLogNamer>(
+ std::unique_ptr<MultiNodeFilesLogNamer> namer =
+ std::make_unique<MultiNodeFilesLogNamer>(
base_name_, &config_.message(), logger_event_loop.get(),
configuration::GetNode(&config_.message(), "pi1"));
@@ -287,8 +287,8 @@
Logger logger(logger_event_loop.get());
logger.set_separate_config(false);
logger.set_polling_period(std::chrono::milliseconds(100));
- std::unique_ptr<MultiNodeLogNamer> namer =
- std::make_unique<MultiNodeLogNamer>(
+ std::unique_ptr<MultiNodeFilesLogNamer> namer =
+ std::make_unique<MultiNodeFilesLogNamer>(
base_name_, &config_.message(), logger_event_loop.get(),
configuration::GetNode(&config_.message(), "pi1"));
@@ -348,8 +348,8 @@
logger.set_separate_config(false);
logger.set_polling_period(std::chrono::milliseconds(100));
- std::unique_ptr<MultiNodeLogNamer> namer =
- std::make_unique<MultiNodeLogNamer>(
+ std::unique_ptr<MultiNodeFilesLogNamer> namer =
+ std::make_unique<MultiNodeFilesLogNamer>(
base_name_, &config_.message(), logger_event_loop.get(),
configuration::GetNode(&config_.message(), "pi1"));
diff --git a/y2020/control_loops/drivetrain/drivetrain_replay.cc b/y2020/control_loops/drivetrain/drivetrain_replay.cc
index d2f2880..57feabb 100644
--- a/y2020/control_loops/drivetrain/drivetrain_replay.cc
+++ b/y2020/control_loops/drivetrain/drivetrain_replay.cc
@@ -31,7 +31,7 @@
LoggerState(aos::logger::LogReader *reader, const aos::Node *node)
: event_loop_(
reader->event_loop_factory()->MakeEventLoop("logger", node)),
- namer_(std::make_unique<aos::logger::MultiNodeLogNamer>(
+ namer_(std::make_unique<aos::logger::MultiNodeFilesLogNamer>(
absl::StrCat(FLAGS_output_folder, "/", node->name()->string_view(),
"/"),
event_loop_.get())),
diff --git a/y2022/localizer/localizer_replay.cc b/y2022/localizer/localizer_replay.cc
index 08479eb..0c09535 100644
--- a/y2022/localizer/localizer_replay.cc
+++ b/y2022/localizer/localizer_replay.cc
@@ -21,7 +21,7 @@
LoggerState(aos::logger::LogReader *reader, const aos::Node *node)
: event_loop_(
reader->event_loop_factory()->MakeEventLoop("logger", node)),
- namer_(std::make_unique<aos::logger::MultiNodeLogNamer>(
+ namer_(std::make_unique<aos::logger::MultiNodeFilesLogNamer>(
absl::StrCat(FLAGS_output_folder, "/", node->name()->string_view(),
"/"),
event_loop_.get())),
diff --git a/y2023/localizer/localizer_replay.cc b/y2023/localizer/localizer_replay.cc
index 7176f4e..c74b708 100644
--- a/y2023/localizer/localizer_replay.cc
+++ b/y2023/localizer/localizer_replay.cc
@@ -20,7 +20,7 @@
LoggerState(aos::logger::LogReader *reader, const aos::Node *node)
: event_loop_(
reader->event_loop_factory()->MakeEventLoop("logger", node)),
- namer_(std::make_unique<aos::logger::MultiNodeLogNamer>(
+ namer_(std::make_unique<aos::logger::MultiNodeFilesLogNamer>(
absl::StrCat(FLAGS_output_folder, "/", node->name()->string_view(),
"/"),
event_loop_.get())),
diff --git a/y2023/vision/image_logger.cc b/y2023/vision/image_logger.cc
index d90f9c2..e8b6bb2 100644
--- a/y2023/vision/image_logger.cc
+++ b/y2023/vision/image_logger.cc
@@ -18,9 +18,9 @@
DEFINE_double(disabled_time, 5.0,
"Continue logging if disabled for this amount of time or less");
-std::unique_ptr<aos::logger::MultiNodeLogNamer> MakeLogNamer(
+std::unique_ptr<aos::logger::MultiNodeFilesLogNamer> MakeLogNamer(
aos::EventLoop *event_loop) {
- return std::make_unique<aos::logger::MultiNodeLogNamer>(
+ return std::make_unique<aos::logger::MultiNodeFilesLogNamer>(
absl::StrCat(aos::logging::GetLogName("fbs_log"), "/"), event_loop);
}