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
This commit is contained in:
Oleg Zhurakousky 2015-11-11 14:06:08 -05:00
parent 6c510fae80
commit 5baafa156a
2 changed files with 38 additions and 4 deletions

View File

@ -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;

View File

@ -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: