Review comments #2

arpadboda · Jan 31, 2020 · 543c2d4 · 543c2d4
1 parent 777d3f0
commit 543c2d4
Show file tree

Hide file tree

Showing 2 changed files with 7 additions and 17 deletions.
diff --git a/libminifi/include/SchedulingAgent.h b/libminifi/include/SchedulingAgent.h
@@ -141,10 +141,6 @@ class SchedulingAgent {
     watchDogTimer_.reset(new utils::CallBackTimer(std::chrono::milliseconds(SCHEDULING_WATCHDOG_CHECK_PERIOD), f));
     watchDogTimer_->start();
   }
-  // Destructor
-  virtual ~SchedulingAgent() {
-    watchDogTimer_->stop();
-  }
 
   // onTrigger, return whether the yield is need
   bool onTrigger(const std::shared_ptr<core::Processor> &processor, const std::shared_ptr<core::ProcessContext> &processContext, const std::shared_ptr<core::ProcessSessionFactory> &sessionFactory);
@@ -202,15 +198,13 @@ class SchedulingAgent {
 
  private:
   struct SchedulingInfo {
-    std::chrono::time_point<std::chrono::steady_clock> start_time_;
+    std::chrono::time_point<std::chrono::steady_clock> start_time_ = std::chrono::steady_clock::now();
     std::string name_;
     std::string uuid_;
 
-    SchedulingInfo(const std::shared_ptr<core::Processor> &processor) {
-     start_time_ = std::chrono::steady_clock::now();
-     name_ = processor->getName();
-     uuid_ = processor->getUUIDStr();
-    }
+    explicit SchedulingInfo(const std::shared_ptr<core::Processor> &processor) :
+      name_(processor->getName()),
+      uuid_(processor->getUUIDStr()) {}
 
     bool operator <(const SchedulingInfo& o) const {
       return std::tie(start_time_, name_, uuid_) < std::tie(o.start_time_, o.name_, o.uuid_);
@@ -219,13 +213,9 @@ class SchedulingAgent {
 
   // Logger
   std::shared_ptr<logging::Logger> logger_;
-  std::mutex watchdog_mtx_;  // used to protect the vector below
-  std::set<SchedulingInfo> scheduled_processors_;
+  std::mutex watchdog_mtx_;  // used to protect the set below
+  std::set<SchedulingInfo> scheduled_processors_;  // set was chosen to avoid iterator invalidation
   std::unique_ptr<utils::CallBackTimer> watchDogTimer_;
-
-  // Prevent default copy constructor and assignment operation
-  // Only support pass by reference or pointer
-
 };
 
 } /* namespace minifi */

diff --git a/libminifi/src/SchedulingAgent.cpp b/libminifi/src/SchedulingAgent.cpp
@@ -144,7 +144,7 @@ void SchedulingAgent::watchDogFunc() {
   for (const auto& info : scheduled_processors_) {
     int64_t elapsed = std::chrono::duration_cast<std::chrono::milliseconds>(now - info.start_time_).count();
     if (elapsed > SCHEDULING_WATCHDOG_ALERT_PERIOD) {
-      logger_->log_warn("%s::onTrigger is running for %lld ms in %s", info.name_, elapsed, info.uuid_);
+      logger_->log_warn("%s::onTrigger has been running for %lld ms in %s", info.name_, elapsed, info.uuid_);
     }
   }
 }