[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #777: MINIFICPP-1216 - Controller Services Integration test is unstable

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

szaszm commented on a change in pull request #777:
URL: https://github.com/apache/nifi-minifi-cpp/pull/777#discussion_r422195976



##########
File path: libminifi/src/FlowController.cpp
##########
@@ -248,9 +248,9 @@ int16_t FlowController::stop(bool force, uint64_t timeToWait) {
     this->timer_scheduler_->stop();
     this->event_scheduler_->stop();
     this->cron_scheduler_->stop();
+    thread_pool_.shutdown();

Review comment:
       Given the above discussion, the changes look reasonable to me. However, the discussion revealed that this code has numerous hidden interconnections: processors may run after initiating stop => both repositories and controller services need to be stopped after the thread pool, controller services need to be stopped because of db locking, affected C2 which I don't understand in detail.
   
   Could you add a code comment mentioning the existence of these to the future reader/developer? While not all of it is strictly in the scope of this fix, I think the value/effort ratio would be outstanding here, making it worth doing IMO.




----------------------------------------------------------------
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