Permalink
Browse files

Merge pull request #250 from benjchristensen/issue-236-1_4

Tripped CircuitBreaker Wouldn't Close Under Contention
  • Loading branch information...
2 parents 3dc58ff + 63cffd9 commit 7a143911df712091b04a75f02c0a81423d691ddf @benjchristensen benjchristensen committed Apr 25, 2014
@@ -139,10 +139,10 @@ protected HystrixCircuitBreakerImpl(HystrixCommandKey key, HystrixCommandGroupKe
public void markSuccess() {
if (circuitOpen.get()) {
- // If we have been 'open' and have a success then we want to close the circuit. This handles the 'singleTest' logic
- circuitOpen.set(false);
// TODO how can we can do this without resetting the counts so we don't lose metrics of short-circuits etc?
metrics.resetCounter();
+ // If we have been 'open' and have a success then we want to close the circuit. This handles the 'singleTest' logic
+ circuitOpen.set(false);
}
}
@@ -202,8 +202,10 @@ public boolean isOpen() {
// How could previousValue be true? If another thread was going through this code at the same time a race-condition could have
// caused another thread to set it to true already even though we were in the process of doing the same
circuitOpenedOrLastTestedTime.set(System.currentTimeMillis());
+ return true;
+ } else {
+ return false;
}
- return true;
}
}
@@ -232,8 +232,10 @@ public int getTotalTimeMean() {
}
/* package */void resetCounter() {
- counter.reset();
// TODO can we do without this somehow?
+ counter.reset();
+ lastHealthCountsSnapshot.set(System.currentTimeMillis());
+ healthCountsSnapshot = new HealthCounts(0, 0, 0);
}
/**
@@ -1,11 +1,18 @@
package com.netflix.hystrix;
-import static org.junit.Assert.*;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+import java.util.Random;
+
+import org.junit.Ignore;
import org.junit.Test;
import com.netflix.hystrix.HystrixCircuitBreaker.HystrixCircuitBreakerImpl;
+import com.netflix.hystrix.strategy.HystrixPlugins;
import com.netflix.hystrix.strategy.eventnotifier.HystrixEventNotifierDefault;
+import com.netflix.hystrix.strategy.executionhook.HystrixCommandExecutionHook;
import com.netflix.hystrix.util.HystrixRollingNumberEvent;
public class HystrixCircuitBreakerTest {
@@ -494,4 +501,98 @@ private static HystrixCircuitBreaker getCircuitBreaker(HystrixCommandKey key, Hy
KEY_ONE, KEY_TWO;
}
+ // ignoring since this never ends ... useful for testing https://github.com/Netflix/Hystrix/issues/236
+ @Ignore
+ @Test
+ public void testSuccessClosesCircuitWhenBusy() throws InterruptedException {
+ HystrixPlugins.getInstance().registerCommandExecutionHook(new MyHystrixCommandExecutionHook());
+ try {
+ performLoad(200, 0, 40);
+ performLoad(250, 100, 40);
+ performLoad(600, 0, 40);
+ } finally {
+ Hystrix.reset();
+ }
+
+ }
+
+ void performLoad(int totalNumCalls, int errPerc, int waitMillis) {
+
+ Random rnd = new Random();
+
+ for (int i = 0; i < totalNumCalls; i++) {
+ //System.out.println(i);
+
+ try {
+ boolean err = rnd.nextFloat() * 100 < errPerc;
+
+ TestCommand cmd = new TestCommand(err);
+ cmd.execute();
+
+ } catch (Exception e) {
+ //System.err.println(e.getMessage());
+ }
+
+ try {
+ Thread.sleep(waitMillis);
+ } catch (InterruptedException e) {
+ }
+ }
+ }
+
+ public class TestCommand extends HystrixCommand<String> {
+
+ boolean error;
+
+ public TestCommand(final boolean error) {
+ super(HystrixCommandGroupKey.Factory.asKey("group"));
+
+ this.error = error;
+ }
+
+ @Override
+ protected String run() throws Exception {
+
+ if (error) {
+ throw new Exception("forced failure");
+ } else {
+ return "success";
+ }
+ }
+
+ @Override
+ protected String getFallback() {
+ if (isFailedExecution()) {
+ return getFailedExecutionException().getMessage();
+ } else {
+ return "other fail reason";
+ }
+ }
+
+ }
+
+ public class MyHystrixCommandExecutionHook extends HystrixCommandExecutionHook {
+
+ @Override
+ public <T> T onComplete(final HystrixCommand<T> command, final T response) {
+
+ logHC(command, response);
+
+ return super.onComplete(command, response);
+ }
+
+ private int counter = 0;
+
+ private <T> void logHC(HystrixCommand<T> command, T response) {
+
+ //if ((counter++ % 20) == 0) {
+ HystrixCommandMetrics metrics = command.getMetrics();
+ System.out.println("cb/error-count/%/total: "
+ + command.isCircuitBreakerOpen() + " "
+ + metrics.getHealthCounts().getErrorCount() + " "
+ + metrics.getHealthCounts().getErrorPercentage() + " "
+ + metrics.getHealthCounts().getTotalRequests() + " => " + response + " " + command.getExecutionEvents());
+ //}
+ }
+ }
}

0 comments on commit 7a14391

Please sign in to comment.