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 98b5fafa1..8c00cb613 100755 --- a/hystrix-core/src/main/java/com/netflix/hystrix/HystrixCommand.java +++ b/hystrix-core/src/main/java/com/netflix/hystrix/HystrixCommand.java @@ -1095,40 +1095,59 @@ private R getFallbackOrThrowException(HystrixEventType eventType, FailureType fa */ private R getFallbackOrThrowException(HystrixEventType eventType, FailureType failureType, String message, Exception e) { try { - // retrieve the fallback - R fallback = getFallbackWithProtection(); - // mark fallback on counter - metrics.markFallbackSuccess(); - // record the executionResult - executionResult = executionResult.addEvents(eventType, HystrixEventType.FALLBACK_SUCCESS); - 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 - 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); - } + if (properties.fallbackEnabled().get()) { + /* fallback behavior is permitted so attempt */ + try { + // retrieve the fallback + R fallback = getFallbackWithProtection(); + // mark fallback on counter + metrics.markFallbackSuccess(); + // record the executionResult + executionResult = executionResult.addEvents(eventType, HystrixEventType.FALLBACK_SUCCESS); + 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 + 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); + 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); - } + /* 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); + throw new HystrixRuntimeException(failureType, this.getClass(), getLogMessagePrefix() + " " + message + " and failed retrieving fallback.", e, fe); + } + } else { + /* fallback is disabled so throw HystrixRuntimeException */ + + logger.debug("Fallback disabled for HystrixCommand so will throw HystrixRuntimeException. ", e); // 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 fallback disabled.", e, null); + } } finally { // record that we're completed (to handle non-successful events we do it here as well as at the end of executeCommand isExecutionComplete.set(true); @@ -5046,6 +5065,48 @@ public void testExecutionHookFailureWithSemaphoreIsolation() { assertEquals(0, command.builder.executionHook.threadComplete.get()); } + /** + * Test a command execution that fails but has a fallback. + */ + @Test + public void testExecutionFailureWithFallbackImplementedButDisabled() { + TestHystrixCommand commandEnabled = new KnownFailureTestCommandWithFallback(new TestCircuitBreaker(), true); + try { + assertEquals(false, commandEnabled.execute()); + } catch (Exception e) { + e.printStackTrace(); + fail("We should have received a response from the fallback."); + } + + TestHystrixCommand commandDisabled = new KnownFailureTestCommandWithFallback(new TestCircuitBreaker(), false); + try { + assertEquals(false, commandDisabled.execute()); + fail("expect exception thrown"); + } catch (Exception e) { + // expected + } + + assertEquals("we failed with a simulated issue", commandDisabled.getFailedExecutionException().getMessage()); + + assertTrue(commandDisabled.isFailedExecution()); + + assertEquals(0, commandDisabled.builder.metrics.getRollingCount(HystrixRollingNumberEvent.SUCCESS)); + assertEquals(1, commandDisabled.builder.metrics.getRollingCount(HystrixRollingNumberEvent.EXCEPTION_THROWN)); + assertEquals(1, commandDisabled.builder.metrics.getRollingCount(HystrixRollingNumberEvent.FAILURE)); + assertEquals(0, commandDisabled.builder.metrics.getRollingCount(HystrixRollingNumberEvent.FALLBACK_REJECTION)); + assertEquals(0, commandDisabled.builder.metrics.getRollingCount(HystrixRollingNumberEvent.FALLBACK_FAILURE)); + assertEquals(0, commandDisabled.builder.metrics.getRollingCount(HystrixRollingNumberEvent.FALLBACK_SUCCESS)); + assertEquals(0, commandDisabled.builder.metrics.getRollingCount(HystrixRollingNumberEvent.SEMAPHORE_REJECTED)); + assertEquals(0, commandDisabled.builder.metrics.getRollingCount(HystrixRollingNumberEvent.SHORT_CIRCUITED)); + assertEquals(0, commandDisabled.builder.metrics.getRollingCount(HystrixRollingNumberEvent.THREAD_POOL_REJECTED)); + assertEquals(0, commandDisabled.builder.metrics.getRollingCount(HystrixRollingNumberEvent.TIMEOUT)); + assertEquals(0, commandDisabled.builder.metrics.getRollingCount(HystrixRollingNumberEvent.RESPONSE_FROM_CACHE)); + + assertEquals(100, commandDisabled.builder.metrics.getHealthCounts().getErrorPercentage()); + + assertEquals(2, HystrixRequestLog.getCurrentRequest().getExecutedCommands().size()); + } + /* ******************************************************************************** */ /* ******************************************************************************** */ /* private HystrixCommand class implementations for unit testing */ @@ -5236,6 +5297,11 @@ public KnownFailureTestCommandWithFallback(TestCircuitBreaker circuitBreaker) { super(testPropsBuilder().setCircuitBreaker(circuitBreaker).setMetrics(circuitBreaker.metrics)); } + public KnownFailureTestCommandWithFallback(TestCircuitBreaker circuitBreaker, boolean fallbackEnabled) { + super(testPropsBuilder().setCircuitBreaker(circuitBreaker).setMetrics(circuitBreaker.metrics) + .setCommandPropertiesDefaults(HystrixCommandProperties.Setter.getUnitTestPropertiesSetter().withFallbackEnabled(fallbackEnabled))); + } + @Override protected Boolean run() { System.out.println("*** simulated failed execution ***"); diff --git a/hystrix-core/src/main/java/com/netflix/hystrix/HystrixCommandMetrics.java b/hystrix-core/src/main/java/com/netflix/hystrix/HystrixCommandMetrics.java index 59894e0ec..74f163c57 100644 --- a/hystrix-core/src/main/java/com/netflix/hystrix/HystrixCommandMetrics.java +++ b/hystrix-core/src/main/java/com/netflix/hystrix/HystrixCommandMetrics.java @@ -114,8 +114,8 @@ public static Collection getInstances() { this.group = commandGroup; this.properties = properties; this.counter = new HystrixRollingNumber(properties.metricsRollingStatisticalWindowInMilliseconds(), properties.metricsRollingStatisticalWindowBuckets()); - this.percentileExecution = new HystrixRollingPercentile(properties.metricsRollingPercentileWindowInMilliseconds(), properties.metricsRollingPercentileWindowBuckets(), properties.metricsRollingPercentileBucketSize()); - this.percentileTotal = new HystrixRollingPercentile(properties.metricsRollingPercentileWindowInMilliseconds(), properties.metricsRollingPercentileWindowBuckets(), properties.metricsRollingPercentileBucketSize()); + this.percentileExecution = new HystrixRollingPercentile(properties.metricsRollingPercentileWindowInMilliseconds(), properties.metricsRollingPercentileWindowBuckets(), properties.metricsRollingPercentileBucketSize(), properties.metricsRollingPercentileEnabled()); + this.percentileTotal = new HystrixRollingPercentile(properties.metricsRollingPercentileWindowInMilliseconds(), properties.metricsRollingPercentileWindowBuckets(), properties.metricsRollingPercentileBucketSize(), properties.metricsRollingPercentileEnabled()); this.eventNotifier = eventNotifier; } @@ -364,9 +364,7 @@ public int getCurrentConcurrentExecutionCount() { * Execution time of {@link HystrixCommand#run()}. */ /* package */void addCommandExecutionTime(long duration) { - if (properties.metricsRollingPercentileEnabled().get()) { - percentileExecution.addValue((int) duration); - } + percentileExecution.addValue((int) duration); } /** @@ -376,9 +374,7 @@ public int getCurrentConcurrentExecutionCount() { * This differs from {@link #addCommandExecutionTime} in that this covers all of the threading and scheduling overhead, not just the execution of the {@link HystrixCommand#run()} method. */ /* package */void addUserThreadExecutionTime(long duration) { - if (properties.metricsRollingPercentileEnabled().get()) { - percentileTotal.addValue((int) duration); - } + percentileTotal.addValue((int) duration); } private volatile HealthCounts healthCountsSnapshot = new HealthCounts(0, 0, 0); diff --git a/hystrix-core/src/main/java/com/netflix/hystrix/HystrixCommandProperties.java b/hystrix-core/src/main/java/com/netflix/hystrix/HystrixCommandProperties.java index e9b786f10..15936117f 100644 --- a/hystrix-core/src/main/java/com/netflix/hystrix/HystrixCommandProperties.java +++ b/hystrix-core/src/main/java/com/netflix/hystrix/HystrixCommandProperties.java @@ -57,6 +57,7 @@ public abstract class HystrixCommandProperties { private static final Boolean default_metricsRollingPercentileEnabled = true; private static final Boolean default_requestCacheEnabled = true; private static final Integer default_fallbackIsolationSemaphoreMaxConcurrentRequests = 10; + private static final Boolean default_fallbackEnabled = true; private static final Integer default_executionIsolationSemaphoreMaxConcurrentRequests = 10; private static final Boolean default_requestLogEnabled = true; private static final Boolean default_circuitBreakerEnabled = true; @@ -77,6 +78,7 @@ public abstract class HystrixCommandProperties { private final HystrixProperty executionIsolationThreadPoolKeyOverride; // What thread-pool this command should run in (if running on a separate thread). private final HystrixProperty executionIsolationSemaphoreMaxConcurrentRequests; // Number of permits for execution semaphore private final HystrixProperty fallbackIsolationSemaphoreMaxConcurrentRequests; // Number of permits for fallback semaphore + private final HystrixProperty fallbackEnabled; // Whether fallback should be attempted. private final HystrixProperty executionIsolationThreadInterruptOnTimeout; // Whether an underlying Future/Thread (when runInSeparateThread == true) should be interrupted after a timeout private final HystrixProperty metricsRollingStatisticalWindowInMilliseconds; // milliseconds back that will be tracked private final HystrixProperty metricsRollingStatisticalWindowBuckets; // number of buckets in the statisticalWindow @@ -122,6 +124,7 @@ protected HystrixCommandProperties(HystrixCommandKey key, HystrixCommandProperti 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); + this.fallbackEnabled = getProperty(propertyPrefix, key, "fallback.enabled", builder.getFallbackEnabled(), default_fallbackEnabled); this.metricsRollingStatisticalWindowInMilliseconds = getProperty(propertyPrefix, key, "metrics.rollingStats.timeInMilliseconds", builder.getMetricsRollingStatisticalWindowInMilliseconds(), default_metricsRollingStatisticalWindow); this.metricsRollingStatisticalWindowBuckets = getProperty(propertyPrefix, key, "metrics.rollingStats.numBuckets", builder.getMetricsRollingStatisticalWindowBuckets(), default_metricsRollingStatisticalWindowBuckets); this.metricsRollingPercentileEnabled = getProperty(propertyPrefix, key, "metrics.rollingPercentile.enabled", builder.getMetricsRollingPercentileEnabled(), default_metricsRollingPercentileEnabled); @@ -273,6 +276,17 @@ public HystrixProperty fallbackIsolationSemaphoreMaxConcurrentRequests( return fallbackIsolationSemaphoreMaxConcurrentRequests; } + /** + * Whether {@link HystrixCommand#getFallback()} should be attempted when failure occurs. + * + * @return {@code HystrixProperty} + * + * @since 1.2 + */ + public HystrixProperty fallbackEnabled() { + return fallbackEnabled; + } + /** * Time in milliseconds to wait between allowing health snapshots to be taken that calculate success and error percentages and affect {@link HystrixCircuitBreaker#isOpen()} status. *

@@ -474,6 +488,7 @@ public static class Setter { private Boolean executionIsolationThreadInterruptOnTimeout = null; private Integer executionIsolationThreadTimeoutInMilliseconds = null; private Integer fallbackIsolationSemaphoreMaxConcurrentRequests = null; + private Boolean fallbackEnabled = null; private Integer metricsHealthSnapshotIntervalInMilliseconds = null; private Integer metricsRollingPercentileBucketSize = null; private Boolean metricsRollingPercentileEnabled = null; @@ -532,6 +547,10 @@ public Integer getFallbackIsolationSemaphoreMaxConcurrentRequests() { return fallbackIsolationSemaphoreMaxConcurrentRequests; } + public Boolean getFallbackEnabled() { + return fallbackEnabled; + } + public Integer getMetricsHealthSnapshotIntervalInMilliseconds() { return metricsHealthSnapshotIntervalInMilliseconds; } @@ -623,6 +642,11 @@ public Setter withFallbackIsolationSemaphoreMaxConcurrentRequests(int value) { return this; } + public Setter withFallbackEnabled(boolean value) { + this.fallbackEnabled = value; + return this; + } + public Setter withMetricsHealthSnapshotIntervalInMilliseconds(int value) { this.metricsHealthSnapshotIntervalInMilliseconds = value; return this; @@ -686,6 +710,7 @@ public Setter withRequestLogEnabled(boolean value) { .withRequestLogEnabled(true) .withExecutionIsolationSemaphoreMaxConcurrentRequests(20) .withFallbackIsolationSemaphoreMaxConcurrentRequests(10) + .withFallbackEnabled(true) .withCircuitBreakerForceClosed(false) .withMetricsRollingPercentileEnabled(true) .withRequestCacheEnabled(true) @@ -765,6 +790,11 @@ public HystrixProperty fallbackIsolationSemaphoreMaxConcurrentRequests( return HystrixProperty.Factory.asProperty(builder.fallbackIsolationSemaphoreMaxConcurrentRequests); } + @Override + public HystrixProperty fallbackEnabled() { + return HystrixProperty.Factory.asProperty(builder.fallbackEnabled); + } + @Override public HystrixProperty metricsHealthSnapshotIntervalInMilliseconds() { return HystrixProperty.Factory.asProperty(builder.metricsHealthSnapshotIntervalInMilliseconds); diff --git a/hystrix-core/src/main/java/com/netflix/hystrix/util/HystrixRollingPercentile.java b/hystrix-core/src/main/java/com/netflix/hystrix/util/HystrixRollingPercentile.java index eda15894c..cb7f4584c 100644 --- a/hystrix-core/src/main/java/com/netflix/hystrix/util/HystrixRollingPercentile.java +++ b/hystrix-core/src/main/java/com/netflix/hystrix/util/HystrixRollingPercentile.java @@ -58,6 +58,7 @@ public class HystrixRollingPercentile { private final HystrixProperty timeInMilliseconds; private final HystrixProperty numberOfBuckets; private final HystrixProperty bucketDataLength; + private final HystrixProperty enabled; /* * This will get flipped each time a new bucket is created. @@ -78,17 +79,22 @@ public class HystrixRollingPercentile { * {@code HystrixProperty} for number of values stored in each bucket *

* Example: 1000 to store a max of 1000 values in each 5 second bucket + * @param enabled + * {@code HystrixProperty} whether data should be tracked and percentiles calculated. + *

+ * If 'false' methods will do nothing. */ - public HystrixRollingPercentile(HystrixProperty timeInMilliseconds, HystrixProperty numberOfBuckets, HystrixProperty bucketDataLength) { - this(ACTUAL_TIME, timeInMilliseconds, numberOfBuckets, bucketDataLength); + public HystrixRollingPercentile(HystrixProperty timeInMilliseconds, HystrixProperty numberOfBuckets, HystrixProperty bucketDataLength, HystrixProperty enabled) { + this(ACTUAL_TIME, timeInMilliseconds, numberOfBuckets, bucketDataLength, enabled); } - private HystrixRollingPercentile(Time time, HystrixProperty timeInMilliseconds, HystrixProperty numberOfBuckets, HystrixProperty bucketDataLength) { + private HystrixRollingPercentile(Time time, HystrixProperty timeInMilliseconds, HystrixProperty numberOfBuckets, HystrixProperty bucketDataLength, HystrixProperty enabled) { this.time = time; this.timeInMilliseconds = timeInMilliseconds; this.numberOfBuckets = numberOfBuckets; this.bucketDataLength = bucketDataLength; + this.enabled = enabled; if (this.timeInMilliseconds.get() % this.numberOfBuckets.get() != 0) { throw new IllegalArgumentException("The timeInMilliseconds must divide equally into numberOfBuckets. For example 1000/10 is ok, 1000/11 is not."); @@ -104,6 +110,10 @@ private HystrixRollingPercentile(Time time, HystrixProperty timeInMilli * Value to be stored in current bucket such as execution latency in milliseconds */ public void addValue(int... value) { + /* no-op if disabled */ + if (!enabled.get()) + return; + for (int v : value) { try { getCurrentBucket().data.addValue(v); @@ -125,6 +135,10 @@ public void addValue(int... value) { * @return int percentile value */ public int getPercentile(double percentile) { + /* no-op if disabled */ + if (!enabled.get()) + return -1; + // force logic to move buckets forward in case other requests aren't making it happen getCurrentBucket(); // fetch the current snapshot @@ -137,6 +151,10 @@ public int getPercentile(double percentile) { * @return mean of all values */ public int getMean() { + /* no-op if disabled */ + if (!enabled.get()) + return -1; + // force logic to move buckets forward in case other requests aren't making it happen getCurrentBucket(); // fetch the current snapshot @@ -261,6 +279,10 @@ private Bucket getCurrentBucket() { * Force a reset so that percentiles start being gathered from scratch. */ public void reset() { + /* no-op if disabled */ + if (!enabled.get()) + return; + // clear buckets so we start over again buckets.clear(); } @@ -603,11 +625,12 @@ public static class UnitTest { private static final HystrixProperty timeInMilliseconds = HystrixProperty.Factory.asProperty(60000); private static final HystrixProperty numberOfBuckets = HystrixProperty.Factory.asProperty(12); // 12 buckets at 5000ms each private static final HystrixProperty bucketDataLength = HystrixProperty.Factory.asProperty(1000); + private static final HystrixProperty enabled = HystrixProperty.Factory.asProperty(true); @Test public void testRolling() { MockedTime time = new MockedTime(); - HystrixRollingPercentile p = new HystrixRollingPercentile(time, timeInMilliseconds, numberOfBuckets, bucketDataLength); + HystrixRollingPercentile p = new HystrixRollingPercentile(time, timeInMilliseconds, numberOfBuckets, bucketDataLength, enabled); p.addValue(1000); p.addValue(1000); p.addValue(1000); @@ -670,7 +693,7 @@ public void testRolling() { @Test public void testValueIsZeroAfterRollingWindowPassesAndNoTraffic() { MockedTime time = new MockedTime(); - HystrixRollingPercentile p = new HystrixRollingPercentile(time, timeInMilliseconds, numberOfBuckets, bucketDataLength); + HystrixRollingPercentile p = new HystrixRollingPercentile(time, timeInMilliseconds, numberOfBuckets, bucketDataLength, enabled); p.addValue(1000); p.addValue(1000); p.addValue(1000); @@ -702,7 +725,7 @@ public void testSampleDataOverTime1() { System.out.println("\n\n***************************** testSampleDataOverTime1 \n"); MockedTime time = new MockedTime(); - HystrixRollingPercentile p = new HystrixRollingPercentile(time, timeInMilliseconds, numberOfBuckets, bucketDataLength); + HystrixRollingPercentile p = new HystrixRollingPercentile(time, timeInMilliseconds, numberOfBuckets, bucketDataLength, enabled); int previousTime = 0; for (int i = 0; i < SampleDataHolder1.data.length; i++) { int timeInMillisecondsSinceStart = SampleDataHolder1.data[i][0]; @@ -742,7 +765,7 @@ public void testSampleDataOverTime2() { System.out.println("\n\n***************************** testSampleDataOverTime2 \n"); MockedTime time = new MockedTime(); int previousTime = 0; - HystrixRollingPercentile p = new HystrixRollingPercentile(time, timeInMilliseconds, numberOfBuckets, bucketDataLength); + HystrixRollingPercentile p = new HystrixRollingPercentile(time, timeInMilliseconds, numberOfBuckets, bucketDataLength, enabled); for (int i = 0; i < SampleDataHolder2.data.length; i++) { int timeInMillisecondsSinceStart = SampleDataHolder2.data[i][0]; int latency = SampleDataHolder2.data[i][1]; @@ -878,6 +901,27 @@ public void testPercentileAlgorithm_NISTExample() { Assert.assertEquals(951990, p.getPercentile(100)); } + /** + * This code should work without throwing exceptions but the data returned will all be -1 since the rolling percentile is disabled. + */ + @Test + public void testDoesNothingWhenDisabled() { + MockedTime time = new MockedTime(); + int previousTime = 0; + HystrixRollingPercentile p = new HystrixRollingPercentile(time, timeInMilliseconds, numberOfBuckets, bucketDataLength, HystrixProperty.Factory.asProperty(false)); + for (int i = 0; i < SampleDataHolder2.data.length; i++) { + int timeInMillisecondsSinceStart = SampleDataHolder2.data[i][0]; + int latency = SampleDataHolder2.data[i][1]; + time.increment(timeInMillisecondsSinceStart - previousTime); + previousTime = timeInMillisecondsSinceStart; + p.addValue(latency); + } + + assertEquals(-1, p.getPercentile(50)); + assertEquals(-1, p.getPercentile(75)); + assertEquals(-1, p.getMean()); + } + private static class MockedTime implements Time { private AtomicInteger time = new AtomicInteger(0);