Add extra comments for various start/end handlers
Hopefully this helps to provide some clarity for people about when to
use the LogReader vs. NodeEventLoopFactory startup handlers.
Change-Id: Ice8501f4c4da3d111cffcfa16298f7099e847caa
Signed-off-by: James Kuszmaul <jabukuszmaul+collab@gmail.com>
diff --git a/aos/events/logging/log_reader.cc b/aos/events/logging/log_reader.cc
index 1e7644f..cbe37d5 100644
--- a/aos/events/logging/log_reader.cc
+++ b/aos/events/logging/log_reader.cc
@@ -2335,7 +2335,12 @@
}
void LogReader::State::NotifyLogfileStart() {
+ // If the start_event_notifier_ is set, that means that a realtime start time
+ // was set manually; when the override is set, we want to delay any startup
+ // handlers that would've happened before requested start time until that
+ // start time.
if (start_event_notifier_) {
+ // Only call OnStart() if the start time for this node (realtime_start_time())
if (start_event_notifier_->realtime_event_time() >
realtime_start_time(boot_count())) {
VLOG(1) << "Skipping, " << start_event_notifier_->realtime_event_time()
@@ -2351,6 +2356,9 @@
}
void LogReader::State::NotifyFlagStart() {
+ // Should only be called if start_event_notifier_ has been set (which happens
+ // as part of setting an explicit start time); only call the startup functions
+ // that occurred *before* the start flag value.
if (start_event_notifier_->realtime_event_time() >=
realtime_start_time(boot_count())) {
RunOnStart();
@@ -2358,16 +2366,22 @@
}
void LogReader::State::NotifyLogfileEnd() {
+ // Don't execute the OnEnd handlers if the logfile was ended artifically
+ // early.
if (found_last_message_) {
return;
}
+ // Ensure that we only call OnEnd() if OnStart() was already called for this
+ // boot (and don't call OnEnd() twice).
if (!stopped_ && started_) {
RunOnEnd();
}
}
void LogReader::State::NotifyFlagEnd() {
+ // Ensure that we only call OnEnd() if OnStart() was already called for this
+ // boot (and don't call OnEnd() twice).
if (!stopped_ && started_) {
RunOnEnd();
SetFoundLastMessage(true);
diff --git a/aos/events/logging/log_reader.h b/aos/events/logging/log_reader.h
index ddf9116..49babe5 100644
--- a/aos/events/logging/log_reader.h
+++ b/aos/events/logging/log_reader.h
@@ -140,9 +140,29 @@
}
// Called whenever a log file starts for a node.
+ // More precisely, this will be called on each boot at max of
+ // (realtime_start_time in the logfiles, SetStartTime()). If a given boot
+ // occurs entirely before the realtime_start_time, the OnStart handler will
+ // never get called for that boot.
+ //
+ // realtime_start_time is defined below, but/ essentially is the time at which
+ // message channels will start being internall consistent on a given node
+ // (i.e., when the logger started). Note: If you wish to see a watcher
+ // triggered for *every* message in a log, OnStart() will not be
+ // sufficient--messages (possibly multiple messages) may be present on
+ // channels prior to the start time. If attempting to do this, prefer to use
+ // NodeEventLoopFactory::OnStart.
void OnStart(std::function<void()> fn);
void OnStart(const Node *node, std::function<void()> fn);
- // Called whenever a log file ends for a node.
+ // Called whenever a log file ends for a node on a given boot, or at the
+ // realtime_end_time specified by a flag or SetEndTime().
+ //
+ // A log file "ends" when there are no more messages to be replayed for that
+ // boot.
+ //
+ // If OnStart() is not called for a given boot, the OnEnd() handlers will not
+ // be called either. OnEnd() handlers will not be called if the logfile for a
+ // given boot has missing data that causes us to terminate replay early.
void OnEnd(std::function<void()> fn);
void OnEnd(const Node *node, std::function<void()> fn);