Add/clean up some CHECKs in LogReader.
Specifically:
- If we reach a point where we have a multi-node config but
none of the nodes have logs to replay, immediately crash.
- Correctly compare the logged_configuration to the replay
configuration for checking node list length, rather than
comparing the replay configuration to itself.
Change-Id: Ibd2da62cacab45153361cafb746102c42711f0d5
diff --git a/aos/events/logging/logger.cc b/aos/events/logging/logger.cc
index de9d344..b3472d7 100644
--- a/aos/events/logging/logger.cc
+++ b/aos/events/logging/logger.cc
@@ -312,9 +312,16 @@
state->channel_merger = std::make_unique<ChannelMerger>(filenames);
} else {
if (replay_configuration) {
- CHECK_EQ(configuration()->nodes()->size(),
+ CHECK_EQ(logged_configuration()->nodes()->size(),
replay_configuration->nodes()->size())
<< ": Log file and replay config need to have matching nodes lists.";
+ for (const Node *node : *logged_configuration()->nodes()) {
+ if (configuration::GetNode(replay_configuration, node) == nullptr) {
+ LOG(FATAL)
+ << "Found node " << FlatbufferToJson(node)
+ << " in logged config that is not present in the replay config.";
+ }
+ }
}
states_.resize(configuration()->nodes()->size());
}
@@ -387,6 +394,11 @@
Register(state->event_loop_unique_ptr.get());
}
+ if (live_nodes_ == 0) {
+ LOG(FATAL)
+ << "Don't have logs from any of the nodes in the replay config--are "
+ "you sure that the replay config matches the original config?";
+ }
// We need to now seed our per-node time offsets and get everything set up to
// run.
diff --git a/aos/events/logging/logger.h b/aos/events/logging/logger.h
index 34dbd24..ce21598 100644
--- a/aos/events/logging/logger.h
+++ b/aos/events/logging/logger.h
@@ -424,6 +424,8 @@
// Returns the offset from the monotonic clock for a node to the distributed
// clock. distributed = monotonic + offset;
std::chrono::nanoseconds offset(int node_index) const {
+ CHECK_LT(node_index, offset_matrix_.rows())
+ << ": Got too high of a node index.";
return -std::chrono::duration_cast<std::chrono::nanoseconds>(
std::chrono::duration<double>(offset_matrix_(node_index))) -
base_offset_matrix_(node_index);
diff --git a/aos/events/logging/logger_test.cc b/aos/events/logging/logger_test.cc
index 9e69ae4..b894bf7 100644
--- a/aos/events/logging/logger_test.cc
+++ b/aos/events/logging/logger_test.cc
@@ -505,6 +505,77 @@
reader.Deregister();
}
+typedef MultinodeLoggerTest MultinodeLoggerDeathTest;
+
+// Test that if we feed the replay with a mismatched node list that we die on
+// the LogReader constructor.
+TEST_F(MultinodeLoggerDeathTest, MultiNodeBadReplayConfig) {
+ const ::std::string tmpdir(getenv("TEST_TMPDIR"));
+ const ::std::string logfile_base = tmpdir + "/multi_logfile";
+ const ::std::string logfile1 = logfile_base + "_pi1_data.bfbs";
+ const ::std::string logfile2 =
+ logfile_base + "_pi2_data/test/aos.examples.Pong.bfbs";
+ const ::std::string logfile3 = logfile_base + "_pi2_data.bfbs";
+
+ // Remove them.
+ unlink(logfile1.c_str());
+ unlink(logfile2.c_str());
+ unlink(logfile3.c_str());
+
+ LOG(INFO) << "Logging data to " << logfile1 << ", " << logfile2 << " and "
+ << logfile3;
+
+ {
+ std::unique_ptr<EventLoop> ping_event_loop =
+ event_loop_factory_.MakeEventLoop("ping", pi1_);
+ Ping ping(ping_event_loop.get());
+ std::unique_ptr<EventLoop> pong_event_loop =
+ event_loop_factory_.MakeEventLoop("pong", pi2_);
+ Pong pong(pong_event_loop.get());
+
+ std::unique_ptr<EventLoop> pi1_logger_event_loop =
+ event_loop_factory_.MakeEventLoop("logger", pi1_);
+ std::unique_ptr<LogNamer> pi1_log_namer =
+ std::make_unique<MultiNodeLogNamer>(
+ logfile_base, pi1_logger_event_loop->configuration(),
+ pi1_logger_event_loop->node());
+
+ std::unique_ptr<EventLoop> pi2_logger_event_loop =
+ event_loop_factory_.MakeEventLoop("logger", pi2_);
+ std::unique_ptr<LogNamer> pi2_log_namer =
+ std::make_unique<MultiNodeLogNamer>(
+ logfile_base, pi2_logger_event_loop->configuration(),
+ pi2_logger_event_loop->node());
+
+ event_loop_factory_.RunFor(chrono::milliseconds(95));
+
+ Logger pi1_logger(std::move(pi1_log_namer), pi1_logger_event_loop.get(),
+ chrono::milliseconds(100));
+
+ Logger pi2_logger(std::move(pi2_log_namer), pi2_logger_event_loop.get(),
+ chrono::milliseconds(100));
+ event_loop_factory_.RunFor(chrono::milliseconds(20000));
+ }
+
+ // Test that, if we add an additional node to the replay config that the
+ // logger complains about the mismatch in number of nodes.
+ FlatbufferDetachedBuffer<Configuration> extra_nodes_config =
+ configuration::MergeWithConfig(&config_.message(), R"({
+ "nodes": [
+ {
+ "name": "extra-node"
+ }
+ ]
+ }
+ )");
+
+ EXPECT_DEATH(LogReader({std::vector<std::string>{logfile1},
+ std::vector<std::string>{logfile3}},
+ &extra_nodes_config.message()),
+ "Log file and replay config need to have matching nodes lists.");
+ ;
+}
+
// Tests that we can read log files where they don't start at the same monotonic
// time.
TEST_F(MultinodeLoggerTest, StaggeredStart) {