Github user alopresto commented on a diff in the pull request:
https://github.com/apache/nifi/pull/541#discussion_r67581551
--- Diff:
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/service/StandardControllerServiceProvider.java
---
@@ -372,98 +369,32 @@ public void enableControllerService(final
ControllerServiceNode serviceNode) {
@Override
public void enableControllerServices(final
Collection<ControllerServiceNode> serviceNodes) {
- final Set<ControllerServiceNode> servicesToEnable = new
HashSet<>();
- // Ensure that all nodes are already disabled
- for (final ControllerServiceNode serviceNode : serviceNodes) {
- final ControllerServiceState curState = serviceNode.getState();
- if (ControllerServiceState.DISABLED.equals(curState)) {
- servicesToEnable.add(serviceNode);
- } else {
- logger.warn("Cannot enable {} because it is not disabled;
current state is {}", serviceNode, curState);
+ boolean shouldStart = true;
+
+ Iterator<ControllerServiceNode> serviceIter =
serviceNodes.iterator();
+ while (serviceIter.hasNext() && shouldStart) {
+ ControllerServiceNode controllerServiceNode =
serviceIter.next();
+ List<ControllerServiceNode> requiredServices =
((StandardControllerServiceNode)
controllerServiceNode).getRequiredControllerServices();
+ for (ControllerServiceNode requiredService : requiredServices)
{
+ if (!requiredService.isActive() &&
!serviceNodes.contains(requiredService)) {
+ shouldStart = false;
+ }
}
}
- // determine the order to load the services. We have to ensure
that if service A references service B, then B
- // is enabled first, and so on.
- final Map<String, ControllerServiceNode> idToNodeMap = new
HashMap<>();
- for (final ControllerServiceNode node : servicesToEnable) {
- idToNodeMap.put(node.getIdentifier(), node);
- }
-
- // We can have many Controller Services dependent on one another.
We can have many of these
- // disparate lists of Controller Services that are dependent on
one another. We refer to each
- // of these as a branch.
- final List<List<ControllerServiceNode>> branches =
determineEnablingOrder(idToNodeMap);
-
- if (branches.isEmpty()) {
- logger.info("No Controller Services to enable");
- return;
- } else {
- logger.info("Will enable {} Controller Services",
servicesToEnable.size());
- }
-
- final Set<ControllerServiceNode> enabledNodes =
Collections.synchronizedSet(new HashSet<ControllerServiceNode>());
- final ExecutorService executor =
Executors.newFixedThreadPool(Math.min(10, branches.size()), new ThreadFactory()
{
- @Override
- public Thread newThread(final Runnable r) {
- final Thread t =
Executors.defaultThreadFactory().newThread(r);
- t.setDaemon(true);
- t.setName("Enable Controller Services");
- return t;
- }
- });
-
- for (final List<ControllerServiceNode> branch : branches) {
- final Runnable enableBranchRunnable = new Runnable() {
+ if (shouldStart) {
+ List<ControllerServiceNode> services = new
ArrayList<>(serviceNodes);
+ Collections.sort(services, new
Comparator<ControllerServiceNode>() {
--- End diff --
This looks like a much cleaner way to perform this behavior, but are there
any tests for the dependency traversal to ensure it's complete?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
