From 7051e68f189025de1210daa3d6772b79f75972a0 Mon Sep 17 00:00:00 2001 From: Ben Christensen Date: Thu, 3 Jan 2013 21:16:25 -0800 Subject: [PATCH 1/8] Update Javadocs for plugin configuration. This makes the Javadocs comply with changes done in https://github.com/Netflix/Hystrix/commit/bfb66969baaa38a16e9dfce872e6ad6a4012d443 --- .../HystrixConcurrencyStrategy.java | 23 ++-------------- .../eventnotifier/HystrixEventNotifier.java | 27 +++---------------- .../metrics/HystrixMetricsPublisher.java | 24 ++--------------- .../properties/HystrixPropertiesStrategy.java | 23 ++-------------- 4 files changed, 9 insertions(+), 88 deletions(-) diff --git a/hystrix-core/src/main/java/com/netflix/hystrix/strategy/concurrency/HystrixConcurrencyStrategy.java b/hystrix-core/src/main/java/com/netflix/hystrix/strategy/concurrency/HystrixConcurrencyStrategy.java index b4a4aa2e1..c2ff67855 100644 --- a/hystrix-core/src/main/java/com/netflix/hystrix/strategy/concurrency/HystrixConcurrencyStrategy.java +++ b/hystrix-core/src/main/java/com/netflix/hystrix/strategy/concurrency/HystrixConcurrencyStrategy.java @@ -37,27 +37,8 @@ * For example, every {@link Callable} executed by {@link HystrixCommand} will call {@link #wrapCallable(Callable)} to give a chance for custom implementations to decorate the {@link Callable} with * additional behavior. *

- * Custom implementations of this interface can be used to override default behavior via 2 mechanisms: - *

- * 1) Injection - *

- * Implementations can be injected into {@link HystrixCommand} and {@link HystrixCollapser} implementation constructors. - *

- * 2) Plugin - *

- * Using {@link HystrixPlugins#registerConcurrencyStrategy} an implementation can be registered globally to take precedence and override all other implementations. - *

- * The order of precedence is: - *

    - *
  1. plugin registered globally using {@link HystrixPlugins#registerConcurrencyStrategy}
    2. - *
  2. injected via {@link HystrixCommand} and {@link HystrixCollapser} constructors
    3. - *
  3. default implementation {@link HystrixConcurrencyStrategyDefault}
    4. - *
- *

- * The injection approach is effective for {@link HystrixCommand} and {@link HystrixCollapser} implementations where you wish to have a different default strategy without - * overriding all implementations. It is also useful when distributing a library where static override should not be used. - *

- * The globally registered plugin is useful when using commands from 3rd party libraries and you want to override the strategy for all implementations in your entire system. + * See {@link HystrixPlugins} or the Hystrix GitHub Wiki for information on configuring plugins: https://github.com/Netflix/Hystrix/wiki/Plugins. */ public abstract class HystrixConcurrencyStrategy { diff --git a/hystrix-core/src/main/java/com/netflix/hystrix/strategy/eventnotifier/HystrixEventNotifier.java b/hystrix-core/src/main/java/com/netflix/hystrix/strategy/eventnotifier/HystrixEventNotifier.java index c9feab7c1..d8d5623c9 100644 --- a/hystrix-core/src/main/java/com/netflix/hystrix/strategy/eventnotifier/HystrixEventNotifier.java +++ b/hystrix-core/src/main/java/com/netflix/hystrix/strategy/eventnotifier/HystrixEventNotifier.java @@ -17,37 +17,16 @@ import java.util.List; -import com.netflix.hystrix.HystrixCollapser; -import com.netflix.hystrix.HystrixCommand; import com.netflix.hystrix.HystrixCommandKey; import com.netflix.hystrix.HystrixCommandProperties.ExecutionIsolationStrategy; -import com.netflix.hystrix.HystrixEventType; import com.netflix.hystrix.strategy.HystrixPlugins; +import com.netflix.hystrix.HystrixEventType; /** * Abstract EventNotifier that allows receiving notifications for different events with default implementations. *

- * Custom implementations of this interface can be used to override default behavior via 2 mechanisms: - *

- * 1) Injection - *

- * Implementations can be injected into {@link HystrixCommand} and {@link HystrixCollapser} implementation constructors. - *

- * 2) Plugin - *

- * Using {@link HystrixPlugins#registerEventNotifier} an implementation can be registered globally to take precedence and override all other implementations. - *

- * The order of precedence is: - *

    - *
  1. plugin registered globally using {@link HystrixPlugins#registerEventNotifier}
    2. - *
  2. injected via {@link HystrixCommand} and {@link HystrixCollapser} constructors
    3. - *
  3. default implementation {@link HystrixEventNotifierDefault}
    4. - *
- *

- * The injection approach is effective for {@link HystrixCommand} and {@link HystrixCollapser} implementations where you wish to have a different default mechanism for event notification without - * overriding all implementations. It is also useful when distributing a library where static override should not be used. - *

- * The globally registered plugin is useful when using commands from 3rd party libraries and you want to override how event notifications are performed for all implementations in your entire system. + * See {@link HystrixPlugins} or the Hystrix GitHub Wiki for information on configuring plugins: https://github.com/Netflix/Hystrix/wiki/Plugins. */ public abstract class HystrixEventNotifier { diff --git a/hystrix-core/src/main/java/com/netflix/hystrix/strategy/metrics/HystrixMetricsPublisher.java b/hystrix-core/src/main/java/com/netflix/hystrix/strategy/metrics/HystrixMetricsPublisher.java index 4ee903d75..23cd06f92 100644 --- a/hystrix-core/src/main/java/com/netflix/hystrix/strategy/metrics/HystrixMetricsPublisher.java +++ b/hystrix-core/src/main/java/com/netflix/hystrix/strategy/metrics/HystrixMetricsPublisher.java @@ -16,7 +16,6 @@ package com.netflix.hystrix.strategy.metrics; import com.netflix.hystrix.HystrixCircuitBreaker; -import com.netflix.hystrix.HystrixCollapser; import com.netflix.hystrix.HystrixCommand; import com.netflix.hystrix.HystrixCommandGroupKey; import com.netflix.hystrix.HystrixCommandKey; @@ -33,27 +32,8 @@ * exposed, published or otherwise retrievable by external systems such as Servo (https://github.com/Netflix/servo) * for monitoring and statistical purposes. *

- * Custom implementations of this interface can be used to override default behavior via 2 mechanisms: - *

- * 1) Injection - *

- * Implementations can be injected into {@link HystrixCommand} and {@link HystrixCollapser} implementation constructors. - *

- * 2) Plugin - *

- * Using {@link HystrixPlugins#registerMetricsPublisher} an implementation can be registered globally to take precedence and override all other implementations. - *

- * The order of precedence is: - *

    - *
  1. plugin registered globally using {@link HystrixPlugins#registerMetricsPublisher}
    2. - *
  2. injected via {@link HystrixCommand} and {@link HystrixCollapser} constructors
    3. - *
  3. default implementation {@link HystrixMetricsPublisherDefault}
    4. - *
- *

- * The injection approach is effective for {@link HystrixCommand} and {@link HystrixCollapser} implementations where you wish to have a different default mechanism for publishing metrics without - * overriding all implementations. It is also useful when distributing a library where static override should not be used. - *

- * The globally registered plugin is useful when using commands from 3rd party libraries and you want to override how properties are defined for all implementations in your entire system. + * See {@link HystrixPlugins} or the Hystrix GitHub Wiki for information on configuring plugins: https://github.com/Netflix/Hystrix/wiki/Plugins. */ public abstract class HystrixMetricsPublisher { diff --git a/hystrix-core/src/main/java/com/netflix/hystrix/strategy/properties/HystrixPropertiesStrategy.java b/hystrix-core/src/main/java/com/netflix/hystrix/strategy/properties/HystrixPropertiesStrategy.java index 6cd08bbe3..1e9d6dae9 100644 --- a/hystrix-core/src/main/java/com/netflix/hystrix/strategy/properties/HystrixPropertiesStrategy.java +++ b/hystrix-core/src/main/java/com/netflix/hystrix/strategy/properties/HystrixPropertiesStrategy.java @@ -29,27 +29,8 @@ /** * Abstract class with default implementations of factory methods for properties used by various components of Hystrix. *

- * Custom implementations of this interface can be used to override default behavior via 2 mechanisms: - *

- * 1) Injection - *

- * Implementations can be injected into {@link HystrixCommand} and {@link HystrixCollapser} implementation constructors. - *

- * 2) Plugin - *

