Teach logger about O_DIRECT
Plump the option down through aos.
This will let us (in a future change) detect whether the storage disk
should use O_DIRECT or not, as appropriate.
Change-Id: I2bc68606a4954460a3bcd61e5e649e122ebb1358
Signed-off-by: James Kuszmaul <james.kuszmaul@bluerivertech.com>
diff --git a/aos/events/logging/log_backend.cc b/aos/events/logging/log_backend.cc
index 30f6181..9eb9089 100644
--- a/aos/events/logging/log_backend.cc
+++ b/aos/events/logging/log_backend.cc
@@ -11,9 +11,6 @@
#include "aos/events/logging/file_operations.h"
#include "aos/util/file.h"
-DEFINE_bool(direct, false,
- "If true, write using O_DIRECT and write 512 byte aligned blocks "
- "whenever possible.");
DEFINE_bool(
sync, false,
"If true, sync data to disk as we go so we don't get too far ahead. Also "
@@ -123,8 +120,8 @@
}
}
-FileHandler::FileHandler(std::string filename)
- : filename_(std::move(filename)), supports_odirect_(FLAGS_direct) {}
+FileHandler::FileHandler(std::string filename, bool supports_odirect)
+ : filename_(std::move(filename)), supports_odirect_(supports_odirect) {}
FileHandler::~FileHandler() { Close(); }
@@ -332,12 +329,14 @@
return ran_out_of_space ? WriteCode::kOutOfSpace : WriteCode::kOk;
}
-FileBackend::FileBackend(std::string_view base_name)
- : base_name_(base_name), separator_(base_name_.back() == '/' ? "" : "_") {}
+FileBackend::FileBackend(std::string_view base_name, bool supports_odirect)
+ : supports_odirect_(supports_odirect),
+ base_name_(base_name),
+ separator_(base_name_.back() == '/' ? "" : "_") {}
std::unique_ptr<LogSink> FileBackend::RequestFile(std::string_view id) {
const std::string filename = absl::StrCat(base_name_, separator_, id);
- return std::make_unique<FileHandler>(filename);
+ return std::make_unique<FileHandler>(filename, supports_odirect_);
}
std::vector<std::string> FileBackend::ListFiles() const {
@@ -365,14 +364,18 @@
return std::make_unique<DummyDecoder>(filename);
}
-RenamableFileBackend::RenamableFileBackend(std::string_view base_name)
- : base_name_(base_name), separator_(base_name_.back() == '/' ? "" : "_") {}
+RenamableFileBackend::RenamableFileBackend(std::string_view base_name,
+ bool supports_odirect)
+ : supports_odirect_(supports_odirect),
+ base_name_(base_name),
+ separator_(base_name_.back() == '/' ? "" : "_") {}
std::unique_ptr<LogSink> 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);
+ return std::make_unique<RenamableFileHandler>(this, filename,
+ supports_odirect_);
}
void RenamableFileBackend::EnableTempFiles() {
diff --git a/aos/events/logging/log_backend.h b/aos/events/logging/log_backend.h
index 193c54f..e75b632 100644
--- a/aos/events/logging/log_backend.h
+++ b/aos/events/logging/log_backend.h
@@ -167,7 +167,7 @@
// use O_DIRECT instead.
static constexpr size_t kSector = 512u;
- explicit FileHandler(std::string filename);
+ explicit FileHandler(std::string filename, bool supports_odirect);
~FileHandler() override;
FileHandler(const FileHandler &) = delete;
@@ -267,7 +267,7 @@
class FileBackend : public LogBackend, public LogSource {
public:
// base_name is the path to the folder where log files are.
- explicit FileBackend(std::string_view base_name);
+ explicit FileBackend(std::string_view base_name, bool supports_direct);
~FileBackend() override = default;
// Request file from a file system. It is not open yet.
@@ -280,6 +280,7 @@
std::unique_ptr<DataDecoder> GetDecoder(std::string_view id) const override;
private:
+ const bool supports_odirect_;
const std::string base_name_;
const std::string_view separator_;
};
@@ -291,8 +292,9 @@
// 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(RenamableFileBackend *owner, std::string filename,
+ bool supports_odirect)
+ : FileHandler(std::move(filename), supports_odirect), owner_(owner) {}
~RenamableFileHandler() final = default;
// Closes and if needed renames file.
@@ -302,7 +304,8 @@
RenamableFileBackend *owner_;
};
- explicit RenamableFileBackend(std::string_view base_name);
+ explicit RenamableFileBackend(std::string_view base_name,
+ bool supports_odirect);
~RenamableFileBackend() = default;
// Request file from a file system. It is not open yet.
@@ -331,6 +334,7 @@
// base name was changed or temp files enabled.
WriteCode RenameFileAfterClose(std::string_view filename);
+ const bool supports_odirect_;
std::string base_name_;
std::string_view separator_;
diff --git a/aos/events/logging/log_backend_test.cc b/aos/events/logging/log_backend_test.cc
index 2720e46..08b6f1e 100644
--- a/aos/events/logging/log_backend_test.cc
+++ b/aos/events/logging/log_backend_test.cc
@@ -28,7 +28,7 @@
TEST(LogBackendTest, CreateSimpleFile) {
const std::string logevent = aos::testing::TestTmpDir() + "/logevent/";
const std::string filename = "test.bfbs";
- FileBackend backend(logevent);
+ FileBackend backend(logevent, false);
auto file = backend.RequestFile(filename);
ASSERT_EQ(file->OpenForWrite(), WriteCode::kOk);
auto result = Write(file.get(), "test");
@@ -51,7 +51,7 @@
TEST(LogBackendTest, CreateRenamableFile) {
const std::string logevent = aos::testing::TestTmpDir() + "/logevent/";
- RenamableFileBackend backend(logevent);
+ RenamableFileBackend backend(logevent, false);
auto file = backend.RequestFile("test.log");
ASSERT_EQ(file->OpenForWrite(), WriteCode::kOk);
auto result = Write(file.get(), "test");
@@ -63,7 +63,7 @@
TEST(LogBackendTest, UseTempRenamableFile) {
const std::string logevent = aos::testing::TestTmpDir() + "/logevent/";
- RenamableFileBackend backend(logevent);
+ RenamableFileBackend backend(logevent, false);
backend.EnableTempFiles();
auto file = backend.RequestFile("test.log");
ASSERT_EQ(file->OpenForWrite(), WriteCode::kOk);
@@ -79,7 +79,7 @@
TEST(LogBackendTest, RenameBaseAfterWrite) {
const std::string logevent = aos::testing::TestTmpDir() + "/logevent/";
- RenamableFileBackend backend(logevent);
+ RenamableFileBackend backend(logevent, false);
auto file = backend.RequestFile("test.log");
ASSERT_EQ(file->OpenForWrite(), WriteCode::kOk);
auto result = Write(file.get(), "test");
@@ -100,7 +100,7 @@
TEST(LogBackendTest, UseTestAndRenameBaseAfterWrite) {
const std::string logevent = aos::testing::TestTmpDir() + "/logevent/";
- RenamableFileBackend backend(logevent);
+ RenamableFileBackend backend(logevent, false);
backend.EnableTempFiles();
auto file = backend.RequestFile("test.log");
ASSERT_EQ(file->OpenForWrite(), WriteCode::kOk);
@@ -279,7 +279,7 @@
std::filesystem::remove_all(file);
VLOG(1) << "Writing to " << file.c_str();
- FileBackend backend(logevent);
+ FileBackend backend(logevent, false);
auto handler = backend.RequestFile("test.log");
ASSERT_EQ(handler->OpenForWrite(), WriteCode::kOk);
@@ -385,7 +385,7 @@
std::filesystem::remove_all(file);
VLOG(1) << "Writing to " << file.c_str();
- FileBackend backend(logevent);
+ FileBackend backend(logevent, false);
auto handler = backend.RequestFile("test.log");
ASSERT_EQ(handler->OpenForWrite(), WriteCode::kOk);
diff --git a/aos/events/logging/log_edit.cc b/aos/events/logging/log_edit.cc
index 7d3ba2f..4585ab7 100644
--- a/aos/events/logging/log_edit.cc
+++ b/aos/events/logging/log_edit.cc
@@ -24,6 +24,9 @@
"Max size of a message to be written. This sets the buffers inside "
"the encoders.");
+DEFINE_bool(direct, false,
+ "If true, write using O_DIRECT and write 512 byte aligned blocks "
+ "whenever possible.");
int main(int argc, char **argv) {
gflags::SetUsageMessage(R"(This tool lets us manipulate log files.)");
aos::InitGoogle(&argc, &argv);
@@ -49,7 +52,7 @@
aos::logger::SpanReader span_reader(orig_path);
CHECK(!span_reader.ReadMessage().empty()) << ": Empty header, aborting";
- aos::logger::FileBackend file_backend("/");
+ aos::logger::FileBackend file_backend("/", FLAGS_direct);
aos::logger::DetachedBufferWriter buffer_writer(
file_backend.RequestFile(FLAGS_logfile),
std::make_unique<aos::logger::DummyEncoder>(FLAGS_max_message_size));
diff --git a/aos/events/logging/log_namer.h b/aos/events/logging/log_namer.h
index 89a9319..8c62559 100644
--- a/aos/events/logging/log_namer.h
+++ b/aos/events/logging/log_namer.h
@@ -499,14 +499,21 @@
class MultiNodeFilesLogNamer : public MultiNodeLogNamer {
public:
MultiNodeFilesLogNamer(std::string_view base_name, EventLoop *event_loop)
- : MultiNodeLogNamer(std::make_unique<RenamableFileBackend>(base_name),
- event_loop) {}
+ : MultiNodeLogNamer(
+ std::make_unique<RenamableFileBackend>(base_name, false),
+ 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) {}
+ : MultiNodeLogNamer(
+ std::make_unique<RenamableFileBackend>(base_name, false),
+ configuration, event_loop, node) {}
+
+ MultiNodeFilesLogNamer(EventLoop *event_loop,
+ std::unique_ptr<RenamableFileBackend> backend)
+ : MultiNodeLogNamer(std::move(backend), event_loop) {}
+
~MultiNodeFilesLogNamer() override = default;
std::string_view base_name() const {
diff --git a/aos/events/logging/log_writer.h b/aos/events/logging/log_writer.h
index f63ac3e..114e874 100644
--- a/aos/events/logging/log_writer.h
+++ b/aos/events/logging/log_writer.h
@@ -154,6 +154,7 @@
// Shortcut to call StartLogging with a MultiNodeFilesLogNamer when event
// processing starts.
+ // Doesn't try to use odirect.
void StartLoggingOnRun(std::string base_name) {
event_loop_->OnRun([this, base_name]() {
StartLogging(std::make_unique<MultiNodeFilesLogNamer>(
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 65591ba..f6fb499 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
@@ -19,7 +19,8 @@
std::array<uint8_t, 10240> data;
data.fill(0);
- aos::logger::FileBackend file_backend("/");
+ // Don't use odirect
+ aos::logger::FileBackend file_backend("/", false);
aos::logger::DetachedBufferWriter writer(
file_backend.RequestFile(FLAGS_tmpfs + "/file"),
std::make_unique<aos::logger::DummyEncoder>(data.size()));
diff --git a/aos/events/logging/logfile_utils_test.cc b/aos/events/logging/logfile_utils_test.cc
index cd6da67..f4071e4 100644
--- a/aos/events/logging/logfile_utils_test.cc
+++ b/aos/events/logging/logfile_utils_test.cc
@@ -37,7 +37,7 @@
// Pick a max size that is rather conservative.
static constexpr size_t kMaxMessageSize = 128 * 1024;
TestDetachedBufferWriter(std::string_view filename)
- : FileBackend("/"),
+ : FileBackend("/", false),
DetachedBufferWriter(FileBackend::RequestFile(filename),
std::make_unique<DummyEncoder>(kMaxMessageSize)) {}
void WriteSizedFlatbuffer(flatbuffers::DetachedBuffer &&buffer) {
diff --git a/aos/events/logging/logger_main.cc b/aos/events/logging/logger_main.cc
index 631f41f..afc166f 100644
--- a/aos/events/logging/logger_main.cc
+++ b/aos/events/logging/logger_main.cc
@@ -14,6 +14,10 @@
#include "aos/init.h"
#include "aos/logging/log_namer.h"
+DEFINE_bool(direct, false,
+ "If true, write using O_DIRECT and write 512 byte aligned blocks "
+ "whenever possible.");
+
DEFINE_string(config, "aos_config.json", "Config file to use.");
DEFINE_bool(skip_renicing, false,
@@ -50,7 +54,9 @@
aos::ShmEventLoop event_loop(&config.message());
auto log_namer = std::make_unique<aos::logger::MultiNodeFilesLogNamer>(
- absl::StrCat(aos::logging::GetLogName("fbs_log"), "/"), &event_loop);
+ &event_loop, std::make_unique<aos::logger::RenamableFileBackend>(
+ absl::StrCat(aos::logging::GetLogName("fbs_log"), "/"),
+ FLAGS_direct));
if (FLAGS_snappy_compress) {
log_namer->set_extension(aos::logger::SnappyDecoder::kExtension);
diff --git a/y2023/vision/image_logger.cc b/y2023/vision/image_logger.cc
index c2cfa0a..1b457f9 100644
--- a/y2023/vision/image_logger.cc
+++ b/y2023/vision/image_logger.cc
@@ -18,6 +18,9 @@
DECLARE_int32(flush_size);
DEFINE_double(disabled_time, 5.0,
"Continue logging if disabled for this amount of time or less");
+DEFINE_bool(direct, false,
+ "If true, write using O_DIRECT and write 512 byte aligned blocks "
+ "whenever possible.");
std::unique_ptr<aos::logger::MultiNodeFilesLogNamer> MakeLogNamer(
aos::EventLoop *event_loop) {
@@ -29,7 +32,8 @@
}
return std::make_unique<aos::logger::MultiNodeFilesLogNamer>(
- absl::StrCat(log_name.value(), "/"), event_loop);
+ event_loop, std::make_unique<aos::logger::RenamableFileBackend>(
+ absl::StrCat(log_name.value(), "/"), FLAGS_direct));
}
int main(int argc, char *argv[]) {