pvillard31 commented on code in PR #11646:
URL: https://github.com/apache/nifi/pull/11646#discussion_r3967887122
##########
nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/controller/service/StandardControllerServiceNode.java:
##########
@@ -782,14 +774,8 @@ public CompletableFuture<Void> disable(final
ScheduledExecutorService scheduler)
}
final CompletableFuture<Void> future = new CompletableFuture<>();
- // If already disabled, complete immediately
- if (getState() == ControllerServiceState.DISABLED) {
- future.complete(null);
- return future;
- }
-
- final boolean transitioned =
this.stateTransition.transitionToDisabling(ControllerServiceState.ENABLING,
future);
- if (transitioned) {
+ final ControllerServiceState previousState =
stateTransition.transitionToDisabling(future);
Review Comment:
Can we prevent a new enable cycle until the existing `@OnEnabled` invocation
returns, since the immediate `DISABLED` transition lets the old invocation mark
the new cycle `ENABLED`?
##########
nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/components/connector/StandardConnectorNodeIT.java:
##########
@@ -397,6 +401,82 @@ public void testControllerServices() throws
FlowUpdateException {
assertInstanceOf(CounterService.class,
serviceNodes.iterator().next().getControllerServiceImplementation());
}
+ @Test
+ @Timeout(20)
+ public void testStopConnectorWhileManagedServiceBlocksInOnEnabled() throws
Exception {
+ final ConnectorNode connectorNode =
initializeControllerServiceEnablingConnector(ControllerServiceEnablingConnector.BLOCKING_ENABLING);
+ final ControllerServiceNode serviceNode =
getManagedControllerService(connectorNode);
+ final BlockingEnablingCounterService service =
(BlockingEnablingCounterService)
serviceNode.getControllerServiceImplementation();
+
+ connectorNode.start(componentLifecycleThreadPool);
+ try {
+ waitForServiceState(serviceNode, ControllerServiceState.ENABLING);
+ waitForEnableInvocation(service::enableInvocationCount, 1);
+
+ final Future<Void> stopFuture =
connectorNode.stop(componentLifecycleThreadPool);
+ stopFuture.get(5, TimeUnit.SECONDS);
+
+ assertEquals(ConnectorState.STOPPED,
connectorNode.getCurrentState());
Review Comment:
Can we add a stop-then-restart test that keeps the first `@OnEnabled`
invocation blocked, to verify that it cannot complete or interfere with the new
enable cycle?
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
