pvillard31 commented on code in PR #10630:
URL: https://github.com/apache/nifi/pull/10630#discussion_r2619597138
##########
nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/controller/service/StandardControllerServiceNode.java:
##########
@@ -753,6 +753,18 @@ public CompletableFuture<Void> disable(final
ScheduledExecutorService scheduler)
final CompletableFuture<Void> future = new CompletableFuture<>();
final boolean transitioned =
this.stateTransition.transitionToDisabling(ControllerServiceState.ENABLING,
future);
if (transitioned) {
+ // If we transitioned from ENABLING to DISABLING, we need to
immediately complete the disable
+ // because the enable task may be scheduled to run far in the
future (up to 10 minutes) due to
+ // validation retries. Rather than making the user wait, we
immediately transition to DISABLED.
+ scheduler.execute(() -> {
+ stateTransition.disable();
Review Comment:
The `@OnDisabled` method is intentionally not invoked when transitioning
from ENABLING to DISABLED. Here's my chain of thoughts:
`@OnDisabled` should only be called if `@OnEnabled` was successfully
invoked, to maintain paired lifecycle semantics.
When we're in `ENABLING` state, `@OnEnabled` has NOT been successfully
called yet:
- If validation is failing (the common case for Issue 2), the enable task is
stuck in the validation retry loop (lines 650-680) and `@OnEnabled` is never
reached.
- If `@OnEnabled` was called and succeeded, the state would have already
transitioned to `ENABLED` (line 690: `stateTransition.enable()`), so we
wouldn't be in `ENABLING` state anymore.
- If `@OnEnabled` was called and failed, the enable task reschedules itself
(line 718) but `@OnEnabled` didn't complete successfully.
This pattern is consistent with existing code - lines 643-647 in the enable
task:
```java
if (!isActive()) {
LOG.warn("Enabling {} stopped: no active status", serviceNode);
stateTransition.disable();
future.complete(null);
return;
}
```
The only case where `@OnDisabled` is called during an interrupted enable is
at lines 693-699, where `@OnEnabled` succeeded but the user disabled before the
state could transition to `ENABLED`. In that specific case, `invokeDisable()`
is called because we know @OnEnabled completed.
Thoughts?
--
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]