- * Using {@link HystrixPlugins#registerPropertiesStrategy} an implementation can be registered globally to take precedence and override all other implementations. - *

- * The order of precedence is: - *

    - *
  1. plugin registered globally using {@link HystrixPlugins#registerPropertiesStrategy}
    2. - *
  2. injected via {@link HystrixCommand} and {@link HystrixCollapser} constructors
    3. - *
  3. default implementation {@link HystrixPropertiesStrategyDefault}
    4. - *
- *

- * The injection approach is effective for {@link HystrixCommand} and {@link HystrixCollapser} implementations where you wish to have a different default mechanism for setting properties without - * overriding all implementations. It is also useful when distributing a library where static override should not be used. - *

- * The globally registered plugin is useful when using commands from 3rd party libraries and you want to override how properties are defined for all implementations in your entire system. + * See {@link HystrixPlugins} or the Hystrix GitHub Wiki for information on configuring plugins: https://github.com/Netflix/Hystrix/wiki/Plugins. */ public abstract class HystrixPropertiesStrategy { From 031deb92cae9b00c8530d01435d1b4e4266295f5 Mon Sep 17 00:00:00 2001 From: Ben Christensen Date: Thu, 3 Jan 2013 21:37:38 -0800 Subject: [PATCH 2/8] Remove TODOs made invalid by HystrixPlugin changes Plugins can not be modified after registration so caching is okay. --- .../src/main/java/com/netflix/hystrix/HystrixCommand.java | 6 ------ 1 file changed, 6 deletions(-) diff --git a/hystrix-core/src/main/java/com/netflix/hystrix/HystrixCommand.java b/hystrix-core/src/main/java/com/netflix/hystrix/HystrixCommand.java index 20a060a8d..fa81d039a 100755 --- a/hystrix-core/src/main/java/com/netflix/hystrix/HystrixCommand.java +++ b/hystrix-core/src/main/java/com/netflix/hystrix/HystrixCommand.java @@ -229,8 +229,6 @@ private HystrixCommand(HystrixCommandGroupKey group, HystrixCommandKey key, Hyst * Metrics initialization */ if (metrics == null) { - // TODO this caches the first time it's loaded and will thus miss changes to threadPoolKey, properties and eventNotifier - // We need a better way of handling this now that we have HystrixPlugins this.metrics = HystrixCommandMetrics.getInstance(this.commandKey, this.commandGroup, this.properties); } else { this.metrics = metrics; @@ -242,8 +240,6 @@ private HystrixCommand(HystrixCommandGroupKey group, HystrixCommandKey key, Hyst if (this.properties.circuitBreakerEnabled().get()) { if (circuitBreaker == null) { // get the default implementation of HystrixCircuitBreaker - // TODO this caches the first time it's loaded and will thus miss changes to properties - // We need a better way of handling this now that we have HystrixPlugins this.circuitBreaker = HystrixCircuitBreaker.Factory.getInstance(this.commandKey, this.commandGroup, this.properties, this.metrics); } else { this.circuitBreaker = circuitBreaker; @@ -253,8 +249,6 @@ private HystrixCommand(HystrixCommandGroupKey group, HystrixCommandKey key, Hyst } /* strategy: HystrixMetricsPublisherCommand */ - // TODO this caches the first time it's loaded and will thus miss changes to properties - // We need a better way of handling this now that we have HystrixPlugins HystrixMetricsPublisherFactory.createOrRetrievePublisherForCommand(this.commandKey, this.commandGroup, this.metrics, this.circuitBreaker, this.properties); /* From ad238f9f409bced22c6d04270cdbb068c7d2171d Mon Sep 17 00:00:00 2001 From: Ben Christensen Date: Thu, 3 Jan 2013 21:40:25 -0800 Subject: [PATCH 3/8] Change Throwable to Exception Generally not good practice to be catching Throwable so changing to Exception. --- .../java/com/netflix/hystrix/HystrixCommand.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/hystrix-core/src/main/java/com/netflix/hystrix/HystrixCommand.java b/hystrix-core/src/main/java/com/netflix/hystrix/HystrixCommand.java index fa81d039a..6f255484d 100755 --- a/hystrix-core/src/main/java/com/netflix/hystrix/HystrixCommand.java +++ b/hystrix-core/src/main/java/com/netflix/hystrix/HystrixCommand.java @@ -427,7 +427,7 @@ public final R execute() { } } - } catch (Throwable e) { + } catch (Exception e) { if (e instanceof HystrixBadRequestException) { throw (HystrixBadRequestException) e; } @@ -756,7 +756,7 @@ private R executeCommand() { * HystrixBadRequestException is treated differently and allowed to propagate without any stats tracking or fallback logic */ throw e; - } catch (Throwable e) { + } catch (Exception e) { if (isCommandTimedOut.get()) { // http://jira/browse/API-4905 HystrixCommand: Error/Timeout Double-count if both occur // this means we have already timed out then we don't count this error stat and we just return @@ -1038,7 +1038,7 @@ private R getFallbackOrThrowException(HystrixEventType eventType, FailureType fa /** * @throws HystrixRuntimeException */ - private R getFallbackOrThrowException(HystrixEventType eventType, FailureType failureType, String message, Throwable e) { + private R getFallbackOrThrowException(HystrixEventType eventType, FailureType failureType, String message, Exception e) { try { // retrieve the fallback R fallback = getFallbackWithProtection(); @@ -1052,7 +1052,7 @@ private R getFallbackOrThrowException(HystrixEventType eventType, FailureType fa // record the executionResult executionResult = executionResult.addEvents(eventType); throw new HystrixRuntimeException(failureType, this.getClass(), getLogMessagePrefix() + " " + message + " and no fallback available.", e, fe); - } catch (Throwable fe) { + } catch (Exception fe) { logger.error("Error retrieving fallback for HystrixCommand. ", fe); metrics.markFallbackFailure(); // record the executionResult @@ -1085,7 +1085,7 @@ private R getFallbackOrThrowException(HystrixEventType eventType, FailureType fa private static class ExecutionResult { private final List events; private final int executionTime; - private final Throwable exception; + private final Exception exception; private ExecutionResult(HystrixEventType... events) { this(Arrays.asList(events), -1, null); @@ -1095,11 +1095,11 @@ public ExecutionResult setExecutionTime(int executionTime) { return new ExecutionResult(events, executionTime, exception); } - public ExecutionResult setException(Throwable e) { + public ExecutionResult setException(Exception e) { return new ExecutionResult(events, executionTime, e); } - private ExecutionResult(List events, int executionTime, Throwable e) { + private ExecutionResult(List events, int executionTime, Exception e) { // we are safe assigning the List reference instead of deep-copying // because we control the original list in 'newEvent' this.events = events; From ce3d6263159c628c1703c6e1ef52dddd7dd69eb4 Mon Sep 17 00:00:00 2001 From: Ben Christensen Date: Thu, 3 Jan 2013 21:41:53 -0800 Subject: [PATCH 4/8] Remove unused code. --- .../src/main/java/com/netflix/hystrix/HystrixCommand.java | 6 ------ 1 file changed, 6 deletions(-) diff --git a/hystrix-core/src/main/java/com/netflix/hystrix/HystrixCommand.java b/hystrix-core/src/main/java/com/netflix/hystrix/HystrixCommand.java index 6f255484d..edff1926b 100755 --- a/hystrix-core/src/main/java/com/netflix/hystrix/HystrixCommand.java +++ b/hystrix-core/src/main/java/com/netflix/hystrix/HystrixCommand.java @@ -1239,7 +1239,6 @@ private boolean isRequestCachingEnabled() { private class QueuedExecutionFuture implements CommandFuture { private final ThreadPoolExecutor executor; private final Callable callable; - private final HystrixCommand command; private final CountDownLatch actualResponseReceived = new CountDownLatch(1); private final AtomicBoolean actualFutureExecuted = new AtomicBoolean(false); private volatile R result; // the result of the get() @@ -1249,7 +1248,6 @@ private class QueuedExecutionFuture implements CommandFuture { private final AtomicBoolean started = new AtomicBoolean(false); public QueuedExecutionFuture(HystrixCommand command, ThreadPoolExecutor executor, Callable callable) { - this.command = command; this.executor = executor; this.callable = callable; } @@ -1533,10 +1531,6 @@ public int getNumberOfPermitsUsed() { return count.get(); } - public int getNumberOfPermits() { - return numberOfPermits.get(); - } - } private String getLogMessagePrefix() { From 6d34231ceeadee3ced17bea7bc877498aa5614ab Mon Sep 17 00:00:00 2001 From: Ben Christensen Date: Thu, 3 Jan 2013 22:52:51 -0800 Subject: [PATCH 5/8] jettyRun support for running webapps via gradle --- hystrix-dashboard/build.gradle | 5 +++++ hystrix-examples-webapp/build.gradle | 7 +++++++ 2 files changed, 12 insertions(+) diff --git a/hystrix-dashboard/build.gradle b/hystrix-dashboard/build.gradle index 65603c914..4c96e5732 100644 --- a/hystrix-dashboard/build.gradle +++ b/hystrix-dashboard/build.gradle @@ -1,5 +1,6 @@ apply plugin: 'java' apply plugin: 'war' +apply plugin: 'jetty' dependencies { compile 'javax.servlet:servlet-api:2.5' @@ -7,3 +8,7 @@ apply plugin: 'war' compile 'log4j:log4j:1.2.17' compile 'org.slf4j:slf4j-api:1.7.0' } + +jettyRun { + httpPort = 7979 +} diff --git a/hystrix-examples-webapp/build.gradle b/hystrix-examples-webapp/build.gradle index 9c8124950..52c615f92 100644 --- a/hystrix-examples-webapp/build.gradle +++ b/hystrix-examples-webapp/build.gradle @@ -1,5 +1,6 @@ apply plugin: 'java' apply plugin: 'war' +apply plugin: 'jetty' dependencies { compile project(':hystrix-core') @@ -7,3 +8,9 @@ apply plugin: 'war' compile project(':hystrix-contrib:hystrix-request-servlet') compile project(':hystrix-contrib:hystrix-metrics-event-stream') } + +jettyRun { + httpPort = 8989 +} + + From 390026a82d0b46c4524667c974581c0dd1a5e93e Mon Sep 17 00:00:00 2001 From: Ben Christensen Date: Fri, 4 Jan 2013 21:30:59 -0800 Subject: [PATCH 6/8] HystrixCommand Execution Hooks Execution Hooks via Plugin https://github.com/Netflix/Hystrix/issues/10 --- .../com/netflix/hystrix/HystrixCommand.java | 942 +++++++++++++++++- .../hystrix/strategy/HystrixPlugins.java | 42 + .../HystrixCommandExecutionHook.java | 170 ++++ .../HystrixCommandExecutionHookDefault.java | 35 + .../strategy/executionhook/package-info.java | 21 + 5 files changed, 1196 insertions(+), 14 deletions(-) create mode 100644 hystrix-core/src/main/java/com/netflix/hystrix/strategy/executionhook/HystrixCommandExecutionHook.java create mode 100644 hystrix-core/src/main/java/com/netflix/hystrix/strategy/executionhook/HystrixCommandExecutionHookDefault.java create mode 100644 hystrix-core/src/main/java/com/netflix/hystrix/strategy/executionhook/package-info.java diff --git a/hystrix-core/src/main/java/com/netflix/hystrix/HystrixCommand.java b/hystrix-core/src/main/java/com/netflix/hystrix/HystrixCommand.java index edff1926b..0728b7545 100755 --- a/hystrix-core/src/main/java/com/netflix/hystrix/HystrixCommand.java +++ b/hystrix-core/src/main/java/com/netflix/hystrix/HystrixCommand.java @@ -62,6 +62,7 @@ import com.netflix.hystrix.strategy.concurrency.HystrixContextRunnable; import com.netflix.hystrix.strategy.concurrency.HystrixRequestContext; import com.netflix.hystrix.strategy.eventnotifier.HystrixEventNotifier; +import com.netflix.hystrix.strategy.executionhook.HystrixCommandExecutionHook; import com.netflix.hystrix.strategy.metrics.HystrixMetricsPublisherFactory; import com.netflix.hystrix.strategy.properties.HystrixPropertiesFactory; import com.netflix.hystrix.strategy.properties.HystrixPropertiesStrategy; @@ -102,15 +103,13 @@ public abstract class HystrixCommand implements HystrixExecutable { private final TryableSemaphore fallbackSemaphoreOverride; /* each circuit has a semaphore to restrict concurrent fallback execution */ private static final ConcurrentHashMap fallbackSemaphorePerCircuit = new ConcurrentHashMap(); - /* END FALLBACK Semaphore */ /* EXECUTION Semaphore */ private final TryableSemaphore executionSemaphoreOverride; /* each circuit has a semaphore to restrict concurrent fallback execution */ private static final ConcurrentHashMap executionSemaphorePerCircuit = new ConcurrentHashMap(); - - /* END FALLBACK Semaphore */ + /* END EXECUTION Semaphore */ /* used to track whenever the user invokes the command using execute(), queue() or fireAndForget() ... also used to know if execution has begun */ private AtomicLong invocationStartTime = new AtomicLong(-1); @@ -125,6 +124,7 @@ public abstract class HystrixCommand implements HystrixExecutable { */ private final HystrixEventNotifier eventNotifier; private final HystrixConcurrencyStrategy concurrencyStrategy; + private final HystrixCommandExecutionHook executionHook; /** * Construct a {@link HystrixCommand} with defined {@link HystrixCommandGroupKey}. @@ -154,7 +154,7 @@ protected HystrixCommand(HystrixCommandGroupKey group) { */ protected HystrixCommand(Setter setter) { // use 'null' to specify use the default - this(setter.groupKey, setter.commandKey, setter.threadPoolKey, null, null, setter.commandPropertiesDefaults, setter.threadPoolPropertiesDefaults, null, null, null, null); + this(setter.groupKey, setter.commandKey, setter.threadPoolKey, null, null, setter.commandPropertiesDefaults, setter.threadPoolPropertiesDefaults, null, null, null, null, null); } /** @@ -167,7 +167,7 @@ protected HystrixCommand(Setter setter) { private HystrixCommand(HystrixCommandGroupKey group, HystrixCommandKey key, HystrixThreadPoolKey threadPoolKey, HystrixCircuitBreaker circuitBreaker, HystrixThreadPool threadPool, HystrixCommandProperties.Setter commandPropertiesDefaults, HystrixThreadPoolProperties.Setter threadPoolPropertiesDefaults, HystrixCommandMetrics metrics, TryableSemaphore fallbackSemaphore, TryableSemaphore executionSemaphore, - HystrixPropertiesStrategy propertiesStrategy) { + HystrixPropertiesStrategy propertiesStrategy, HystrixCommandExecutionHook executionHook) { /* * CommandGroup initialization */ @@ -251,6 +251,14 @@ private HystrixCommand(HystrixCommandGroupKey group, HystrixCommandKey key, Hyst /* strategy: HystrixMetricsPublisherCommand */ HystrixMetricsPublisherFactory.createOrRetrievePublisherForCommand(this.commandKey, this.commandGroup, this.metrics, this.circuitBreaker, this.properties); + /* strategy: HystrixCommandExecutionHook */ + if (executionHook == null) { + this.executionHook = HystrixPlugins.getInstance().getCommandExecutionHook(); + } else { + // used for unit testing + this.executionHook = executionHook; + } + /* * ThreadPool initialization */ @@ -269,7 +277,6 @@ private HystrixCommand(HystrixCommandGroupKey group, HystrixCommandKey key, Hyst /* setup the request cache for this instance */ this.requestCache = HystrixRequestCache.getInstance(this.commandKey, this.concurrencyStrategy); - } private static String getDefaultNameFromClass(@SuppressWarnings("rawtypes") Class cls) { @@ -405,6 +412,9 @@ public final R execute() { } } + // mark that we're starting execution on the ExecutionHook + executionHook.onStart(this); + /* determine if we're allowed to execute */ if (!circuitBreaker.allowRequest()) { // record that we are returning a short-circuited fallback @@ -418,8 +428,7 @@ public final R execute() { return queueInThread().get(); } else { try { - R r = executeWithSemaphore(); - return r; + return executeWithSemaphore(); } catch (RuntimeException e) { // count that we're throwing an exception and rethrow metrics.markExceptionThrown(); @@ -461,6 +470,7 @@ private R executeWithSemaphore() { // we want to run it synchronously R response = executeCommand(); + response = executionHook.onSuccess(this, response); // put in cache if (isRequestCachingEnabled()) { requestCache.putIfAbsent(getCacheKey(), asFutureForCache(response)); @@ -516,6 +526,9 @@ public final Future queue() { } } + // mark that we're starting execution on the ExecutionHook + executionHook.onStart(this); + /* determine if we're allowed to execute */ if (!circuitBreaker.allowRequest()) { // record that we are returning a short-circuited fallback @@ -613,6 +626,7 @@ public ExecutionResult getExecutionResult() { // execute outside of future so that fireAndForget will still work (ie. someone calls queue() but not get()) and so that multiple requests can be deduped through request caching R r = executeCommand(); + r = executionHook.onSuccess(this, r); value.set(r); return responseFuture; @@ -643,6 +657,9 @@ private Future queueInThread() { // a snapshot of time so the thread can measure how long it waited before executing final long timeBeforeExecution = System.currentTimeMillis(); + + final HystrixCommand _this = this; + // wrap the synchronous execute() method in a Callable and execute in the threadpool QueuedExecutionFuture future = new QueuedExecutionFuture(this, threadPool.getExecutor(), new HystrixContextCallable(new Callable() { @@ -653,6 +670,9 @@ public R call() throws Exception { // anywhere along the way the exception knows who the calling thread is and can include it in the stacktrace ExceptionThreadingUtility.assignCallingThread(callingThread); + // execution hook + executionHook.onThreadStart(_this); + // count the active thread threadPool.markThreadExecution(); @@ -684,7 +704,7 @@ public R call() throws Exception { // execute the command R r = executeCommand(); - return r; + return executionHook.onSuccess(_this, r); } catch (Exception e) { if (!isCommandTimedOut.get()) { // count (if we didn't timeout) that we are throwing an exception and re-throw it @@ -693,6 +713,11 @@ public R call() throws Exception { throw e; } finally { threadPool.markThreadCompletion(); + try { + executionHook.onThreadComplete(_this); + } catch (Exception e) { + logger.warn("ExecutionHook.onThreadComplete threw an exception that will be ignored.", e); + } } } })); @@ -735,7 +760,8 @@ private R executeCommand() { /* capture start time for logging */ long startTime = System.currentTimeMillis(); try { - R response = run(); + executionHook.onRunStart(this); + R response = executionHook.onRunSuccess(this, run()); long duration = System.currentTimeMillis() - startTime; metrics.addCommandExecutionTime(duration); @@ -752,11 +778,29 @@ private R executeCommand() { return response; } } catch (HystrixBadRequestException e) { + try { + Exception decorated = executionHook.onRunError(this, e); + if (decorated instanceof HystrixBadRequestException) { + e = (HystrixBadRequestException) decorated; + } else { + logger.warn("ExecutionHook.endRunFailure returned an exception that was not an instance of HystrixBadRequestException so will be ignored.", decorated); + } + throw e; + } catch (Exception hookException) { + logger.warn("Error calling ExecutionHook.endRunFailure", hookException); + } + /* * HystrixBadRequestException is treated differently and allowed to propagate without any stats tracking or fallback logic */ throw e; } catch (Exception e) { + try { + e = executionHook.onRunError(this, e); + } catch (Exception hookException) { + logger.warn("Error calling ExecutionHook.endRunFailure", hookException); + } + if (isCommandTimedOut.get()) { // http://jira/browse/API-4905 HystrixCommand: Error/Timeout Double-count if both occur // this means we have already timed out then we don't count this error stat and we just return @@ -797,7 +841,18 @@ private R getFallbackWithProtection() { // acquire a permit if (fallbackSemaphore.tryAcquire()) { try { - return getFallback(); + executionHook.onFallbackStart(this); + return executionHook.onFallbackSuccess(this, getFallback()); + } catch (RuntimeException e) { + Exception decorated = executionHook.onFallbackError(this, e); + if (decorated instanceof RuntimeException) { + e = (RuntimeException) decorated; + } else { + logger.warn("ExecutionHook.onFallbackError returned an exception that was not an instance of RuntimeException so will be ignored.", decorated); + } + + // re-throw to calling method + throw e; } finally { fallbackSemaphore.release(); } @@ -1046,17 +1101,33 @@ private R getFallbackOrThrowException(HystrixEventType eventType, FailureType fa metrics.markFallbackSuccess(); // record the executionResult executionResult = executionResult.addEvents(eventType, HystrixEventType.FALLBACK_SUCCESS); - return fallback; + return executionHook.onSuccess(this, fallback); } catch (UnsupportedOperationException fe) { logger.debug("No fallback for HystrixCommand. ", fe); // debug only since we're throwing the exception and someone higher will do something with it // record the executionResult executionResult = executionResult.addEvents(eventType); + + /* executionHook for all errors */ + try { + e = executionHook.onError(this, failureType, e); + } catch (Exception hookException) { + logger.warn("Error calling ExecutionHook.onError", hookException); + } + throw new HystrixRuntimeException(failureType, this.getClass(), getLogMessagePrefix() + " " + message + " and no fallback available.", e, fe); } catch (Exception fe) { logger.error("Error retrieving fallback for HystrixCommand. ", fe); metrics.markFallbackFailure(); // record the executionResult executionResult = executionResult.addEvents(eventType, HystrixEventType.FALLBACK_FAILURE); + + /* executionHook for all errors */ + try { + e = executionHook.onError(this, failureType, e); + } catch (Exception hookException) { + logger.warn("Error calling ExecutionHook.onError", hookException); + } + throw new HystrixRuntimeException(failureType, this.getClass(), getLogMessagePrefix() + " " + message + " and failed retrieving fallback.", e, fe); } finally { // record that we're completed (to handle non-successful events we do it here as well as at the end of executeCommand @@ -1704,6 +1775,7 @@ public void testExecutionSuccess() { assertEquals(0, command.builder.metrics.getHealthCounts().getErrorPercentage()); assertEquals(1, HystrixRequestLog.getCurrentRequest().getExecutedCommands().size()); + } catch (Exception e) { e.printStackTrace(); fail("We received an exception."); @@ -4231,6 +4303,749 @@ public void testCheckedException() { assertEquals(1, circuitBreaker.metrics.getRollingCount(HystrixRollingNumberEvent.EXCEPTION_THROWN)); } + /** + * Execution hook on successful execution + */ + @Test + public void testExecutionHookSuccessfulCommand() { + /* test with execute() */ + TestHystrixCommand command = new SuccessfulTestCommand(); + command.execute(); + + // the run() method should run as we're not short-circuited or rejected + assertEquals(1, command.builder.executionHook.startRun.get()); + // we expect a successful response from run() + assertNotNull(command.builder.executionHook.runSuccessResponse); + // we do not expect an exception + assertNull(command.builder.executionHook.runFailureException); + + // the fallback() method should not be run as we were successful + assertEquals(0, command.builder.executionHook.startFallback.get()); + // null since it didn't run + assertNull(command.builder.executionHook.fallbackSuccessResponse); + // null since it didn't run + assertNull(command.builder.executionHook.fallbackFailureException); + + // the execute() method was used + assertEquals(1, command.builder.executionHook.startExecute.get()); + // we should have a response from execute() since run() succeeded + assertNotNull(command.builder.executionHook.endExecuteSuccessResponse); + // we should not have an exception since run() succeeded + assertNull(command.builder.executionHook.endExecuteFailureException); + + // thread execution + assertEquals(1, command.builder.executionHook.threadStart.get()); + assertEquals(1, command.builder.executionHook.threadComplete.get()); + + /* test with queue() */ + command = new SuccessfulTestCommand(); + try { + command.queue().get(); + } catch (Exception e) { + throw new RuntimeException(e); + } + + // the run() method should run as we're not short-circuited or rejected + assertEquals(1, command.builder.executionHook.startRun.get()); + // we expect a successful response from run() + assertNotNull(command.builder.executionHook.runSuccessResponse); + // we do not expect an exception + assertNull(command.builder.executionHook.runFailureException); + + // the fallback() method should not be run as we were successful + assertEquals(0, command.builder.executionHook.startFallback.get()); + // null since it didn't run + assertNull(command.builder.executionHook.fallbackSuccessResponse); + // null since it didn't run + assertNull(command.builder.executionHook.fallbackFailureException); + + // the queue() method was used + assertEquals(1, command.builder.executionHook.startExecute.get()); + // we should have a response from queue() since run() succeeded + assertNotNull(command.builder.executionHook.endExecuteSuccessResponse); + // we should not have an exception since run() succeeded + assertNull(command.builder.executionHook.endExecuteFailureException); + + // thread execution + assertEquals(1, command.builder.executionHook.threadStart.get()); + assertEquals(1, command.builder.executionHook.threadComplete.get()); + } + + /** + * Execution hook on successful execution with "fire and forget" approach + */ + @Test + public void testExecutionHookSuccessfulCommandViaFireAndForget() { + TestHystrixCommand command = new SuccessfulTestCommand(); + try { + // do not block on "get()" ... fire this asynchronously + command.queue(); + } catch (Exception e) { + throw new RuntimeException(e); + } + + // wait for command to execute without calling get on the future + while (!command.isExecutionComplete()) { + try { + Thread.sleep(10); + } catch (InterruptedException e) { + throw new RuntimeException("interrupted"); + } + } + + /* + * All the hooks should still work even though we didn't call get() on the future + */ + + // the run() method should run as we're not short-circuited or rejected + assertEquals(1, command.builder.executionHook.startRun.get()); + // we expect a successful response from run() + assertNotNull(command.builder.executionHook.runSuccessResponse); + // we do not expect an exception + assertNull(command.builder.executionHook.runFailureException); + + // the fallback() method should not be run as we were successful + assertEquals(0, command.builder.executionHook.startFallback.get()); + // null since it didn't run + assertNull(command.builder.executionHook.fallbackSuccessResponse); + // null since it didn't run + assertNull(command.builder.executionHook.fallbackFailureException); + + // the queue() method was used + assertEquals(1, command.builder.executionHook.startExecute.get()); + // we should have a response from queue() since run() succeeded + assertNotNull(command.builder.executionHook.endExecuteSuccessResponse); + // we should not have an exception since run() succeeded + assertNull(command.builder.executionHook.endExecuteFailureException); + + // thread execution + assertEquals(1, command.builder.executionHook.threadStart.get()); + assertEquals(1, command.builder.executionHook.threadComplete.get()); + } + + /** + * Execution hook on successful execution with multiple get() calls to Future + */ + @Test + public void testExecutionHookSuccessfulCommandWithMultipleGetsOnFuture() { + TestHystrixCommand command = new SuccessfulTestCommand(); + try { + Future f = command.queue(); + f.get(); + f.get(); + f.get(); + f.get(); + } catch (Exception e) { + throw new RuntimeException(e); + } + + /* + * Despite multiple calls to get() we should only have 1 call to the hooks. + */ + + // the run() method should run as we're not short-circuited or rejected + assertEquals(1, command.builder.executionHook.startRun.get()); + // we expect a successful response from run() + assertNotNull(command.builder.executionHook.runSuccessResponse); + // we do not expect an exception + assertNull(command.builder.executionHook.runFailureException); + + // the fallback() method should not be run as we were successful + assertEquals(0, command.builder.executionHook.startFallback.get()); + // null since it didn't run + assertNull(command.builder.executionHook.fallbackSuccessResponse); + // null since it didn't run + assertNull(command.builder.executionHook.fallbackFailureException); + + // the queue() method was used + assertEquals(1, command.builder.executionHook.startExecute.get()); + // we should have a response from queue() since run() succeeded + assertNotNull(command.builder.executionHook.endExecuteSuccessResponse); + // we should not have an exception since run() succeeded + assertNull(command.builder.executionHook.endExecuteFailureException); + + // thread execution + assertEquals(1, command.builder.executionHook.threadStart.get()); + assertEquals(1, command.builder.executionHook.threadComplete.get()); + } + + /** + * Execution hook on failed execution without a fallback + */ + @Test + public void testExecutionHookRunFailureWithoutFallback() { + /* test with execute() */ + TestHystrixCommand command = new UnknownFailureTestCommandWithoutFallback(); + try { + command.execute(); + fail("Expecting exception"); + } catch (Exception e) { + // ignore + } + + // the run() method should run as we're not short-circuited or rejected + assertEquals(1, command.builder.executionHook.startRun.get()); + // we should not have a response + assertNull(command.builder.executionHook.runSuccessResponse); + // we should have an exception + assertNotNull(command.builder.executionHook.runFailureException); + + // the fallback() method should be run since run() failed + assertEquals(1, command.builder.executionHook.startFallback.get()); + // no response since fallback is not implemented + assertNull(command.builder.executionHook.fallbackSuccessResponse); + // not null since it's not implemented and throws an exception + assertNotNull(command.builder.executionHook.fallbackFailureException); + + // the execute() method was used + assertEquals(1, command.builder.executionHook.startExecute.get()); + // we should not have a response from execute() since we do not have a fallback and run() failed + assertNull(command.builder.executionHook.endExecuteSuccessResponse); + // we should have an exception since run() failed + assertNotNull(command.builder.executionHook.endExecuteFailureException); + // run() failure + assertEquals(FailureType.COMMAND_EXCEPTION, command.builder.executionHook.endExecuteFailureType); + + // thread execution + assertEquals(1, command.builder.executionHook.threadStart.get()); + assertEquals(1, command.builder.executionHook.threadComplete.get()); + + /* test with queue() */ + command = new UnknownFailureTestCommandWithoutFallback(); + try { + command.queue().get(); + fail("Expecting exception"); + } catch (Exception e) { + // ignore + } + + // the run() method should run as we're not short-circuited or rejected + assertEquals(1, command.builder.executionHook.startRun.get()); + // we should not have a response + assertNull(command.builder.executionHook.runSuccessResponse); + // we should have an exception + assertNotNull(command.builder.executionHook.runFailureException); + + // the fallback() method should be run since run() failed + assertEquals(1, command.builder.executionHook.startFallback.get()); + // no response since fallback is not implemented + assertNull(command.builder.executionHook.fallbackSuccessResponse); + // not null since it's not implemented and throws an exception + assertNotNull(command.builder.executionHook.fallbackFailureException); + + // the queue() method was used + assertEquals(1, command.builder.executionHook.startExecute.get()); + // we should not have a response from queue() since we do not have a fallback and run() failed + assertNull(command.builder.executionHook.endExecuteSuccessResponse); + // we should have an exception since run() failed + assertNotNull(command.builder.executionHook.endExecuteFailureException); + // run() failure + assertEquals(FailureType.COMMAND_EXCEPTION, command.builder.executionHook.endExecuteFailureType); + + // thread execution + assertEquals(1, command.builder.executionHook.threadStart.get()); + assertEquals(1, command.builder.executionHook.threadComplete.get()); + + } + + /** + * Execution hook on failed execution with a fallback + */ + @Test + public void testExecutionHookRunFailureWithFallback() { + /* test with execute() */ + TestHystrixCommand command = new KnownFailureTestCommandWithFallback(new TestCircuitBreaker()); + command.execute(); + + // the run() method should run as we're not short-circuited or rejected + assertEquals(1, command.builder.executionHook.startRun.get()); + // we should not have a response from run since run() failed + assertNull(command.builder.executionHook.runSuccessResponse); + // we should have an exception since run() failed + assertNotNull(command.builder.executionHook.runFailureException); + + // the fallback() method should be run since run() failed + assertEquals(1, command.builder.executionHook.startFallback.get()); + // a response since fallback is implemented + assertNotNull(command.builder.executionHook.fallbackSuccessResponse); + // null since it's implemented and succeeds + assertNull(command.builder.executionHook.fallbackFailureException); + + // the execute() method was used + assertEquals(1, command.builder.executionHook.startExecute.get()); + // we should have a response from execute() since we expect a fallback despite failure of run() + assertNotNull(command.builder.executionHook.endExecuteSuccessResponse); + // we should not have an exception because we expect a fallback + assertNull(command.builder.executionHook.endExecuteFailureException); + + // thread execution + assertEquals(1, command.builder.executionHook.threadStart.get()); + assertEquals(1, command.builder.executionHook.threadComplete.get()); + + /* test with queue() */ + command = new KnownFailureTestCommandWithFallback(new TestCircuitBreaker()); + try { + command.queue().get(); + } catch (Exception e) { + throw new RuntimeException(e); + } + + // the run() method should run as we're not short-circuited or rejected + assertEquals(1, command.builder.executionHook.startRun.get()); + // we should not have a response from run since run() failed + assertNull(command.builder.executionHook.runSuccessResponse); + // we should have an exception since run() failed + assertNotNull(command.builder.executionHook.runFailureException); + + // the fallback() method should be run since run() failed + assertEquals(1, command.builder.executionHook.startFallback.get()); + // a response since fallback is implemented + assertNotNull(command.builder.executionHook.fallbackSuccessResponse); + // null since it's implemented and succeeds + assertNull(command.builder.executionHook.fallbackFailureException); + + // the queue() method was used + assertEquals(1, command.builder.executionHook.startExecute.get()); + // we should have a response from queue() since we expect a fallback despite failure of run() + assertNotNull(command.builder.executionHook.endExecuteSuccessResponse); + // we should not have an exception because we expect a fallback + assertNull(command.builder.executionHook.endExecuteFailureException); + + // thread execution + assertEquals(1, command.builder.executionHook.threadStart.get()); + assertEquals(1, command.builder.executionHook.threadComplete.get()); + } + + /** + * Execution hook on failed execution with a fallback failure + */ + @Test + public void testExecutionHookRunFailureWithFallbackFailure() { + /* test with execute() */ + TestHystrixCommand command = new KnownFailureTestCommandWithFallbackFailure(); + try { + command.execute(); + fail("Expecting exception"); + } catch (Exception e) { + // ignore + } + + // the run() method should run as we're not short-circuited or rejected + assertEquals(1, command.builder.executionHook.startRun.get()); + // we should not have a response because run() and fallback fail + assertNull(command.builder.executionHook.runSuccessResponse); + // we should have an exception because run() and fallback fail + assertNotNull(command.builder.executionHook.runFailureException); + + // the fallback() method should be run since run() failed + assertEquals(1, command.builder.executionHook.startFallback.get()); + // no response since fallback fails + assertNull(command.builder.executionHook.fallbackSuccessResponse); + // not null since it's implemented but fails + assertNotNull(command.builder.executionHook.fallbackFailureException); + + // the execute() method was used + assertEquals(1, command.builder.executionHook.startExecute.get()); + // we should not have a response because run() and fallback fail + assertNull(command.builder.executionHook.endExecuteSuccessResponse); + // we should have an exception because run() and fallback fail + assertNotNull(command.builder.executionHook.endExecuteFailureException); + // run() failure + assertEquals(FailureType.COMMAND_EXCEPTION, command.builder.executionHook.endExecuteFailureType); + + // thread execution + assertEquals(1, command.builder.executionHook.threadStart.get()); + assertEquals(1, command.builder.executionHook.threadComplete.get()); + + /* test with queue() */ + command = new KnownFailureTestCommandWithFallbackFailure(); + try { + command.queue().get(); + fail("Expecting exception"); + } catch (Exception e) { + // ignore + } + + // the run() method should run as we're not short-circuited or rejected + assertEquals(1, command.builder.executionHook.startRun.get()); + // we should not have a response because run() and fallback fail + assertNull(command.builder.executionHook.runSuccessResponse); + // we should have an exception because run() and fallback fail + assertNotNull(command.builder.executionHook.runFailureException); + + // the fallback() method should be run since run() failed + assertEquals(1, command.builder.executionHook.startFallback.get()); + // no response since fallback fails + assertNull(command.builder.executionHook.fallbackSuccessResponse); + // not null since it's implemented but fails + assertNotNull(command.builder.executionHook.fallbackFailureException); + + // the queue() method was used + assertEquals(1, command.builder.executionHook.startExecute.get()); + // we should not have a response because run() and fallback fail + assertNull(command.builder.executionHook.endExecuteSuccessResponse); + // we should have an exception because run() and fallback fail + assertNotNull(command.builder.executionHook.endExecuteFailureException); + // run() failure + assertEquals(FailureType.COMMAND_EXCEPTION, command.builder.executionHook.endExecuteFailureType); + + // thread execution + assertEquals(1, command.builder.executionHook.threadStart.get()); + assertEquals(1, command.builder.executionHook.threadComplete.get()); + } + + /** + * Execution hook on timeout without a fallback + */ + @Test + public void testExecutionHookTimeoutWithoutFallback() { + TestHystrixCommand command = new TestCommandWithTimeout(50, TestCommandWithTimeout.FALLBACK_NOT_IMPLEMENTED); + try { + command.queue().get(); + fail("Expecting exception"); + } catch (Exception e) { + // ignore + } + + // the run() method should run as we're not short-circuited or rejected + assertEquals(1, command.builder.executionHook.startRun.get()); + // we should not have a response because of timeout and no fallback + assertNull(command.builder.executionHook.runSuccessResponse); + // we should not have an exception because run() didn't fail, it timed out + assertNull(command.builder.executionHook.runFailureException); + + // the fallback() method should be run due to timeout + assertEquals(1, command.builder.executionHook.startFallback.get()); + // no response since no fallback + assertNull(command.builder.executionHook.fallbackSuccessResponse); + // not null since no fallback implementation + assertNotNull(command.builder.executionHook.fallbackFailureException); + + // execution occurred + assertEquals(1, command.builder.executionHook.startExecute.get()); + // we should not have a response because of timeout and no fallback + assertNull(command.builder.executionHook.endExecuteSuccessResponse); + // we should have an exception because of timeout and no fallback + assertNotNull(command.builder.executionHook.endExecuteFailureException); + // timeout failure + assertEquals(FailureType.TIMEOUT, command.builder.executionHook.endExecuteFailureType); + + // thread execution + assertEquals(1, command.builder.executionHook.threadStart.get()); + + // we need to wait for the thread to complete before the onThreadComplete hook will be called + try { + Thread.sleep(400); + } catch (InterruptedException e) { + // ignore + } + assertEquals(1, command.builder.executionHook.threadComplete.get()); + } + + /** + * Execution hook on timeout with a fallback + */ + @Test + public void testExecutionHookTimeoutWithFallback() { + TestHystrixCommand command = new TestCommandWithTimeout(50, TestCommandWithTimeout.FALLBACK_SUCCESS); + try { + command.queue().get(); + } catch (Exception e) { + throw new RuntimeException("not expecting", e); + } + + // the run() method should run as we're not short-circuited or rejected + assertEquals(1, command.builder.executionHook.startRun.get()); + // we should not have a response because of timeout + assertNull(command.builder.executionHook.runSuccessResponse); + // we should not have an exception because run() didn't fail, it timed out + assertNull(command.builder.executionHook.runFailureException); + + // the fallback() method should be run due to timeout + assertEquals(1, command.builder.executionHook.startFallback.get()); + // response since we have a fallback + assertNotNull(command.builder.executionHook.fallbackSuccessResponse); + // null since fallback succeeds + assertNull(command.builder.executionHook.fallbackFailureException); + + // execution occurred + assertEquals(1, command.builder.executionHook.startExecute.get()); + // we should have a response because of fallback + assertNotNull(command.builder.executionHook.endExecuteSuccessResponse); + // we should not have an exception because of fallback + assertNull(command.builder.executionHook.endExecuteFailureException); + + // thread execution + assertEquals(1, command.builder.executionHook.threadStart.get()); + + // we need to wait for the thread to complete before the onThreadComplete hook will be called + try { + Thread.sleep(400); + } catch (InterruptedException e) { + // ignore + } + assertEquals(1, command.builder.executionHook.threadComplete.get()); + } + + /** + * Execution hook on rejected with a fallback + */ + @Test + public void testExecutionHookRejectedWithFallback() { + TestCircuitBreaker circuitBreaker = new TestCircuitBreaker(); + SingleThreadedPool pool = new SingleThreadedPool(1); + + try { + // fill the queue + new TestCommandRejection(circuitBreaker, pool, 500, 600, TestCommandRejection.FALLBACK_SUCCESS).queue(); + new TestCommandRejection(circuitBreaker, pool, 500, 600, TestCommandRejection.FALLBACK_SUCCESS).queue(); + } catch (Exception e) { + // ignore + } + + TestCommandRejection command = new TestCommandRejection(circuitBreaker, pool, 500, 600, TestCommandRejection.FALLBACK_SUCCESS); + try { + // now execute one that will be rejected + command.queue().get(); + } catch (Exception e) { + throw new RuntimeException("not expecting", e); + } + + assertTrue(command.isResponseRejected()); + + // the run() method should not run as we're rejected + assertEquals(0, command.builder.executionHook.startRun.get()); + // we should not have a response because of rejection + assertNull(command.builder.executionHook.runSuccessResponse); + // we should not have an exception because we didn't run + assertNull(command.builder.executionHook.runFailureException); + + // the fallback() method should be run due to rejection + assertEquals(1, command.builder.executionHook.startFallback.get()); + // response since we have a fallback + assertNotNull(command.builder.executionHook.fallbackSuccessResponse); + // null since fallback succeeds + assertNull(command.builder.executionHook.fallbackFailureException); + + // execution occurred + assertEquals(1, command.builder.executionHook.startExecute.get()); + // we should have a response because of fallback + assertNotNull(command.builder.executionHook.endExecuteSuccessResponse); + // we should not have an exception because of fallback + assertNull(command.builder.executionHook.endExecuteFailureException); + + // thread execution + assertEquals(0, command.builder.executionHook.threadStart.get()); + assertEquals(0, command.builder.executionHook.threadComplete.get()); + } + + /** + * Execution hook on short-circuit with a fallback + */ + @Test + public void testExecutionHookShortCircuitedWithFallbackViaQueue() { + TestCircuitBreaker circuitBreaker = new TestCircuitBreaker().setForceShortCircuit(true); + KnownFailureTestCommandWithoutFallback command = new KnownFailureTestCommandWithoutFallback(circuitBreaker); + try { + // now execute one that will be short-circuited + command.queue().get(); + fail("we expect an error as there is no fallback"); + } catch (Exception e) { + // expecting + } + + assertTrue(command.isResponseShortCircuited()); + + // the run() method should not run as we're rejected + assertEquals(0, command.builder.executionHook.startRun.get()); + // we should not have a response because of rejection + assertNull(command.builder.executionHook.runSuccessResponse); + // we should not have an exception because we didn't run + assertNull(command.builder.executionHook.runFailureException); + + // the fallback() method should be run due to rejection + assertEquals(1, command.builder.executionHook.startFallback.get()); + // no response since we don't have a fallback + assertNull(command.builder.executionHook.fallbackSuccessResponse); + // not null since fallback fails and throws an exception + assertNotNull(command.builder.executionHook.fallbackFailureException); + + // execution occurred + assertEquals(1, command.builder.executionHook.startExecute.get()); + // we should not have a response because fallback fails + assertNull(command.builder.executionHook.endExecuteSuccessResponse); + // we won't have an exception because short-circuit doesn't have one + assertNull(command.builder.executionHook.endExecuteFailureException); + // but we do expect to receive a onError call with FailureType.SHORTCIRCUIT + assertEquals(FailureType.SHORTCIRCUIT, command.builder.executionHook.endExecuteFailureType); + + // thread execution + assertEquals(0, command.builder.executionHook.threadStart.get()); + assertEquals(0, command.builder.executionHook.threadComplete.get()); + } + + /** + * Execution hook on short-circuit with a fallback + */ + @Test + public void testExecutionHookShortCircuitedWithFallbackViaExecute() { + TestCircuitBreaker circuitBreaker = new TestCircuitBreaker().setForceShortCircuit(true); + KnownFailureTestCommandWithoutFallback command = new KnownFailureTestCommandWithoutFallback(circuitBreaker); + try { + // now execute one that will be short-circuited + command.execute(); + fail("we expect an error as there is no fallback"); + } catch (Exception e) { + // expecting + } + + assertTrue(command.isResponseShortCircuited()); + + // the run() method should not run as we're rejected + assertEquals(0, command.builder.executionHook.startRun.get()); + // we should not have a response because of rejection + assertNull(command.builder.executionHook.runSuccessResponse); + // we should not have an exception because we didn't run + assertNull(command.builder.executionHook.runFailureException); + + // the fallback() method should be run due to rejection + assertEquals(1, command.builder.executionHook.startFallback.get()); + // no response since we don't have a fallback + assertNull(command.builder.executionHook.fallbackSuccessResponse); + // not null since fallback fails and throws an exception + assertNotNull(command.builder.executionHook.fallbackFailureException); + + // execution occurred + assertEquals(1, command.builder.executionHook.startExecute.get()); + // we should not have a response because fallback fails + assertNull(command.builder.executionHook.endExecuteSuccessResponse); + // we won't have an exception because short-circuit doesn't have one + assertNull(command.builder.executionHook.endExecuteFailureException); + // but we do expect to receive a onError call with FailureType.SHORTCIRCUIT + assertEquals(FailureType.SHORTCIRCUIT, command.builder.executionHook.endExecuteFailureType); + + // thread execution + assertEquals(0, command.builder.executionHook.threadStart.get()); + assertEquals(0, command.builder.executionHook.threadComplete.get()); + } + + /** + * Execution hook on successful execution with semaphore isolation + */ + @Test + public void testExecutionHookSuccessfulCommandWithSemaphoreIsolation() { + /* test with execute() */ + TestSemaphoreCommand command = new TestSemaphoreCommand(new TestCircuitBreaker(), 1, 10); + command.execute(); + + assertFalse(command.isExecutedInThread()); + + // the run() method should run as we're not short-circuited or rejected + assertEquals(1, command.builder.executionHook.startRun.get()); + // we expect a successful response from run() + assertNotNull(command.builder.executionHook.runSuccessResponse); + // we do not expect an exception + assertNull(command.builder.executionHook.runFailureException); + + // the fallback() method should not be run as we were successful + assertEquals(0, command.builder.executionHook.startFallback.get()); + // null since it didn't run + assertNull(command.builder.executionHook.fallbackSuccessResponse); + // null since it didn't run + assertNull(command.builder.executionHook.fallbackFailureException); + + // the execute() method was used + assertEquals(1, command.builder.executionHook.startExecute.get()); + // we should have a response from execute() since run() succeeded + assertNotNull(command.builder.executionHook.endExecuteSuccessResponse); + // we should not have an exception since run() succeeded + assertNull(command.builder.executionHook.endExecuteFailureException); + + // thread execution + assertEquals(0, command.builder.executionHook.threadStart.get()); + assertEquals(0, command.builder.executionHook.threadComplete.get()); + + /* test with queue() */ + command = new TestSemaphoreCommand(new TestCircuitBreaker(), 1, 10); + try { + command.queue().get(); + } catch (Exception e) { + throw new RuntimeException(e); + } + + assertFalse(command.isExecutedInThread()); + + // the run() method should run as we're not short-circuited or rejected + assertEquals(1, command.builder.executionHook.startRun.get()); + // we expect a successful response from run() + assertNotNull(command.builder.executionHook.runSuccessResponse); + // we do not expect an exception + assertNull(command.builder.executionHook.runFailureException); + + // the fallback() method should not be run as we were successful + assertEquals(0, command.builder.executionHook.startFallback.get()); + // null since it didn't run + assertNull(command.builder.executionHook.fallbackSuccessResponse); + // null since it didn't run + assertNull(command.builder.executionHook.fallbackFailureException); + + // the queue() method was used + assertEquals(1, command.builder.executionHook.startExecute.get()); + // we should have a response from queue() since run() succeeded + assertNotNull(command.builder.executionHook.endExecuteSuccessResponse); + // we should not have an exception since run() succeeded + assertNull(command.builder.executionHook.endExecuteFailureException); + + // thread execution + assertEquals(0, command.builder.executionHook.threadStart.get()); + assertEquals(0, command.builder.executionHook.threadComplete.get()); + } + + /** + * Execution hook on successful execution with semaphore isolation + */ + @Test + public void testExecutionHookFailureWithSemaphoreIsolation() { + /* test with execute() */ + TestSemaphoreCommand command = new TestSemaphoreCommand(new TestCircuitBreaker(), 0, 200); + try { + command.execute(); + fail("we expect a failure"); + } catch (Exception e) { + // expected + } + + assertFalse(command.isExecutedInThread()); + assertTrue(command.isResponseRejected()); + + // the run() method should not run as we are rejected + assertEquals(0, command.builder.executionHook.startRun.get()); + // null as run() does not get invoked + assertNull(command.builder.executionHook.runSuccessResponse); + // null as run() does not get invoked + assertNull(command.builder.executionHook.runFailureException); + + // the fallback() method should run because of rejection + assertEquals(1, command.builder.executionHook.startFallback.get()); + // null since there is no fallback + assertNull(command.builder.executionHook.fallbackSuccessResponse); + // not null since the fallback is not implemented + assertNotNull(command.builder.executionHook.fallbackFailureException); + + // the execute() method was used + assertEquals(1, command.builder.executionHook.startExecute.get()); + // we should not have a response since fallback has nothing + assertNull(command.builder.executionHook.endExecuteSuccessResponse); + // we won't have an exception because rejection doesn't have one + assertNull(command.builder.executionHook.endExecuteFailureException); + // but we do expect to receive a onError call with FailureType.SHORTCIRCUIT + assertEquals(FailureType.REJECTED_SEMAPHORE_EXECUTION, command.builder.executionHook.endExecuteFailureType); + + // thread execution + assertEquals(0, command.builder.executionHook.threadStart.get()); + assertEquals(0, command.builder.executionHook.threadComplete.get()); + } + /* ******************************************************************************** */ /* ******************************************************************************** */ /* private HystrixCommand class implementations for unit testing */ @@ -4247,7 +5062,7 @@ public void testCheckedException() { TestHystrixCommand(TestCommandBuilder builder) { super(builder.owner, builder.dependencyKey, builder.threadPoolKey, builder.circuitBreaker, builder.threadPool, builder.commandPropertiesDefaults, builder.threadPoolPropertiesDefaults, builder.metrics, - builder.fallbackSemaphore, builder.executionSemaphore, TEST_PROPERTIES_FACTORY); + builder.fallbackSemaphore, builder.executionSemaphore, TEST_PROPERTIES_FACTORY, builder.executionHook); this.builder = builder; } @@ -4267,6 +5082,7 @@ static class TestCommandBuilder { HystrixCommandMetrics metrics = _cb.metrics; TryableSemaphore fallbackSemaphore = null; TryableSemaphore executionSemaphore = null; + TestExecutionHook executionHook = new TestExecutionHook(); TestCommandBuilder setOwner(HystrixCommandGroupKey owner) { this.owner = owner; @@ -4608,6 +5424,7 @@ protected Boolean run() { } catch (Exception e2) { // ignore } + System.out.println("after interruption with extra sleep"); } return true; } @@ -4775,7 +5592,10 @@ private static class TestSemaphoreCommand extends TestHystrixCommand { private TestSemaphoreCommand(TestCircuitBreaker circuitBreaker, int executionSemaphoreCount, long executionSleep) { super(testPropsBuilder().setCircuitBreaker(circuitBreaker).setMetrics(circuitBreaker.metrics) - .setCommandPropertiesDefaults(HystrixCommandProperties.Setter.getUnitTestPropertiesSetter().withExecutionIsolationStrategy(ExecutionIsolationStrategy.SEMAPHORE).withExecutionIsolationSemaphoreMaxConcurrentRequests(executionSemaphoreCount))); + .setCommandPropertiesDefaults(HystrixCommandProperties.Setter.getUnitTestPropertiesSetter() + .withExecutionIsolationStrategy(ExecutionIsolationStrategy.SEMAPHORE) + .withExecutionIsolationSemaphoreMaxConcurrentRequests(executionSemaphoreCount)) + .setExecutionSemaphore(new TryableSemaphore(HystrixProperty.Factory.asProperty(executionSemaphoreCount)))); this.executionSleep = executionSleep; } @@ -4961,6 +5781,100 @@ public String getCollapserPropertiesCacheKey(HystrixCollapserKey collapserKey, c } } + + private static class TestExecutionHook extends HystrixCommandExecutionHook { + + AtomicInteger startExecute = new AtomicInteger(); + + @Override + public void onStart(HystrixCommand commandInstance) { + super.onStart(commandInstance); + startExecute.incrementAndGet(); + } + + Object endExecuteSuccessResponse = null; + + @Override + public T onSuccess(HystrixCommand commandInstance, T response) { + endExecuteSuccessResponse = response; + return super.onSuccess(commandInstance, response); + } + + Exception endExecuteFailureException = null; + FailureType endExecuteFailureType = null; + + @Override + public Exception onError(HystrixCommand commandInstance, FailureType failureType, Exception e) { + endExecuteFailureException = e; + endExecuteFailureType = failureType; + return super.onError(commandInstance, failureType, e); + } + + AtomicInteger startRun = new AtomicInteger(); + + @Override + public void onRunStart(HystrixCommand commandInstance) { + super.onRunStart(commandInstance); + startRun.incrementAndGet(); + } + + Object runSuccessResponse = null; + + @Override + public T onRunSuccess(HystrixCommand commandInstance, T response) { + runSuccessResponse = response; + return super.onRunSuccess(commandInstance, response); + } + + Exception runFailureException = null; + + @Override + public Exception onRunError(HystrixCommand commandInstance, Exception e) { + runFailureException = e; + return super.onRunError(commandInstance, e); + } + + AtomicInteger startFallback = new AtomicInteger(); + + @Override + public void onFallbackStart(HystrixCommand commandInstance) { + super.onFallbackStart(commandInstance); + startFallback.incrementAndGet(); + } + + Object fallbackSuccessResponse = null; + + @Override + public T onFallbackSuccess(HystrixCommand commandInstance, T response) { + fallbackSuccessResponse = response; + return super.onFallbackSuccess(commandInstance, response); + } + + Exception fallbackFailureException = null; + + @Override + public Exception onFallbackError(HystrixCommand commandInstance, Exception e) { + fallbackFailureException = e; + return super.onFallbackError(commandInstance, e); + } + + AtomicInteger threadStart = new AtomicInteger(); + + @Override + public void onThreadStart(HystrixCommand commandInstance) { + super.onThreadStart(commandInstance); + threadStart.incrementAndGet(); + } + + AtomicInteger threadComplete = new AtomicInteger(); + + @Override + public void onThreadComplete(HystrixCommand commandInstance) { + super.onThreadComplete(commandInstance); + threadComplete.incrementAndGet(); + } + + } } } diff --git a/hystrix-core/src/main/java/com/netflix/hystrix/strategy/HystrixPlugins.java b/hystrix-core/src/main/java/com/netflix/hystrix/strategy/HystrixPlugins.java index 892f54724..ddb4b357a 100644 --- a/hystrix-core/src/main/java/com/netflix/hystrix/strategy/HystrixPlugins.java +++ b/hystrix-core/src/main/java/com/netflix/hystrix/strategy/HystrixPlugins.java @@ -26,6 +26,8 @@ import com.netflix.hystrix.strategy.concurrency.HystrixConcurrencyStrategyDefault; import com.netflix.hystrix.strategy.eventnotifier.HystrixEventNotifier; import com.netflix.hystrix.strategy.eventnotifier.HystrixEventNotifierDefault; +import com.netflix.hystrix.strategy.executionhook.HystrixCommandExecutionHook; +import com.netflix.hystrix.strategy.executionhook.HystrixCommandExecutionHookDefault; import com.netflix.hystrix.strategy.metrics.HystrixMetricsPublisher; import com.netflix.hystrix.strategy.metrics.HystrixMetricsPublisherDefault; import com.netflix.hystrix.strategy.properties.HystrixPropertiesStrategy; @@ -48,6 +50,7 @@ public class HystrixPlugins { private final AtomicReference concurrencyStrategy = new AtomicReference(); private final AtomicReference metricsPublisher = new AtomicReference(); private final AtomicReference propertiesFactory = new AtomicReference(); + private final AtomicReference commandExecutionHook = new AtomicReference(); private HystrixPlugins() { @@ -209,6 +212,45 @@ public void registerPropertiesStrategy(HystrixPropertiesStrategy impl) { } } + /** + * Retrieve instance of {@link HystrixCommandExecutionHook} to use based on order of precedence as defined in {@link HystrixPlugins} class header. + *

+ * Override default by using {@link #registerCommandExecutionHook(HystrixCommandExecutionHook)} or setting property: hystrix.plugin.HystrixCommandExecutionHook.implementation with the + * full classname to + * load. + * + * @return {@link HystrixCommandExecutionHook} implementation to use + */ + public HystrixCommandExecutionHook getCommandExecutionHook() { + if (commandExecutionHook.get() == null) { + // check for an implementation from System.getProperty first + Object impl = getPluginImplementationViaProperty(HystrixCommandExecutionHook.class); + if (impl == null) { + // nothing set via properties so initialize with default + commandExecutionHook.compareAndSet(null, HystrixCommandExecutionHookDefault.getInstance()); + // we don't return from here but call get() again in case of thread-race so the winner will always get returned + } else { + // we received an implementation from the system property so use it + commandExecutionHook.compareAndSet(null, (HystrixCommandExecutionHook) impl); + } + } + return commandExecutionHook.get(); + } + + /** + * Register a {@link HystrixCommandExecutionHook} implementation as a global override of any injected or default implementations. + * + * @param impl + * {@link HystrixCommandExecutionHook} implementation + * @throws IllegalStateException + * if called more than once or after the default was initialized (if usage occurs before trying to register) + */ + public void registerCommandExecutionHook(HystrixCommandExecutionHook impl) { + if (!commandExecutionHook.compareAndSet(null, impl)) { + throw new IllegalStateException("Another strategy was already registered."); + } + } + private static Object getPluginImplementationViaProperty(Class pluginClass) { String classSimpleName = pluginClass.getSimpleName(); /* diff --git a/hystrix-core/src/main/java/com/netflix/hystrix/strategy/executionhook/HystrixCommandExecutionHook.java b/hystrix-core/src/main/java/com/netflix/hystrix/strategy/executionhook/HystrixCommandExecutionHook.java new file mode 100644 index 000000000..e0089d413 --- /dev/null +++ b/hystrix-core/src/main/java/com/netflix/hystrix/strategy/executionhook/HystrixCommandExecutionHook.java @@ -0,0 +1,170 @@ +/** + * Copyright 2013 Netflix, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.netflix.hystrix.strategy.executionhook; + +import com.netflix.hystrix.HystrixCommand; +import com.netflix.hystrix.HystrixCommandProperties.ExecutionIsolationStrategy; +import com.netflix.hystrix.exception.HystrixRuntimeException; +import com.netflix.hystrix.exception.HystrixRuntimeException.FailureType; +import com.netflix.hystrix.strategy.HystrixPlugins; + +/** + * Abstract ExecutionHook with invocations at different lifecycle points of {@link HystrixCommand} execution with default no-op implementations. + *

+ * See {@link HystrixPlugins} or the Hystrix GitHub Wiki for information on configuring plugins: https://github.com/Netflix/Hystrix/wiki/Plugins. + * */ +public abstract class HystrixCommandExecutionHook { + + /** + * Invoked before {@link HystrixCommand#run()} is about to be executed. + * + * @param commandInstance + * The executing HystrixCommand instance. + */ + public void onRunStart(HystrixCommand commandInstance) { + // do nothing by default + } + + /** + * Invoked after successful execution of {@link HystrixCommand#run()} with response value. + * + * @param commandInstance + * The executing HystrixCommand instance. + * @param response + * from {@link HystrixCommand#run()} + * @return T response object that can be modified, decorated, replaced or just returned as a pass-thru. + */ + public T onRunSuccess(HystrixCommand commandInstance, T response) { + // pass-thru by default + return response; + } + + /** + * Invoked after failed execution of {@link HystrixCommand#run()} with thrown Exception. + * + * @param commandInstance + * The executing HystrixCommand instance. + * @param e + * Exception thrown by {@link HystrixCommand#run()} + * @return Exception that can be decorated, replaced or just returned as a pass-thru. + */ + public Exception onRunError(HystrixCommand commandInstance, Exception e) { + // pass-thru by default + return e; + } + + /** + * Invoked before {@link HystrixCommand#getFallback()} is about to be executed. + * + * @param commandInstance + * The executing HystrixCommand instance. + */ + public void onFallbackStart(HystrixCommand commandInstance) { + // do nothing by default + } + + /** + * Invoked after successful execution of {@link HystrixCommand#getFallback()} with response value. + * + * @param commandInstance + * The executing HystrixCommand instance. + * @param fallbackResponse + * from {@link HystrixCommand#getFallback()} + * @return T response object that can be modified, decorated, replaced or just returned as a pass-thru. + */ + public T onFallbackSuccess(HystrixCommand commandInstance, T fallbackResponse) { + // pass-thru by default + return fallbackResponse; + } + + /** + * Invoked after failed execution of {@link HystrixCommand#getFallback()} with thrown exception. + * + * @param commandInstance + * The executing HystrixCommand instance. + * @param e + * Exception thrown by {@link HystrixCommand#getFallback()} + * @return Exception that can be decorated, replaced or just returned as a pass-thru. + */ + public Exception onFallbackError(HystrixCommand commandInstance, Exception e) { + // pass-thru by default + return e; + } + + /** + * Invoked before {@link HystrixCommand} executes. + * + * @param commandInstance + * The executing HystrixCommand instance. + */ + public void onStart(HystrixCommand commandInstance) { + // do nothing by default + } + + /** + * Invoked after successful completion of {@link HystrixCommand} execution. + * + * @param commandInstance + * The executing HystrixCommand instance. + * @param response + * from {@link HystrixCommand} + * @return T response object that can be modified, decorated, replaced or just returned as a pass-thru. + */ + public T onSuccess(HystrixCommand commandInstance, T response) { + // pass-thru by default + return response; + } + + /** + * Invoked after failed completion of {@link HystrixCommand} execution. + * + * @param commandInstance + * The executing HystrixCommand instance. + * @param failureType + * {@link FailureType} representing the type of failure that occurred. + *

+ * See {@link HystrixRuntimeException} for more information. + * @param e + * Exception thrown by {@link HystrixCommand} + * @return Exception that can be decorated, replaced or just returned as a pass-thru. + */ + public Exception onError(HystrixCommand commandInstance, FailureType failureType, Exception e) { + // pass-thru by default + return e; + } + + /** + * Invoked at start of thread execution when {@link HystrixCommand} is executed using {@link ExecutionIsolationStrategy#THREAD}. + * + * @param commandInstance + * The executing HystrixCommand instance. + */ + public void onThreadStart(HystrixCommand commandInstance) { + // do nothing by default + } + + /** + * Invoked at completion of thread execution when {@link HystrixCommand} is executed using {@link ExecutionIsolationStrategy#THREAD}. + * + * @param commandInstance + * The executing HystrixCommand instance. + */ + public void onThreadComplete(HystrixCommand commandInstance) { + // do nothing by default + } + +} diff --git a/hystrix-core/src/main/java/com/netflix/hystrix/strategy/executionhook/HystrixCommandExecutionHookDefault.java b/hystrix-core/src/main/java/com/netflix/hystrix/strategy/executionhook/HystrixCommandExecutionHookDefault.java new file mode 100644 index 000000000..89820edd7 --- /dev/null +++ b/hystrix-core/src/main/java/com/netflix/hystrix/strategy/executionhook/HystrixCommandExecutionHookDefault.java @@ -0,0 +1,35 @@ +/** + * Copyright 2013 Netflix, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.netflix.hystrix.strategy.executionhook; + +/** + * Default implementations of {@link HystrixCommandExecutionHook} that does nothing. + * + * @ExcludeFromJavadoc + */ +public class HystrixCommandExecutionHookDefault extends HystrixCommandExecutionHook { + + private static HystrixCommandExecutionHookDefault INSTANCE = new HystrixCommandExecutionHookDefault(); + + private HystrixCommandExecutionHookDefault() { + + } + + public static HystrixCommandExecutionHook getInstance() { + return INSTANCE; + } + +} diff --git a/hystrix-core/src/main/java/com/netflix/hystrix/strategy/executionhook/package-info.java b/hystrix-core/src/main/java/com/netflix/hystrix/strategy/executionhook/package-info.java new file mode 100644 index 000000000..103996747 --- /dev/null +++ b/hystrix-core/src/main/java/com/netflix/hystrix/strategy/executionhook/package-info.java @@ -0,0 +1,21 @@ +/** + * Copyright 2013 Netflix, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +/** + * Strategy definition for execution hook. + * + * @since 1.2.0 + */ +package com.netflix.hystrix.strategy.executionhook; \ No newline at end of file From aedb7962f8c533c97ef5526c9f2ad1b3324a0724 Mon Sep 17 00:00:00 2001 From: Ben Christensen Date: Fri, 4 Jan 2013 21:45:50 -0800 Subject: [PATCH 7/8] ExecutionHook: Change onSuccess to onComplete I think onComplete is clearer since the response can be either from run() or getFallback() --- .../java/com/netflix/hystrix/HystrixCommand.java | 12 ++++++------ .../executionhook/HystrixCommandExecutionHook.java | 8 +++++--- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/hystrix-core/src/main/java/com/netflix/hystrix/HystrixCommand.java b/hystrix-core/src/main/java/com/netflix/hystrix/HystrixCommand.java index 0728b7545..98b5fafa1 100755 --- a/hystrix-core/src/main/java/com/netflix/hystrix/HystrixCommand.java +++ b/hystrix-core/src/main/java/com/netflix/hystrix/HystrixCommand.java @@ -470,7 +470,7 @@ private R executeWithSemaphore() { // we want to run it synchronously R response = executeCommand(); - response = executionHook.onSuccess(this, response); + response = executionHook.onComplete(this, response); // put in cache if (isRequestCachingEnabled()) { requestCache.putIfAbsent(getCacheKey(), asFutureForCache(response)); @@ -626,7 +626,7 @@ public ExecutionResult getExecutionResult() { // execute outside of future so that fireAndForget will still work (ie. someone calls queue() but not get()) and so that multiple requests can be deduped through request caching R r = executeCommand(); - r = executionHook.onSuccess(this, r); + r = executionHook.onComplete(this, r); value.set(r); return responseFuture; @@ -704,7 +704,7 @@ public R call() throws Exception { // execute the command R r = executeCommand(); - return executionHook.onSuccess(_this, r); + return executionHook.onComplete(_this, r); } catch (Exception e) { if (!isCommandTimedOut.get()) { // count (if we didn't timeout) that we are throwing an exception and re-throw it @@ -1101,7 +1101,7 @@ private R getFallbackOrThrowException(HystrixEventType eventType, FailureType fa metrics.markFallbackSuccess(); // record the executionResult executionResult = executionResult.addEvents(eventType, HystrixEventType.FALLBACK_SUCCESS); - return executionHook.onSuccess(this, fallback); + return executionHook.onComplete(this, fallback); } catch (UnsupportedOperationException fe) { logger.debug("No fallback for HystrixCommand. ", fe); // debug only since we're throwing the exception and someone higher will do something with it // record the executionResult @@ -5795,9 +5795,9 @@ public void onStart(HystrixCommand commandInstance) { Object endExecuteSuccessResponse = null; @Override - public T onSuccess(HystrixCommand commandInstance, T response) { + public T onComplete(HystrixCommand commandInstance, T response) { endExecuteSuccessResponse = response; - return super.onSuccess(commandInstance, response); + return super.onComplete(commandInstance, response); } Exception endExecuteFailureException = null; diff --git a/hystrix-core/src/main/java/com/netflix/hystrix/strategy/executionhook/HystrixCommandExecutionHook.java b/hystrix-core/src/main/java/com/netflix/hystrix/strategy/executionhook/HystrixCommandExecutionHook.java index e0089d413..8e9cfebeb 100644 --- a/hystrix-core/src/main/java/com/netflix/hystrix/strategy/executionhook/HystrixCommandExecutionHook.java +++ b/hystrix-core/src/main/java/com/netflix/hystrix/strategy/executionhook/HystrixCommandExecutionHook.java @@ -116,15 +116,17 @@ public void onStart(HystrixCommand commandInstance) { } /** - * Invoked after successful completion of {@link HystrixCommand} execution. + * Invoked after completion of {@link HystrixCommand} execution that results in a response. + *

+ * The response can come either from {@link HystrixCommand#run()} or {@link HystrixCommand#getFallback()}. * * @param commandInstance * The executing HystrixCommand instance. * @param response - * from {@link HystrixCommand} + * from {@link HystrixCommand#run()} or {@link HystrixCommand#getFallback()}. * @return T response object that can be modified, decorated, replaced or just returned as a pass-thru. */ - public T onSuccess(HystrixCommand commandInstance, T response) { + public T onComplete(HystrixCommand commandInstance, T response) { // pass-thru by default return response; } From fbc838f659dfec6fe3cf1ada2930f5e4558c715b Mon Sep 17 00:00:00 2001 From: Ben Christensen Date: Mon, 7 Jan 2013 11:15:53 -0800 Subject: [PATCH 8/8] Additional javadocs for ExecutionHook/EventNotifier --- .../hystrix/strategy/HystrixPlugins.java | 4 +++ .../eventnotifier/HystrixEventNotifier.java | 9 +++++- .../HystrixCommandExecutionHook.java | 31 +++++++++++++++++++ 3 files changed, 43 insertions(+), 1 deletion(-) diff --git a/hystrix-core/src/main/java/com/netflix/hystrix/strategy/HystrixPlugins.java b/hystrix-core/src/main/java/com/netflix/hystrix/strategy/HystrixPlugins.java index ddb4b357a..385f765c3 100644 --- a/hystrix-core/src/main/java/com/netflix/hystrix/strategy/HystrixPlugins.java +++ b/hystrix-core/src/main/java/com/netflix/hystrix/strategy/HystrixPlugins.java @@ -220,6 +220,8 @@ public void registerPropertiesStrategy(HystrixPropertiesStrategy impl) { * load. * * @return {@link HystrixCommandExecutionHook} implementation to use + * + * @since 1.2 */ public HystrixCommandExecutionHook getCommandExecutionHook() { if (commandExecutionHook.get() == null) { @@ -244,6 +246,8 @@ public HystrixCommandExecutionHook getCommandExecutionHook() { * {@link HystrixCommandExecutionHook} implementation * @throws IllegalStateException * if called more than once or after the default was initialized (if usage occurs before trying to register) + * + * @since 1.2 */ public void registerCommandExecutionHook(HystrixCommandExecutionHook impl) { if (!commandExecutionHook.compareAndSet(null, impl)) { diff --git a/hystrix-core/src/main/java/com/netflix/hystrix/strategy/eventnotifier/HystrixEventNotifier.java b/hystrix-core/src/main/java/com/netflix/hystrix/strategy/eventnotifier/HystrixEventNotifier.java index d8d5623c9..c2e5a4931 100644 --- a/hystrix-core/src/main/java/com/netflix/hystrix/strategy/eventnotifier/HystrixEventNotifier.java +++ b/hystrix-core/src/main/java/com/netflix/hystrix/strategy/eventnotifier/HystrixEventNotifier.java @@ -19,14 +19,21 @@ import com.netflix.hystrix.HystrixCommandKey; import com.netflix.hystrix.HystrixCommandProperties.ExecutionIsolationStrategy; -import com.netflix.hystrix.strategy.HystrixPlugins; import com.netflix.hystrix.HystrixEventType; +import com.netflix.hystrix.strategy.HystrixPlugins; /** * Abstract EventNotifier that allows receiving notifications for different events with default implementations. *

* See {@link HystrixPlugins} or the Hystrix GitHub Wiki for information on configuring plugins: https://github.com/Netflix/Hystrix/wiki/Plugins. + *

+ * Note on thread-safety and performance + *

+ * A single implementation of this class will be used globally so methods on this class will be invoked concurrently from multiple threads so all functionality must be thread-safe. + *

+ * Methods are also invoked synchronously and will add to execution time of the commands so all behavior should be fast. If anything time-consuming is to be done it should be spawned asynchronously + * onto separate worker threads. */ public abstract class HystrixEventNotifier { diff --git a/hystrix-core/src/main/java/com/netflix/hystrix/strategy/executionhook/HystrixCommandExecutionHook.java b/hystrix-core/src/main/java/com/netflix/hystrix/strategy/executionhook/HystrixCommandExecutionHook.java index 8e9cfebeb..33b29a0b1 100644 --- a/hystrix-core/src/main/java/com/netflix/hystrix/strategy/executionhook/HystrixCommandExecutionHook.java +++ b/hystrix-core/src/main/java/com/netflix/hystrix/strategy/executionhook/HystrixCommandExecutionHook.java @@ -26,6 +26,15 @@ *

* See {@link HystrixPlugins} or the Hystrix GitHub Wiki for information on configuring plugins: https://github.com/Netflix/Hystrix/wiki/Plugins. + *

+ * Note on thread-safety and performance + *

+ * A single implementation of this class will be used globally so methods on this class will be invoked concurrently from multiple threads so all functionality must be thread-safe. + *

+ * Methods are also invoked synchronously and will add to execution time of the commands so all behavior should be fast. If anything time-consuming is to be done it should be spawned asynchronously + * onto separate worker threads. + * + * @since 1.2 * */ public abstract class HystrixCommandExecutionHook { @@ -34,6 +43,8 @@ public abstract class HystrixCommandExecutionHook { * * @param commandInstance * The executing HystrixCommand instance. + * + * @since 1.2 */ public void onRunStart(HystrixCommand commandInstance) { // do nothing by default @@ -47,6 +58,8 @@ public void onRunStart(HystrixCommand commandInstance) { * @param response * from {@link HystrixCommand#run()} * @return T response object that can be modified, decorated, replaced or just returned as a pass-thru. + * + * @since 1.2 */ public T onRunSuccess(HystrixCommand commandInstance, T response) { // pass-thru by default @@ -61,6 +74,8 @@ public T onRunSuccess(HystrixCommand commandInstance, T response) { * @param e * Exception thrown by {@link HystrixCommand#run()} * @return Exception that can be decorated, replaced or just returned as a pass-thru. + * + * @since 1.2 */ public Exception onRunError(HystrixCommand commandInstance, Exception e) { // pass-thru by default @@ -72,6 +87,8 @@ public Exception onRunError(HystrixCommand commandInstance, Exception e) * * @param commandInstance * The executing HystrixCommand instance. + * + * @since 1.2 */ public void onFallbackStart(HystrixCommand commandInstance) { // do nothing by default @@ -85,6 +102,8 @@ public void onFallbackStart(HystrixCommand commandInstance) { * @param fallbackResponse * from {@link HystrixCommand#getFallback()} * @return T response object that can be modified, decorated, replaced or just returned as a pass-thru. + * + * @since 1.2 */ public T onFallbackSuccess(HystrixCommand commandInstance, T fallbackResponse) { // pass-thru by default @@ -99,6 +118,8 @@ public T onFallbackSuccess(HystrixCommand commandInstance, T fallbackResp * @param e * Exception thrown by {@link HystrixCommand#getFallback()} * @return Exception that can be decorated, replaced or just returned as a pass-thru. + * + * @since 1.2 */ public Exception onFallbackError(HystrixCommand commandInstance, Exception e) { // pass-thru by default @@ -110,6 +131,8 @@ public Exception onFallbackError(HystrixCommand commandInstance, Exceptio * * @param commandInstance * The executing HystrixCommand instance. + * + * @since 1.2 */ public void onStart(HystrixCommand commandInstance) { // do nothing by default @@ -125,6 +148,8 @@ public void onStart(HystrixCommand commandInstance) { * @param response * from {@link HystrixCommand#run()} or {@link HystrixCommand#getFallback()}. * @return T response object that can be modified, decorated, replaced or just returned as a pass-thru. + * + * @since 1.2 */ public T onComplete(HystrixCommand commandInstance, T response) { // pass-thru by default @@ -143,6 +168,8 @@ public T onComplete(HystrixCommand commandInstance, T response) { * @param e * Exception thrown by {@link HystrixCommand} * @return Exception that can be decorated, replaced or just returned as a pass-thru. + * + * @since 1.2 */ public Exception onError(HystrixCommand commandInstance, FailureType failureType, Exception e) { // pass-thru by default @@ -154,6 +181,8 @@ public Exception onError(HystrixCommand commandInstance, FailureType fail * * @param commandInstance * The executing HystrixCommand instance. + * + * @since 1.2 */ public void onThreadStart(HystrixCommand commandInstance) { // do nothing by default @@ -164,6 +193,8 @@ public void onThreadStart(HystrixCommand commandInstance) { * * @param commandInstance * The executing HystrixCommand instance. + * + * @since 1.2 */ public void onThreadComplete(HystrixCommand commandInstance) { // do nothing by default