Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ability to disable timeout per-command and wire it in #742

Merged
merged 1 commit into from
Apr 6, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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;
}
}
}