arpadboda commented on a change in pull request #777:
URL: https://github.com/apache/nifi-minifi-cpp/pull/777#discussion_r421143128
##########
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:
So the reason behind:
When the schedulers above are stopped, they don't actually stop the
processors they schedule, only remove all waiting tasks of those from the
threadpool.
This means that ontrigger calls can actually be in process by the time we
start disabling all controller services. Which seems to be a potential risk to
me. I guess this introduced the test case flicker related to mocked controller
services, for eg in this CI execution:
https://travis-ci.org/github/apache/nifi-minifi-cpp/jobs/683790379
In my opinion controller services have no dependency on the threadpool of
flowcontroller (they don't even get reference to flowcontroller, just the
configuration as far as I see), so shutting down the threadpool and disabling
controller services only at the end seems to be the safety choice.
@bakaid as you authored the original change to stop controller services
here, what do you think?
----------------------------------------------------------------
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:
[email protected]
