Updated Branches: refs/heads/master c9fe86147 -> 63080ac78
CAMEL-6341: Shutdown strategy requires a positive timeout value to be set, as using 0 is counter intuitive, and can lead to stuck app. Project: http://git-wip-us.apache.org/repos/asf/camel/repo Commit: http://git-wip-us.apache.org/repos/asf/camel/commit/63080ac7 Tree: http://git-wip-us.apache.org/repos/asf/camel/tree/63080ac7 Diff: http://git-wip-us.apache.org/repos/asf/camel/diff/63080ac7 Branch: refs/heads/master Commit: 63080ac7831932467e9c4ca1949916f0927e8718 Parents: c9fe861 Author: Claus Ibsen <[email protected]> Authored: Fri May 17 15:47:01 2013 +0200 Committer: Claus Ibsen <[email protected]> Committed: Fri May 17 15:47:20 2013 +0200 ---------------------------------------------------------------------- .../apache/camel/impl/DefaultShutdownStrategy.java | 40 ++++++-------- 1 files changed, 17 insertions(+), 23 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/camel/blob/63080ac7/camel-core/src/main/java/org/apache/camel/impl/DefaultShutdownStrategy.java ---------------------------------------------------------------------- diff --git a/camel-core/src/main/java/org/apache/camel/impl/DefaultShutdownStrategy.java b/camel-core/src/main/java/org/apache/camel/impl/DefaultShutdownStrategy.java index a7e9cf8..9d48fd9 100644 --- a/camel-core/src/main/java/org/apache/camel/impl/DefaultShutdownStrategy.java +++ b/camel-core/src/main/java/org/apache/camel/impl/DefaultShutdownStrategy.java @@ -28,6 +28,7 @@ import java.util.concurrent.ExecutorService; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicBoolean; import org.apache.camel.CamelContext; import org.apache.camel.CamelContextAware; @@ -161,21 +162,17 @@ public class DefaultShutdownStrategy extends ServiceSupport implements ShutdownS Collections.reverse(routesOrdered); } - if (timeout > 0) { - LOG.info("Starting to graceful shutdown " + routesOrdered.size() + " routes (timeout " + timeout + " " + timeUnit.toString().toLowerCase(Locale.ENGLISH) + ")"); - } else { - LOG.info("Starting to graceful shutdown " + routesOrdered.size() + " routes (no timeout)"); - } + LOG.info("Starting to graceful shutdown " + routesOrdered.size() + " routes (timeout " + timeout + " " + timeUnit.toString().toLowerCase(Locale.ENGLISH) + ")"); // use another thread to perform the shutdowns so we can support timeout - Future<?> future = getExecutorService().submit(new ShutdownTask(context, routesOrdered, timeout, timeUnit, suspendOnly, abortAfterTimeout)); + final AtomicBoolean timeoutOccurred = new AtomicBoolean(); + Future<?> future = getExecutorService().submit(new ShutdownTask(context, routesOrdered, timeout, timeUnit, suspendOnly, abortAfterTimeout, timeoutOccurred)); try { - if (timeout > 0) { - future.get(timeout, timeUnit); - } else { - future.get(); - } + future.get(timeout, timeUnit); } catch (TimeoutException e) { + // we hit a timeout, so set the flag + timeoutOccurred.set(true); + // timeout then cancel the task future.cancel(true); @@ -220,8 +217,8 @@ public class DefaultShutdownStrategy extends ServiceSupport implements ShutdownS } public void setTimeout(long timeout) { - if (timeout < 0) { - throw new IllegalArgumentException("Timeout must not be lesser than 0."); + if (timeout <= 0) { + throw new IllegalArgumentException("Timeout must be a positive value"); } this.timeout = timeout; } @@ -427,15 +424,17 @@ public class DefaultShutdownStrategy extends ServiceSupport implements ShutdownS private final boolean abortAfterTimeout; private final long timeout; private final TimeUnit timeUnit; + private final AtomicBoolean timeoutOccurred; public ShutdownTask(CamelContext context, List<RouteStartupOrder> routes, long timeout, TimeUnit timeUnit, - boolean suspendOnly, boolean abortAfterTimeout) { + boolean suspendOnly, boolean abortAfterTimeout, AtomicBoolean timeoutOccurred) { this.context = context; this.routes = routes; this.suspendOnly = suspendOnly; this.abortAfterTimeout = abortAfterTimeout; this.timeout = timeout; this.timeUnit = timeUnit; + this.timeoutOccurred = timeoutOccurred; } public void run() { @@ -518,7 +517,7 @@ public class DefaultShutdownStrategy extends ServiceSupport implements ShutdownS boolean done = false; long loopDelaySeconds = 1; long loopCount = 0; - while (!done) { + while (!done && !timeoutOccurred.get()) { int size = 0; for (RouteStartupOrder order : routes) { int inflight = context.getInflightRepository().size(order.getRoute().getId()); @@ -536,14 +535,9 @@ public class DefaultShutdownStrategy extends ServiceSupport implements ShutdownS } if (size > 0) { try { - if (timeout > 0) { - LOG.info("Waiting as there are still " + size + " inflight and pending exchanges to complete, timeout in " - + (TimeUnit.SECONDS.convert(timeout, timeUnit) - (loopCount++ * loopDelaySeconds)) + " seconds."); - Thread.sleep(loopDelaySeconds * 1000); - } else { - // we should not wait here - throw new InterruptedException(); - } + LOG.info("Waiting as there are still " + size + " inflight and pending exchanges to complete, timeout in " + + (TimeUnit.SECONDS.convert(timeout, timeUnit) - (loopCount++ * loopDelaySeconds)) + " seconds."); + Thread.sleep(loopDelaySeconds * 1000); } catch (InterruptedException e) { if (abortAfterTimeout) { LOG.warn("Interrupted while waiting during graceful shutdown, will abort.");
