Skip to content

Commit

Permalink
Merge pull request Netflix#742 from mattrjacobs/add-timeout-disable-flag
Browse files Browse the repository at this point in the history
Add ability to disable timeout per-command and wire it in
  • Loading branch information
mattrjacobs committed Apr 6, 2015
2 parents 8e0bcf6 + bf374c9 commit 263bb0a
Show file tree
Hide file tree
Showing 4 changed files with 80 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -542,7 +542,11 @@ public void call(Notification<? super R> n) {
}


}).lift(new HystrixObservableTimeoutOperator<R>(_self)).doOnNext(new Action1<R>() {
});
if (properties.executionTimeoutEnabled().get()) {
run = run.lift(new HystrixObservableTimeoutOperator<R>(_self));
}
run = run.doOnNext(new Action1<R>() {
@Override
public void call(R r) {
if (shouldOutputOnNextEvents()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -69,6 +70,7 @@ public abstract class HystrixCommandProperties {
private final HystrixProperty<Boolean> circuitBreakerForceClosed; // a property to allow ignoring errors and therefore never trip 'open' (ie. allow all traffic through)
private final HystrixProperty<ExecutionIsolationStrategy> executionIsolationStrategy; // Whether a command should be executed in a separate thread or not.
private final HystrixProperty<Integer> executionTimeoutInMilliseconds; // Timeout value in milliseconds for a command
private final HystrixProperty<Boolean> executionTimeoutEnabled; //Whether timeout should be triggered
private final HystrixProperty<String> executionIsolationThreadPoolKeyOverride; // What thread-pool this command should run in (if running on a separate thread).
private final HystrixProperty<Integer> executionIsolationSemaphoreMaxConcurrentRequests; // Number of permits for execution semaphore
private final HystrixProperty<Integer> fallbackIsolationSemaphoreMaxConcurrentRequests; // Number of permits for fallback semaphore
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -280,7 +283,18 @@ public HystrixProperty<Integer> executionTimeoutInMilliseconds() {
*/
return executionIsolationThreadTimeoutInMilliseconds();
}


/**
* Whether the timeout mechanism is enabled for this command
*
* @return {@code HystrixProperty<Boolean>}
*
* @since 1.4.4
*/
public HystrixProperty<Boolean> 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.
*
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -561,7 +576,11 @@ public Integer getExecutionIsolationThreadTimeoutInMilliseconds() {
public Integer getExecutionTimeoutInMilliseconds() {
return executionTimeoutInMilliseconds;
}


public Boolean getExecutionTimeoutEnabled() {
return executionTimeoutEnabled;
}

public Integer getFallbackIsolationSemaphoreMaxConcurrentRequests() {
return fallbackIsolationSemaphoreMaxConcurrentRequests;
}
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -120,6 +121,11 @@ public HystrixProperty<Integer> executionTimeoutInMilliseconds() {
return HystrixProperty.Factory.asProperty(builder.getExecutionTimeoutInMilliseconds());
}

@Override
public HystrixProperty<Boolean> executionTimeoutEnabled() {
return HystrixProperty.Factory.asProperty(builder.getExecutionTimeoutEnabled());
}

@Override
public HystrixProperty<Integer> fallbackIsolationSemaphoreMaxConcurrentRequests() {
return HystrixProperty.Factory.asProperty(builder.getFallbackIsolationSemaphoreMaxConcurrentRequests());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -4987,4 +5004,30 @@ protected Boolean run() throws Exception {
return hasBeenInterrupted;
}
}

private static class CommandWithDisabledTimeout extends TestHystrixCommand<Boolean> {
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;
}
}
}

0 comments on commit 263bb0a

Please sign in to comment.