Push renaming of the base away from general logger
It is moved to specific instance of log backend. Not all backends
support renaming.
Change-Id: I2176a283b5dcb1101f69428b9072ffaab7c4d8f9
Signed-off-by: Austin Schuh <austin.schuh@bluerivertech.com>
diff --git a/aos/events/logging/log_backend.cc b/aos/events/logging/log_backend.cc
index 9b3f090..2700c4d 100644
--- a/aos/events/logging/log_backend.cc
+++ b/aos/events/logging/log_backend.cc
@@ -188,7 +188,7 @@
VLOG(1) << "Started " << (was_aligned ? "aligned" : "unaligned")
<< " at offset " << total_write_bytes_ << " on " << filename();
- // Walk through aligned queue and batch writes basel on aligned flag
+ // Walk through aligned queue and batch writes based on aligned flag
for (const auto &item : queue_aligner_.aligned_queue()) {
if (was_aligned != item.aligned) {
// Switching aligned context. Let's flush current batch.
diff --git a/aos/events/logging/log_backend.h b/aos/events/logging/log_backend.h
index 469c120..b3cea6c 100644
--- a/aos/events/logging/log_backend.h
+++ b/aos/events/logging/log_backend.h
@@ -243,7 +243,7 @@
: FileHandler(std::move(filename)), owner_(owner) {}
~RenamableFileHandler() final = default;
- // Returns false if not enough memory, true otherwise.
+ // Closes and if needed renames file.
WriteCode Close() final;
private:
@@ -258,7 +258,7 @@
// 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_; }
+ std::string_view base_name() const { 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
diff --git a/aos/events/logging/log_namer.cc b/aos/events/logging/log_namer.cc
index 9ca1274..10e08db 100644
--- a/aos/events/logging/log_namer.cc
+++ b/aos/events/logging/log_namer.cc
@@ -561,12 +561,12 @@
}
MultiNodeLogNamer::MultiNodeLogNamer(
- std::unique_ptr<RenamableFileBackend> log_backend, EventLoop *event_loop)
+ std::unique_ptr<LogBackend> log_backend, EventLoop *event_loop)
: MultiNodeLogNamer(std::move(log_backend), event_loop->configuration(),
event_loop, event_loop->node()) {}
MultiNodeLogNamer::MultiNodeLogNamer(
- std::unique_ptr<RenamableFileBackend> log_backend,
+ std::unique_ptr<LogBackend> log_backend,
const Configuration *configuration, EventLoop *event_loop, const Node *node)
: LogNamer(configuration, event_loop, node),
log_backend_(std::move(log_backend)),
diff --git a/aos/events/logging/log_namer.h b/aos/events/logging/log_namer.h
index 84745b6..fcb3f87 100644
--- a/aos/events/logging/log_namer.h
+++ b/aos/events/logging/log_namer.h
@@ -176,17 +176,6 @@
}
virtual ~LogNamer() = default;
- 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.
- // 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
// primary node).
//
@@ -306,28 +295,13 @@
// Log namer which uses a config to name a bunch of files.
class MultiNodeLogNamer : public LogNamer {
public:
- MultiNodeLogNamer(std::unique_ptr<RenamableFileBackend> log_backend,
+ MultiNodeLogNamer(std::unique_ptr<LogBackend> log_backend,
EventLoop *event_loop);
- MultiNodeLogNamer(std::unique_ptr<RenamableFileBackend> log_backend,
+ MultiNodeLogNamer(std::unique_ptr<LogBackend> log_backend,
const Configuration *configuration, EventLoop *event_loop,
const Node *node);
~MultiNodeLogNamer() override;
- std::string_view base_name() const final { return log_backend_->base_name(); }
-
- void set_base_name(std::string_view base_name) final {
- log_backend_->RenameLogBase(base_name);
- }
-
- // 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 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.
//
@@ -462,6 +436,12 @@
void ResetStatistics();
+ protected:
+ // TODO (Alexei): consider to move ownership of log_namer to concrete sub
+ // class and make log_backend_ raw pointer.
+ LogBackend *log_backend() { return log_backend_.get(); }
+ const LogBackend *log_backend() const { return log_backend_.get(); }
+
private:
// Opens up a writer for timestamps forwarded back.
void OpenForwardedTimestampWriter(const Channel *channel,
@@ -492,7 +472,7 @@
return t;
}
- std::unique_ptr<RenamableFileBackend> log_backend_;
+ std::unique_ptr<LogBackend> log_backend_;
bool ran_out_of_space_ = false;
std::vector<std::string> all_filenames_;
@@ -528,6 +508,36 @@
: MultiNodeLogNamer(std::make_unique<RenamableFileBackend>(base_name),
configuration, event_loop, node) {}
~MultiNodeFilesLogNamer() override = default;
+
+ std::string_view base_name() const {
+ return renamable_file_backend()->base_name();
+ }
+
+ // 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.
+ void set_base_name(std::string_view base_name) {
+ renamable_file_backend()->RenameLogBase(base_name);
+ }
+
+ // 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 EnableTempFiles() { renamable_file_backend()->EnableTempFiles(); }
+
+ private:
+ RenamableFileBackend *renamable_file_backend() {
+ return reinterpret_cast<RenamableFileBackend *>(log_backend());
+ }
+ const RenamableFileBackend *renamable_file_backend() const {
+ return reinterpret_cast<const RenamableFileBackend *>(log_backend());
+ }
};
} // namespace logger
diff --git a/aos/events/logging/log_writer.cc b/aos/events/logging/log_writer.cc
index 3db6646..b6ee4a5 100644
--- a/aos/events/logging/log_writer.cc
+++ b/aos/events/logging/log_writer.cc
@@ -230,16 +230,7 @@
}
}
-bool Logger::RenameLogBase(std::string new_base_name) {
- // TODO(Naman): Got a crash in RenameLogBase. Putting in a CHECK_NOTNULL to
- // catch the bug if it happens again
- if (new_base_name == CHECK_NOTNULL(log_namer_)->base_name()) {
- return true;
- }
- log_namer_->set_base_name(new_base_name);
- Rotate();
- return true;
-}
+
std::string Logger::WriteConfiguration(LogNamer *log_namer) {
std::string config_sha256;
diff --git a/aos/events/logging/log_writer.h b/aos/events/logging/log_writer.h
index d127f1b..65c68f4 100644
--- a/aos/events/logging/log_writer.h
+++ b/aos/events/logging/log_writer.h
@@ -139,11 +139,6 @@
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/multinode_logger_test.cc b/aos/events/logging/multinode_logger_test.cc
index d9d04b2..9e2d3a3 100644
--- a/aos/events/logging/multinode_logger_test.cc
+++ b/aos/events/logging/multinode_logger_test.cc
@@ -1816,8 +1816,16 @@
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_));
+
+ // Sequence of set_base_name and Rotate simulates rename operation. Since
+ // rename is not supported by all namers, RenameLogBase moved from logger to
+ // the higher level abstraction, yet log_namers support rename, and it is
+ // legal to test it here.
+ pi1_logger.log_namer->set_base_name(logfile_base1_);
+ pi1_logger.logger->Rotate();
+ pi2_logger.log_namer->set_base_name(logfile_base2_);
+ pi2_logger.logger->Rotate();
+
for (auto &file : logfiles_) {
struct stat s;
EXPECT_EQ(0, stat(file.c_str(), &s));
@@ -1836,7 +1844,7 @@
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_); },
+ EXPECT_DEATH({ pi1_logger.log_namer->set_base_name(logfile_base1_); },
"Rename of file base from");
}
diff --git a/aos/events/logging/multinode_logger_test_lib.h b/aos/events/logging/multinode_logger_test_lib.h
index 06a4a21..1f04ef6 100644
--- a/aos/events/logging/multinode_logger_test_lib.h
+++ b/aos/events/logging/multinode_logger_test_lib.h
@@ -50,7 +50,7 @@
std::unique_ptr<Logger> logger;
const Configuration *configuration;
const Node *node;
- MultiNodeLogNamer *log_namer;
+ MultiNodeFilesLogNamer *log_namer;
CompressionParams params;
void AppendAllFilenames(std::vector<std::string> *filenames);