You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nifi.apache.org by ma...@apache.org on 2015/11/11 20:23:31 UTC

[1/2] nifi git commit: NIFI-1143 Fixed race condition which caused intermittent failures Fixed the order of service state check in PropertyDescriptor Encapsulated the check into private method for readability Modified and documented test to validate corr

Repository: nifi
Updated Branches:
  refs/heads/master 11fcad90d -> 854d20398


NIFI-1143 Fixed race condition which caused intermittent failures
Fixed the order of service state check in PropertyDescriptor
Encapsulated the check into private method for readability
Modified and documented test to validate correct behavior.
For more details please see comment in https://issues.apache.org/jira/browse/NIFI-1143


Project: http://git-wip-us.apache.org/repos/asf/nifi/repo
Commit: http://git-wip-us.apache.org/repos/asf/nifi/commit/5baafa15
Tree: http://git-wip-us.apache.org/repos/asf/nifi/tree/5baafa15
Diff: http://git-wip-us.apache.org/repos/asf/nifi/diff/5baafa15

Branch: refs/heads/master
Commit: 5baafa156a58eda7fec7366cc5116da9a2a5a9ec
Parents: 6c510fa
Author: Oleg Zhurakousky <ol...@suitcase.io>
Authored: Wed Nov 11 14:06:08 2015 -0500
Committer: Oleg Zhurakousky <ol...@suitcase.io>
Committed: Wed Nov 11 14:06:08 2015 -0500

----------------------------------------------------------------------
 .../nifi/components/PropertyDescriptor.java     | 25 ++++++++++++++++++--
 .../TestStandardControllerServiceProvider.java  | 17 +++++++++++--
 2 files changed, 38 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/nifi/blob/5baafa15/nifi-api/src/main/java/org/apache/nifi/components/PropertyDescriptor.java
----------------------------------------------------------------------
diff --git a/nifi-api/src/main/java/org/apache/nifi/components/PropertyDescriptor.java b/nifi-api/src/main/java/org/apache/nifi/components/PropertyDescriptor.java
index acda6c4..77c349a 100644
--- a/nifi-api/src/main/java/org/apache/nifi/components/PropertyDescriptor.java
+++ b/nifi-api/src/main/java/org/apache/nifi/components/PropertyDescriptor.java
@@ -150,8 +150,7 @@ public final class PropertyDescriptor implements Comparable<PropertyDescriptor>
                 }
 
                 final String serviceId = controllerService.getIdentifier();
-                if (!context.getControllerServiceLookup().isControllerServiceEnabled(serviceId)
-                        && !context.getControllerServiceLookup().isControllerServiceEnabling(serviceId)) {
+                if (!isDependentServiceEnableable(context, serviceId)) {
                     return new ValidationResult.Builder()
                             .input(context.getControllerServiceLookup().getControllerServiceName(serviceId))
                             .subject(getName())
@@ -197,6 +196,28 @@ public final class PropertyDescriptor implements Comparable<PropertyDescriptor>
         return lastResult;
     }
 
+    /**
+     * Will validate if the dependent service (service identified with the
+     * 'serviceId') is 'enableable' which means that the dependent service is
+     * either in ENABLING or ENABLED state. The important issue here is to
+     * understand the order in which states are assigned:
+     *
+     * - Upon the initialization of the service its state is set to ENABLING.
+     *
+     * - Transition to ENABLED will happen asynchronously.
+     *
+     * So we check first for ENABLING state and if it succeeds we skip the check
+     * for ENABLED state even though by the time this method returns the
+     * dependent service's state could be fully ENABLED.
+     */
+    private boolean isDependentServiceEnableable(ValidationContext context, String serviceId) {
+        boolean enableable = context.getControllerServiceLookup().isControllerServiceEnabling(serviceId);
+        if (!enableable) {
+            enableable = context.getControllerServiceLookup().isControllerServiceEnabled(serviceId);
+        }
+        return enableable;
+    }
+
     public static final class Builder {
 
         private String displayName = null;

http://git-wip-us.apache.org/repos/asf/nifi/blob/5baafa15/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/controller/service/TestStandardControllerServiceProvider.java
----------------------------------------------------------------------
diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/controller/service/TestStandardControllerServiceProvider.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/controller/service/TestStandardControllerServiceProvider.java
index a3589fa..bca13eb 100644
--- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/controller/service/TestStandardControllerServiceProvider.java
+++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/controller/service/TestStandardControllerServiceProvider.java
@@ -20,6 +20,7 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
+import java.beans.PropertyDescriptor;
 import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.List;
@@ -110,9 +111,21 @@ public class TestStandardControllerServiceProvider {
         }
     }
 
-    @Test(timeout=10000)
-    public void testEnableReferencingServicesGraph() {
+    /**
+     * We run the same test 1000 times and prior to bug fix (see NIFI-1143) it
+     * would fail on some iteration. For more details please see
+     * {@link PropertyDescriptor}.isDependentServiceEnableable() as well as
+     * https://issues.apache.org/jira/browse/NIFI-1143
+     */
+    @Test(timeout = 10000)
+    public void testConcurrencyWithEnablingReferencingServicesGraph() {
         final ProcessScheduler scheduler = createScheduler();
+        for (int i = 0; i < 1000; i++) {
+            testEnableReferencingServicesGraph(scheduler);
+        }
+    }
+
+    public void testEnableReferencingServicesGraph(ProcessScheduler scheduler) {
         final StandardControllerServiceProvider provider = new StandardControllerServiceProvider(scheduler, null);
 
         // build a graph of controller services with dependencies as such:


[2/2] nifi git commit: Merge branch 'NIFI-1143' of https://github.com/olegz/nifi into NIFI-1143

Posted by ma...@apache.org.
Merge branch 'NIFI-1143' of https://github.com/olegz/nifi into NIFI-1143


Project: http://git-wip-us.apache.org/repos/asf/nifi/repo
Commit: http://git-wip-us.apache.org/repos/asf/nifi/commit/854d2039
Tree: http://git-wip-us.apache.org/repos/asf/nifi/tree/854d2039
Diff: http://git-wip-us.apache.org/repos/asf/nifi/diff/854d2039

Branch: refs/heads/master
Commit: 854d20398242a4500b6849d2f9c7f9ec917b4ec7
Parents: 11fcad9 5baafa1
Author: Mark Payne <ma...@hotmail.com>
Authored: Wed Nov 11 14:14:54 2015 -0500
Committer: Mark Payne <ma...@hotmail.com>
Committed: Wed Nov 11 14:14:54 2015 -0500

----------------------------------------------------------------------
 .../nifi/components/PropertyDescriptor.java     | 25 ++++++++++++++++++--
 .../TestStandardControllerServiceProvider.java  | 17 +++++++++++--
 2 files changed, 38 insertions(+), 4 deletions(-)
----------------------------------------------------------------------