aos/logging: add rename of current log folder
This lets us move the location of an active log if new information
arrives which necessitates updating the folder name.
Change-Id: I457083ec4b45599d6e45a0ac42b0ddcac3438bed
diff --git a/aos/events/logging/log_namer.cc b/aos/events/logging/log_namer.cc
index 2067bdc..6cf91db 100644
--- a/aos/events/logging/log_namer.cc
+++ b/aos/events/logging/log_namer.cc
@@ -87,7 +87,10 @@
MultiNodeLogNamer::MultiNodeLogNamer(std::string_view base_name,
const Configuration *configuration,
const Node *node)
- : LogNamer(node), base_name_(base_name), configuration_(configuration) {}
+ : LogNamer(node),
+ base_name_(base_name),
+ old_base_name_(),
+ configuration_(configuration) {}
MultiNodeLogNamer::~MultiNodeLogNamer() {
if (!ran_out_of_space_) {
@@ -376,11 +379,28 @@
if (temp_suffix_.empty()) {
return;
}
- const std::string current_filename = std::string(destination->filename());
+ std::string current_filename = std::string(destination->filename());
CHECK(current_filename.size() > temp_suffix_.size());
- const std::string final_filename =
+ std::string final_filename =
current_filename.substr(0, current_filename.size() - temp_suffix_.size());
- const int result = rename(current_filename.c_str(), final_filename.c_str());
+ 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;
@@ -389,6 +409,8 @@
PLOG(FATAL) << "Renaming " << current_filename << " to " << final_filename
<< " failed";
}
+ } else {
+ VLOG(1) << "Renamed " << current_filename << " -> " << final_filename;
}
}
diff --git a/aos/events/logging/log_namer.h b/aos/events/logging/log_namer.h
index 8f3f712..3b2c83c 100644
--- a/aos/events/logging/log_namer.h
+++ b/aos/events/logging/log_namer.h
@@ -23,6 +23,15 @@
LogNamer(const Node *node) : node_(node) { nodes_.emplace_back(node_); }
virtual ~LogNamer() {}
+ virtual std::string_view base_name() const = 0;
+
+ // Rotate should be called at least once in between calls to set_base_name.
+ // Otherwise temporary files will not be recoverable.
+ // 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.
+ virtual void set_base_name(std::string_view base_name) = 0;
+
// Writes the header to all log files for a specific node. This function
// needs to be called after all the writers are created.
//
@@ -101,6 +110,12 @@
data_writer_(OpenDataWriter()) {}
~LocalLogNamer() override = default;
+ std::string_view base_name() const final { return base_name_; }
+
+ void set_base_name(std::string_view base_name) final {
+ base_name_ = base_name;
+ }
+
void WriteHeader(
aos::SizePrefixedFlatbufferDetachedBuffer<LogFileHeader> *header,
const Node *node) override;
@@ -132,7 +147,7 @@
std::make_unique<aos::logger::DummyEncoder>());
}
- const std::string base_name_;
+ std::string base_name_;
const UUID uuid_;
size_t part_number_ = 0;
std::unique_ptr<DetachedBufferWriter> data_writer_;
@@ -145,7 +160,12 @@
const Configuration *configuration, const Node *node);
~MultiNodeLogNamer() override;
- std::string_view base_name() const { return base_name_; }
+ std::string_view base_name() const final { return base_name_; }
+
+ void set_base_name(std::string_view base_name) final {
+ old_base_name_ = base_name_;
+ base_name_ = 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
@@ -344,7 +364,8 @@
return t;
}
- const std::string base_name_;
+ std::string base_name_;
+ std::string old_base_name_;
const Configuration *const configuration_;
bool ran_out_of_space_ = false;
diff --git a/aos/events/logging/log_writer.cc b/aos/events/logging/log_writer.cc
index 6f99c7b..4016a65 100644
--- a/aos/events/logging/log_writer.cc
+++ b/aos/events/logging/log_writer.cc
@@ -1,5 +1,7 @@
#include "aos/events/logging/log_writer.h"
+#include <dirent.h>
+
#include <functional>
#include <map>
#include <vector>
@@ -168,6 +170,49 @@
}
}
+bool Logger::RenameLogBase(std::string new_base_name) {
+ if (new_base_name == 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("/");
+ auto new_path_split = new_directory.rfind("/");
+
+ 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;
+}
+
void Logger::StartLogging(std::unique_ptr<LogNamer> log_namer,
std::optional<UUID> log_start_uuid) {
CHECK(!log_namer_) << ": Already logging";
diff --git a/aos/events/logging/log_writer.h b/aos/events/logging/log_writer.h
index c546286..df13ec6 100644
--- a/aos/events/logging/log_writer.h
+++ b/aos/events/logging/log_writer.h
@@ -128,6 +128,11 @@
void StartLogging(std::unique_ptr<LogNamer> log_namer,
std::optional<UUID> log_start_uuid = std::nullopt);
+ // 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 new_base_name);
+
// Stops logging. Ensures any messages through end_time make it into the log.
//
// If you want to stop ASAP, pass min_time to avoid reading any more messages.
diff --git a/aos/events/logging/logger_test.cc b/aos/events/logging/logger_test.cc
index f60a62a..a65048c 100644
--- a/aos/events/logging/logger_test.cc
+++ b/aos/events/logging/logger_test.cc
@@ -1,5 +1,7 @@
#include "aos/events/logging/log_reader.h"
+#include <sys/stat.h>
+
#include "absl/strings/str_format.h"
#include "aos/events/event_loop.h"
#include "aos/events/logging/log_writer.h"
@@ -2077,6 +2079,48 @@
}
}
+// Test that renaming the base, renames the folder.
+TEST_F(MultinodeLoggerTest, LoggerRenameFolder) {
+ time_converter_.AddMonotonic(
+ {monotonic_clock::epoch(),
+ monotonic_clock::epoch() + chrono::seconds(1000)});
+ logfile_base1_ = tmp_dir_ + "/renamefolder/multi_logfile1";
+ logfile_base2_ = tmp_dir_ + "/renamefolder/multi_logfile2";
+ logfiles_ = MakeLogFiles(logfile_base1_, logfile_base2_);
+ LoggerState pi1_logger = MakeLogger(pi1_);
+ LoggerState pi2_logger = MakeLogger(pi2_);
+
+ StartLogger(&pi1_logger);
+ StartLogger(&pi2_logger);
+
+ event_loop_factory_.RunFor(chrono::milliseconds(10000));
+ logfile_base1_ = tmp_dir_ + "/new-good/multi_logfile1";
+ logfile_base2_ = tmp_dir_ + "/new-good/multi_logfile2";
+ logfiles_ = MakeLogFiles(logfile_base1_, logfile_base2_);
+ ASSERT_TRUE(pi1_logger.logger->RenameLogBase(logfile_base1_));
+ ASSERT_TRUE(pi2_logger.logger->RenameLogBase(logfile_base2_));
+ for (auto &file : logfiles_) {
+ struct stat s;
+ EXPECT_EQ(0, stat(file.c_str(), &s));
+ }
+}
+
+// Test that renaming the file base dies.
+TEST_P(MultinodeLoggerDeathTest, LoggerRenameFile) {
+ time_converter_.AddMonotonic(
+ {monotonic_clock::epoch(),
+ monotonic_clock::epoch() + chrono::seconds(1000)});
+ logfile_base1_ = tmp_dir_ + "/renamefile/multi_logfile1";
+ logfile_base2_ = tmp_dir_ + "/renamefile/multi_logfile2";
+ logfiles_ = MakeLogFiles(logfile_base1_, logfile_base2_);
+ LoggerState pi1_logger = MakeLogger(pi1_);
+ StartLogger(&pi1_logger);
+ event_loop_factory_.RunFor(chrono::milliseconds(10000));
+ logfile_base1_ = tmp_dir_ + "/new-renamefile/new_multi_logfile1";
+ EXPECT_DEATH({ pi1_logger.logger->RenameLogBase(logfile_base1_); },
+ "Rename of file base from");
+}
+
// TODO(austin): We can write a test which recreates a logfile and confirms that
// we get it back. That is the ultimate test.