nifi-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (NIFI-2032) Processors could be started before the Controller Services that they depend on
Date Mon, 20 Jun 2016 18:24:57 GMT

    [ https://issues.apache.org/jira/browse/NIFI-2032?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15340111#comment-15340111
] 

ASF GitHub Bot commented on NIFI-2032:
--------------------------------------

Github user alopresto commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/541#discussion_r67740493
  
    --- 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>()
{
                     @Override
    -                public void run() {
    -                    logger.debug("Enabling Controller Service Branch {}", branch);
    -
    -                    for (final ControllerServiceNode serviceNode : branch) {
    -                        try {
    -                            if (!enabledNodes.contains(serviceNode)) {
    -                                enabledNodes.add(serviceNode);
    -
    -                                logger.info("Enabling {}", serviceNode);
    -                                try {
    -                                    serviceNode.verifyCanEnable();
    -                                    processScheduler.enableControllerService(serviceNode);
    -                                } catch (final Exception e) {
    -                                    logger.error("Failed to enable " + serviceNode +
" due to " + e);
    -                                    if (logger.isDebugEnabled()) {
    -                                        logger.error("", e);
    -                                    }
    -
    -                                    if (bulletinRepo != null) {
    --- End diff --
    
    Ok, we should probably maintain that behavior, correct? Not sure why it was commented
out initially. 


> Processors could be started before the Controller Services that they depend on
> ------------------------------------------------------------------------------
>
>                 Key: NIFI-2032
>                 URL: https://issues.apache.org/jira/browse/NIFI-2032
>             Project: Apache NiFi
>          Issue Type: Bug
>          Components: Core Framework
>            Reporter: Mark Payne
>            Assignee: Oleg Zhurakousky
>            Priority: Critical
>             Fix For: 1.0.0, 0.7.0
>
>
> in the StandardControllerServiceProvider, we enable a Collection of Controller Services
but do so in the background with a limited number of threads. We need to ensure that all Controller
Services have at least become ENABLING before returning from this method. Otherwise, Processors
that depend on them could attempt to start. If this happens, the Processor will be invalid
because it references a Disabled Controller Service. As a result, the Processor will not start.
In a clustered environment, we will end up with inconsistent run states across the nodes.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message