Add constructor new to MultiNodeFilesLogNamer
Add a constructor to MultiNodeFilesLogNamer allowing a aos
configuration to be specified. This is necessary when doing a
rerun and saving a log while using MultiNodeFilesLogNamer.
Change-Id: Ia991df8343f739e7b1cb2981bcd036333e6731d9
Signed-off-by: James Kuszmaul <james.kuszmaul@bluerivertech.com>
diff --git a/aos/configuration.cc b/aos/configuration.cc
index 3b50ab4..f2488a5 100644
--- a/aos/configuration.cc
+++ b/aos/configuration.cc
@@ -1163,6 +1163,30 @@
return nullptr;
}
+bool IsNodeFromConfiguration(const Configuration *config, const Node *node) {
+ if (config == nullptr) {
+ return false;
+ }
+
+ // Check if is multinode
+ if (MultiNode(config)) {
+ if (node == nullptr) {
+ return false;
+ }
+ for (const Node *node_from_config : *config->nodes()) {
+ if (node_from_config == node) {
+ return true;
+ }
+ }
+ return false;
+ } else {
+ // nullptr is the node for all single node configurations. Return true so
+ // this function can be used in the same way for single or multinode
+ // configurations.
+ return node == nullptr;
+ }
+}
+
std::string_view NodeName(const Configuration *config, size_t node_index) {
if (!configuration::MultiNode(config)) {
return "(singlenode)";
diff --git a/aos/configuration.h b/aos/configuration.h
index 80c0238..a34b5a8 100644
--- a/aos/configuration.h
+++ b/aos/configuration.h
@@ -127,6 +127,11 @@
const Node *GetMyNode(const Configuration *config);
const Node *GetNodeFromHostname(const Configuration *config,
std::string_view name);
+// Determine if this node exists in this configuration. This function checks to
+// make sure this pointer exists in the configuration. This function is useful
+// for determing if the node is named the same but from a different
+// configuration.
+bool IsNodeFromConfiguration(const Configuration *config, const Node *node);
// Returns a printable name for the node. (singlenode) if we are on a single
// node system, and the name otherwise.
diff --git a/aos/configuration_test.cc b/aos/configuration_test.cc
index 0ce6772..c1f00be 100644
--- a/aos/configuration_test.cc
+++ b/aos/configuration_test.cc
@@ -1262,4 +1262,84 @@
test_channel_type);
}
+// Test fixture for testing IsNodeFromConfiguration.
+// Initializes multiple configurations which share the same node names.
+// Use IsNodeFromConfiguration to check if a node is in a configuration.
+class IsNodeFromConfigurationFixtureTest : public ConfigurationTest {
+ protected:
+ // Use unique_ptr for deferred initialization
+ std::unique_ptr<FlatbufferDetachedBuffer<Configuration>> config1;
+ std::unique_ptr<FlatbufferDetachedBuffer<Configuration>> config2;
+ const Node *node1_config1;
+ const Node *node2_config1;
+ const Node *node1_config2;
+ const Node *node2_config2;
+
+ IsNodeFromConfigurationFixtureTest() {
+ // Initialize configurations here
+ config1 = std::make_unique<FlatbufferDetachedBuffer<Configuration>>(
+ JsonToFlatbuffer(R"config({
+ "nodes": [
+ {"name": "node1"},
+ {"name": "node2"}
+ ]
+ })config",
+ Configuration::MiniReflectTypeTable()));
+
+ config2 = std::make_unique<FlatbufferDetachedBuffer<Configuration>>(
+ JsonToFlatbuffer(R"config({
+ "nodes": [
+ {"name": "node1"},
+ {"name": "node2"}
+ ]
+ })config",
+ Configuration::MiniReflectTypeTable()));
+
+ // Initialize nodes after configuration initialization
+ node1_config1 = config1->message().nodes()->Get(0);
+ node2_config1 = config1->message().nodes()->Get(1);
+ node1_config2 = config2->message().nodes()->Get(0);
+ node2_config2 = config2->message().nodes()->Get(1);
+ }
+
+ void SetUp() override {
+ ConfigurationTest::SetUp();
+ // Any additional setup can go here.
+ }
+};
+
+// Test case when node exists in the configuration.
+TEST_F(IsNodeFromConfigurationFixtureTest, NodeExistsInConfiguration) {
+ EXPECT_TRUE(IsNodeFromConfiguration(&config1->message(), node1_config1));
+ EXPECT_TRUE(IsNodeFromConfiguration(&config1->message(), node2_config1));
+}
+
+// Test case when node does not exist in the configuration.
+TEST_F(IsNodeFromConfigurationFixtureTest, NodeDoesNotExistsInConfiguration) {
+ EXPECT_FALSE(IsNodeFromConfiguration(&config1->message(), node1_config2));
+ EXPECT_FALSE(IsNodeFromConfiguration(&config1->message(), node2_config2));
+}
+
+// Test case for nodes with same names but from different configurations.
+TEST_F(IsNodeFromConfigurationFixtureTest, SameNameDifferentConfiguration) {
+ EXPECT_FALSE(IsNodeFromConfiguration(&config1->message(), node1_config2));
+ EXPECT_FALSE(IsNodeFromConfiguration(&config1->message(), node2_config2));
+ EXPECT_FALSE(IsNodeFromConfiguration(&config2->message(), node1_config1));
+ EXPECT_FALSE(IsNodeFromConfiguration(&config2->message(), node2_config1));
+}
+
+// Test case for null pointers.
+TEST_F(IsNodeFromConfigurationFixtureTest, NullPointers) {
+ EXPECT_FALSE(IsNodeFromConfiguration(nullptr, nullptr));
+ EXPECT_FALSE(IsNodeFromConfiguration(&config1->message(), nullptr));
+ EXPECT_FALSE(IsNodeFromConfiguration(nullptr, node1_config1));
+}
+
+// Tests that SourceNode reasonably handles both single and multi-node configs.
+TEST(IsNodeFromConfigurationTest, SingleNode) {
+ FlatbufferDetachedBuffer<Configuration> config_single_node =
+ ReadConfig(ArtifactPath("aos/testdata/config1.json"));
+ EXPECT_TRUE(IsNodeFromConfiguration(&config_single_node.message(), nullptr));
+}
+
} // namespace aos::configuration::testing
diff --git a/aos/events/logging/log_namer.h b/aos/events/logging/log_namer.h
index d7c97ae..08601f2 100644
--- a/aos/events/logging/log_namer.h
+++ b/aos/events/logging/log_namer.h
@@ -232,6 +232,10 @@
configuration_(configuration),
node_(node),
logger_node_index_(configuration::GetNodeIndex(configuration_, node_)) {
+ // The LogNamer should never need the node from the event loop, only the
+ // node from the logger configuration. Check to ensure that the correct node
+ // was passed to the constructor.
+ CHECK(configuration::IsNodeFromConfiguration(configuration_, node_));
nodes_.emplace_back(node_);
}
virtual ~LogNamer() = default;
@@ -293,9 +297,10 @@
realtime_clock::time_point realtime_start_time,
monotonic_clock::time_point logger_monotonic_start_time,
realtime_clock::time_point logger_realtime_start_time) {
- VLOG(1) << "Setting node " << node_index << " to start time "
- << monotonic_start_time << " rt " << realtime_start_time << " UUID "
- << boot_uuid;
+ VLOG(1) << "Setting node_index " << node_index << ", node_name: "
+ << configuration_->nodes()->Get(node_index)->name()->string_view()
+ << " to start time " << monotonic_start_time << " rt "
+ << realtime_start_time << " UUID " << boot_uuid;
NodeState *node_state = GetNodeState(node_index, boot_uuid);
node_state->monotonic_start_time = monotonic_start_time;
node_state->realtime_start_time = realtime_start_time;
@@ -622,6 +627,13 @@
std::unique_ptr<RenamableFileBackend> backend)
: MultiNodeLogNamer(std::move(backend), event_loop) {}
+ MultiNodeFilesLogNamer(EventLoop *event_loop,
+ const Configuration *configuration,
+ std::unique_ptr<RenamableFileBackend> backend,
+ const Node *node)
+ : MultiNodeLogNamer(std::move(backend), configuration, event_loop, node) {
+ }
+
~MultiNodeFilesLogNamer() override = default;
std::string_view base_name() const {
diff --git a/aos/events/logging/log_writer.cc b/aos/events/logging/log_writer.cc
index ae43851..c00d216 100644
--- a/aos/events/logging/log_writer.cc
+++ b/aos/events/logging/log_writer.cc
@@ -498,6 +498,9 @@
last_synchronized_time_ = monotonic_start_time;
for (const Node *node : log_namer_->nodes()) {
+ // Verify the log_namer is using the nodes from configuration_.
+ CHECK(configuration::IsNodeFromConfiguration(configuration_, node));
+
const int node_index = configuration::GetNodeIndex(configuration_, node);
MaybeUpdateTimestamp(node, node_index, monotonic_start_time,
realtime_start_time);