bakaid commented on a change in pull request #777:
URL: https://github.com/apache/nifi-minifi-cpp/pull/777#discussion_r421151804
##########
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:
@arpadboda During development of the new state storage at one point I
remember shutting down the thread pool before disabling the controller
services. While it was my gut feeling that there should be no requirement for
the thread pool to be running (nor did I found one), nevertheless it caused the
shutdown to fail and the whole agent to freeze on C2 reload for example.
I was operating under the assumption that `this->root_->stopProcessing` will
stop the processors. If that's not the case, `this->flow_file_repo_->stop();`
and `this->provenance_repo_->stop();` that also currently come before the
processors' running onTriggers could end would also be problematic (in the same
manner as disabling the controller services: it's not a nullptr exception but
the last onTrigger will not be able to use them).
This forced `disableAllControllerServices` was done because we leak
resources on flow reload and we can't have two
RocksDbPersistableKeyValueStoreServices running at the same time, because the
first locks the db: the new flow's one will fail. This is just a workaround,
not a proper solution: a proper solution would be to refactor and fix flow
reload.
If `disableAllControllerServices` can run fine without the thread pool
(again, it failed once, but it might have been just one state inbetween thread
pool and C2 refactors that came and went while I was doing work on the state
PR), then I agree that it would be the safer option to so, but this change has
to be tested thoroughly on C2 reload and agent shutdown conditions.
Also, in this case looking into the repo shutdowns would also make sense.
----------------------------------------------------------------
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]
