[GitHub] [nifi-minifi-cpp] arpadboda commented on a change in pull request #890: MINIFICPP-1288 - FlowController start/load refactor

[GitBox <gi...@apache.org>]

arpadboda commented on a change in pull request #890:
URL: https://github.com/apache/nifi-minifi-cpp/pull/890#discussion_r484368876



##########
File path: libminifi/include/FlowController.h
##########
@@ -338,6 +335,10 @@ class FlowController : public core::controller::ControllerServiceProvider, publi
   utils::optional<std::chrono::milliseconds> loadShutdownTimeoutFromConfiguration();
 
  private:
+  void restartThreadPool();
+  void initializeUninitializedSchedulers();
+  void reinitializeSchedulersWithNewThreadPool();

Review comment:
       I'm not sure if the name of this function actually reflect what it does. 
   There is no new threadpool, as far as I see it's created only once and stopped/started according to usage later. 

##########
File path: libminifi/include/FlowController.h
##########
@@ -100,18 +100,15 @@ class FlowController : public core::controller::ControllerServiceProvider, publi
   }
 
   // Load flow xml from disk, after that, create the root process group and its children, initialize the flows
-  virtual void load(const std::shared_ptr<core::ProcessGroup> &root = nullptr, bool reload = false);
+  virtual void load(const std::shared_ptr<core::ProcessGroup> &root = nullptr);
 
   // Whether the Flow Controller is start running
   bool isRunning() override {
     return running_.load() || updating_.load();
   }
 
-  // Whether the Flow Controller has already been initialized (loaded flow XML)
-  virtual bool isInitialized() {
-    return initialized_.load();
-  }
   // Start to run the Flow Controller which internally start the root process group and all its children
+  // int16_t start() override;

Review comment:
       What's the purpose of this new comment here?

##########
File path: libminifi/src/FlowController.cpp
##########
@@ -311,56 +302,62 @@ void FlowController::unload() {
   if (running_) {
     stop();
   }
-  if (initialized_) {
-    logger_->log_info("Unload Flow Controller");
-    initialized_ = false;
-    name_ = "";
-  }
+  logger_->log_info("Unload Flow Controller");
+  name_ = "";
 }
 
-void FlowController::load(const std::shared_ptr<core::ProcessGroup> &root, bool reload) {
+void FlowController::restartThreadPool() {
+  auto base_shared_ptr = std::dynamic_pointer_cast<core::controller::ControllerServiceProvider>(shared_from_this());
+  thread_pool_.shutdown();
+  thread_pool_.setMaxConcurrentTasks(configuration_->getInt(Configure::nifi_flow_engine_threads, 2));
+  thread_pool_.setControllerServiceProvider(base_shared_ptr);
+  thread_pool_.start();
+}
+
+void FlowController::initializeUninitializedSchedulers() {
+  conditionalReloadScheduler<TimerDrivenSchedulingAgent>(timer_scheduler_, !timer_scheduler_);
+  conditionalReloadScheduler<EventDrivenSchedulingAgent>(event_scheduler_, !event_scheduler_);
+  conditionalReloadScheduler<CronDrivenSchedulingAgent>(cron_scheduler_, !cron_scheduler_);
+}
+
+void FlowController::reinitializeSchedulersWithNewThreadPool() {
+  using ControllerServiceProvider = core::controller::ControllerServiceProvider;

Review comment:
       Just fancy: in case we increase code readability this way, why don't we alias the templated gsl::not_null class instead of the parameter?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org