NIFI-745: Only call methods with @OnDisabled once, regardless of whether or not they succeed
Project: http://git-wip-us.apache.org/repos/asf/incubator-nifi/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-nifi/commit/20840247 Tree: http://git-wip-us.apache.org/repos/asf/incubator-nifi/tree/20840247 Diff: http://git-wip-us.apache.org/repos/asf/incubator-nifi/diff/20840247 Branch: refs/heads/master Commit: 208402472dd99c629afc056f4464b925b8f834ab Parents: f3b55d4 Author: Mark Payne <marka...@hotmail.com> Authored: Fri Jul 3 14:35:59 2015 -0400 Committer: Mark Payne <marka...@hotmail.com> Committed: Fri Jul 3 15:00:31 2015 -0400 ---------------------------------------------------------------------- .../nifi/annotation/lifecycle/OnDisabled.java | 10 ++--- .../scheduling/StandardProcessScheduler.java | 39 +++++++++----------- 2 files changed, 22 insertions(+), 27 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-nifi/blob/20840247/nifi/nifi-api/src/main/java/org/apache/nifi/annotation/lifecycle/OnDisabled.java ---------------------------------------------------------------------- diff --git a/nifi/nifi-api/src/main/java/org/apache/nifi/annotation/lifecycle/OnDisabled.java b/nifi/nifi-api/src/main/java/org/apache/nifi/annotation/lifecycle/OnDisabled.java index f205bc7..f8ca038 100644 --- a/nifi/nifi-api/src/main/java/org/apache/nifi/annotation/lifecycle/OnDisabled.java +++ b/nifi/nifi-api/src/main/java/org/apache/nifi/annotation/lifecycle/OnDisabled.java @@ -36,11 +36,11 @@ import org.apache.nifi.controller.ConfigurationContext; * Methods using this annotation are permitted to take zero arguments or to take * a single argument of type {@link ConfigurationContext}. If a method with this * annotation throws a Throwable, a log message and bulletin will be issued for - * the service, and the service will remain in a 'DISABLING' state. When this - * occurs, the method with this annotation will be called again after some - * period of time. This will continue until the method returns without throwing - * any Throwable. Until that time, the service will remain in a 'DISABLING' - * state and cannot be enabled again. + * the service, but the service will still be marked as Disabled. The failing + * method will not be called again until the service is enabled and disabled again. + * This is done in order to prevent a ControllerService from continually failing + * in such a way that the service could not be disabled and updated without + * restarting the instance of NiFi. * </p> * * <p> http://git-wip-us.apache.org/repos/asf/incubator-nifi/blob/20840247/nifi/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/scheduling/StandardProcessScheduler.java ---------------------------------------------------------------------- diff --git a/nifi/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/scheduling/StandardProcessScheduler.java b/nifi/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/scheduling/StandardProcessScheduler.java index d976bd0..5ac4a0b 100644 --- a/nifi/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/scheduling/StandardProcessScheduler.java +++ b/nifi/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/scheduling/StandardProcessScheduler.java @@ -685,33 +685,28 @@ public final class StandardProcessScheduler implements ProcessScheduler { try (final NarCloseable x = NarCloseable.withNarLoader()) { final ConfigurationContext configContext = new StandardConfigurationContext(service, controllerServiceProvider, null); - while (true) { - try { - ReflectionUtils.invokeMethodsWithAnnotation(OnDisabled.class, service.getControllerServiceImplementation(), configContext); - heartbeater.heartbeat(); - service.setState(ControllerServiceState.DISABLED); - return; - } catch (final Exception e) { - final Throwable cause = e instanceof InvocationTargetException ? e.getCause() : e; - final ComponentLog componentLog = new SimpleProcessLogger(service.getIdentifier(), service); - componentLog.error("Failed to invoke @OnDisabled method due to {}", cause); - - LOG.error("Failed to invoke @OnDisabled method of {} due to {}", service.getControllerServiceImplementation(), cause.toString()); - if (LOG.isDebugEnabled()) { - LOG.error("", cause); - } + try { + ReflectionUtils.invokeMethodsWithAnnotation(OnDisabled.class, service.getControllerServiceImplementation(), configContext); + } catch (final Exception e) { + final Throwable cause = e instanceof InvocationTargetException ? e.getCause() : e; + final ComponentLog componentLog = new SimpleProcessLogger(service.getIdentifier(), service); + componentLog.error("Failed to invoke @OnDisabled method due to {}", cause); - ReflectionUtils.quietlyInvokeMethodsWithAnnotation(OnDisabled.class, service.getControllerServiceImplementation(), configContext); - try { - Thread.sleep(administrativeYieldMillis); - } catch (final InterruptedException ie) { - } + LOG.error("Failed to invoke @OnDisabled method of {} due to {}", service.getControllerServiceImplementation(), cause.toString()); + if (LOG.isDebugEnabled()) { + LOG.error("", cause); + } - continue; + ReflectionUtils.quietlyInvokeMethodsWithAnnotation(OnDisabled.class, service.getControllerServiceImplementation(), configContext); + try { + Thread.sleep(administrativeYieldMillis); + } catch (final InterruptedException ie) { } + } finally { + service.setState(ControllerServiceState.DISABLED); + heartbeater.heartbeat(); } } - } };