From bf374c999cf0b34f9dc9089e545515c9ee92d96d Mon Sep 17 00:00:00 2001 From: Matt Jacobs Date: Mon, 6 Apr 2015 13:10:35 -0700 Subject: [PATCH] Add ability to disable timeout per-command and wire it in --- .../com/netflix/hystrix/AbstractCommand.java | 6 ++- .../hystrix/HystrixCommandProperties.java | 28 +++++++++++- .../hystrix/HystrixCommandPropertiesTest.java | 6 +++ .../netflix/hystrix/HystrixCommandTest.java | 43 +++++++++++++++++++ 4 files changed, 80 insertions(+), 3 deletions(-) diff --git a/hystrix-core/src/main/java/com/netflix/hystrix/AbstractCommand.java b/hystrix-core/src/main/java/com/netflix/hystrix/AbstractCommand.java index 76b1b1157..6a6f08dc4 100644 --- a/hystrix-core/src/main/java/com/netflix/hystrix/AbstractCommand.java +++ b/hystrix-core/src/main/java/com/netflix/hystrix/AbstractCommand.java @@ -542,7 +542,11 @@ public void call(Notification n) { } - }).lift(new HystrixObservableTimeoutOperator(_self)).doOnNext(new Action1() { + }); + if (properties.executionTimeoutEnabled().get()) { + run = run.lift(new HystrixObservableTimeoutOperator(_self)); + } + run = run.doOnNext(new Action1() { @Override public void call(R r) { if (shouldOutputOnNextEvents()) { diff --git a/hystrix-core/src/main/java/com/netflix/hystrix/HystrixCommandProperties.java b/hystrix-core/src/main/java/com/netflix/hystrix/HystrixCommandProperties.java index 168622a19..84fcdcde6 100644 --- a/hystrix-core/src/main/java/com/netflix/hystrix/HystrixCommandProperties.java +++ b/hystrix-core/src/main/java/com/netflix/hystrix/HystrixCommandProperties.java @@ -46,6 +46,7 @@ public abstract class HystrixCommandProperties { private static final Boolean default_circuitBreakerForceOpen = false;// default => forceCircuitOpen = false (we want to allow traffic) /* package */ static final Boolean default_circuitBreakerForceClosed = false;// default => ignoreErrors = false private static final Integer default_executionTimeoutInMilliseconds = 1000; // default => executionTimeoutInMilliseconds: 1000 = 1 second + private static final Boolean default_executionTimeoutEnabled = true; private static final ExecutionIsolationStrategy default_executionIsolationStrategy = ExecutionIsolationStrategy.THREAD; private static final Boolean default_executionIsolationThreadInterruptOnTimeout = true; private static final Boolean default_metricsRollingPercentileEnabled = true; @@ -69,6 +70,7 @@ public abstract class HystrixCommandProperties { private final HystrixProperty circuitBreakerForceClosed; // a property to allow ignoring errors and therefore never trip 'open' (ie. allow all traffic through) private final HystrixProperty executionIsolationStrategy; // Whether a command should be executed in a separate thread or not. private final HystrixProperty executionTimeoutInMilliseconds; // Timeout value in milliseconds for a command + private final HystrixProperty executionTimeoutEnabled; //Whether timeout should be triggered private final HystrixProperty executionIsolationThreadPoolKeyOverride; // What thread-pool this command should run in (if running on a separate thread). private final HystrixProperty executionIsolationSemaphoreMaxConcurrentRequests; // Number of permits for execution semaphore private final HystrixProperty fallbackIsolationSemaphoreMaxConcurrentRequests; // Number of permits for fallback semaphore @@ -116,6 +118,7 @@ protected HystrixCommandProperties(HystrixCommandKey key, HystrixCommandProperti this.executionIsolationStrategy = getProperty(propertyPrefix, key, "execution.isolation.strategy", builder.getExecutionIsolationStrategy(), default_executionIsolationStrategy); //this property name is now misleading. //TODO figure out a good way to deprecate this property name this.executionTimeoutInMilliseconds = getProperty(propertyPrefix, key, "execution.isolation.thread.timeoutInMilliseconds", builder.getExecutionIsolationThreadTimeoutInMilliseconds(), default_executionTimeoutInMilliseconds); + this.executionTimeoutEnabled = getProperty(propertyPrefix, key, "execution.timeout.enabled", builder.getExecutionTimeoutEnabled(), default_executionTimeoutEnabled); this.executionIsolationThreadInterruptOnTimeout = getProperty(propertyPrefix, key, "execution.isolation.thread.interruptOnTimeout", builder.getExecutionIsolationThreadInterruptOnTimeout(), default_executionIsolationThreadInterruptOnTimeout); this.executionIsolationSemaphoreMaxConcurrentRequests = getProperty(propertyPrefix, key, "execution.isolation.semaphore.maxConcurrentRequests", builder.getExecutionIsolationSemaphoreMaxConcurrentRequests(), default_executionIsolationSemaphoreMaxConcurrentRequests); this.fallbackIsolationSemaphoreMaxConcurrentRequests = getProperty(propertyPrefix, key, "fallback.isolation.semaphore.maxConcurrentRequests", builder.getFallbackIsolationSemaphoreMaxConcurrentRequests(), default_fallbackIsolationSemaphoreMaxConcurrentRequests); @@ -280,7 +283,18 @@ public HystrixProperty executionTimeoutInMilliseconds() { */ return executionIsolationThreadTimeoutInMilliseconds(); } - + + /** + * Whether the timeout mechanism is enabled for this command + * + * @return {@code HystrixProperty} + * + * @since 1.4.4 + */ + public HystrixProperty executionTimeoutEnabled() { + return executionTimeoutEnabled; + } + /** * Number of concurrent requests permitted to {@link HystrixCommand#getFallback()}. Requests beyond the concurrent limit will fail-fast and not attempt retrieving a fallback. * @@ -501,6 +515,7 @@ public static class Setter { private ExecutionIsolationStrategy executionIsolationStrategy = null; private Boolean executionIsolationThreadInterruptOnTimeout = null; private Integer executionTimeoutInMilliseconds = null; + private Boolean executionTimeoutEnabled = null; private Integer fallbackIsolationSemaphoreMaxConcurrentRequests = null; private Boolean fallbackEnabled = null; private Integer metricsHealthSnapshotIntervalInMilliseconds = null; @@ -561,7 +576,11 @@ public Integer getExecutionIsolationThreadTimeoutInMilliseconds() { public Integer getExecutionTimeoutInMilliseconds() { return executionTimeoutInMilliseconds; } - + + public Boolean getExecutionTimeoutEnabled() { + return executionTimeoutEnabled; + } + public Integer getFallbackIsolationSemaphoreMaxConcurrentRequests() { return fallbackIsolationSemaphoreMaxConcurrentRequests; } @@ -662,6 +681,11 @@ public Setter withExecutionTimeoutInMilliseconds(int value) { return this; } + public Setter withExecutionTimeoutEnabled(boolean value) { + this.executionTimeoutEnabled = value; + return this; + } + public Setter withFallbackIsolationSemaphoreMaxConcurrentRequests(int value) { this.fallbackIsolationSemaphoreMaxConcurrentRequests = value; return this; diff --git a/hystrix-core/src/test/java/com/netflix/hystrix/HystrixCommandPropertiesTest.java b/hystrix-core/src/test/java/com/netflix/hystrix/HystrixCommandPropertiesTest.java index 32e2e63e9..dd513b532 100644 --- a/hystrix-core/src/test/java/com/netflix/hystrix/HystrixCommandPropertiesTest.java +++ b/hystrix-core/src/test/java/com/netflix/hystrix/HystrixCommandPropertiesTest.java @@ -33,6 +33,7 @@ public class HystrixCommandPropertiesTest { /* package */static HystrixCommandProperties.Setter getUnitTestPropertiesSetter() { return new HystrixCommandProperties.Setter() .withExecutionTimeoutInMilliseconds(1000)// when an execution will be timed out + .withExecutionTimeoutEnabled(true) .withExecutionIsolationStrategy(ExecutionIsolationStrategy.THREAD) // we want thread execution by default in tests .withExecutionIsolationThreadInterruptOnTimeout(true) .withCircuitBreakerForceOpen(false) // we don't want short-circuiting by default @@ -120,6 +121,11 @@ public HystrixProperty executionTimeoutInMilliseconds() { return HystrixProperty.Factory.asProperty(builder.getExecutionTimeoutInMilliseconds()); } + @Override + public HystrixProperty executionTimeoutEnabled() { + return HystrixProperty.Factory.asProperty(builder.getExecutionTimeoutEnabled()); + } + @Override public HystrixProperty fallbackIsolationSemaphoreMaxConcurrentRequests() { return HystrixProperty.Factory.asProperty(builder.getFallbackIsolationSemaphoreMaxConcurrentRequests()); diff --git a/hystrix-core/src/test/java/com/netflix/hystrix/HystrixCommandTest.java b/hystrix-core/src/test/java/com/netflix/hystrix/HystrixCommandTest.java index 802d1b129..72299fd80 100644 --- a/hystrix-core/src/test/java/com/netflix/hystrix/HystrixCommandTest.java +++ b/hystrix-core/src/test/java/com/netflix/hystrix/HystrixCommandTest.java @@ -1845,6 +1845,23 @@ public void testTimedOutCommandDoesNotExecute() { assertEquals(2, HystrixRequestLog.getCurrentRequest().getAllExecutedCommands().size()); } + @Test + public void testDisabledTimeoutWorks() { + CommandWithDisabledTimeout cmd = new CommandWithDisabledTimeout(100, 900); + boolean result = false; + try { + result = cmd.execute(); + } catch (Throwable ex) { + ex.printStackTrace(); + fail("should not fail"); + } + + assertEquals(true, result); + assertFalse(cmd.isResponseTimedOut()); + System.out.println("CMD : " + cmd.currentRequestLog.getExecutedCommandsAsString()); + assertTrue(cmd.executionResult.getExecutionTime() >= 900); + } + @Test public void testFallbackSemaphore() { TestCircuitBreaker circuitBreaker = new TestCircuitBreaker(); @@ -4987,4 +5004,30 @@ protected Boolean run() throws Exception { return hasBeenInterrupted; } } + + private static class CommandWithDisabledTimeout extends TestHystrixCommand { + private final int latency; + + public CommandWithDisabledTimeout(int timeout, int latency) { + super(testPropsBuilder().setCommandPropertiesDefaults(HystrixCommandPropertiesTest.getUnitTestPropertiesSetter() + .withExecutionTimeoutInMilliseconds(timeout) + .withExecutionTimeoutEnabled(false))); + this.latency = latency; + } + + @Override + protected Boolean run() throws Exception { + try { + Thread.sleep(latency); + return true; + } catch (InterruptedException ex) { + return false; + } + } + + @Override + protected Boolean getFallback() { + return false; + } + } }