Hystrix: CircuitBreaker.allowRequest change state of isOpen

[narusas]

Hi.

Recently, I need to know is circuit breaker is half-opened? can I execute command?

  @Test
  public void can_read_state_of_HALF_OPEN() {
    String commandKey = "test444"; 
    executeBeforeCircuiteOpenLimit(commandKey); // 
    {
      waitHealthUpdate();
      CommandHelloWorld cmd = new CommandHelloWorld(commandKey, true); <- when second arg is true, throw execption
      assertFalse(cmd.isCircuitBreakerOpen());
      try {
        // over 50% error!!
        cmd.execute();
        fail("Must throw exception");
      } catch (HystrixRuntimeException ex) {
        // ok, circuit is opened! 
      }
      waitHealthUpdate();
      assertTrue("Error ratio is over 50%, circuit must be opened", cmd.isCircuitBreakerOpen());
      {
        CommandHelloWorld cmdAllowRequest = new CommandHelloWorld(commandKey, true);
        assertFalse("When circuit is opened, not allow request", cmdAllowRequest.allowRequest());
      }
    }

    {
      waitSleepTime();
      waitHealthUpdate();
      CommandHelloWorld cmd = new CommandHelloWorld(commandKey, false);

      assertTrue("Over sleep time, but no request tried, no status change. So circuit status is opened, too", cmd.isCircuitBreakerOpen());
      /* CHECK ALLOW */ assertTrue("But over sleep time, circuit is half opened, must allow request ", cmd.allowRequest());


    }

    { 
      CommandHelloWorld cmd = new CommandHelloWorld(commandKey, false); <- when second arg is false, not throw execption, success
     /* HERE */  cmd.execute();

      assertFalse("over sleep, first request is succcees, circut status must be  changed", cmd.isCircuitBreakerOpen());
    }
    {
      CommandHelloWorld cmd = new CommandHelloWorld(commandKey, false);
      cmd.execute();
    }

  }

  class CommandHelloWorld extends HystrixCommand<String> {
    ...
    public boolean allowRequest() {
      return circuitBreaker.allowRequest();
    }
    ...
    @Override
    protected String run() {
      if (error) {
        throw new IllegalArgumentException("No");
      }
      return "Hello " + name + "!";
    }

  }

But this test is failed at /* HERE */ .
Result: Circuit is shorted

So I remove /* CHECK ALLOW */ line (remove call allowRequest ),
Test is success.

I trace into circuitBreaker.allowRequest(),
And found ​HystrixCircuitBreakerImpl allowSingleTest method

  public boolean allowSingleTest() {
    long timeCircuitOpenedOrWasLastTested = circuitOpenedOrLastTestedTime.get();
    // 1) if the circuit is open
    // 2) and it's been longer than 'sleepWindow' since we opened the circuit
    if (circuitOpen.get() && System.currentTimeMillis() > timeCircuitOpenedOrWasLastTested + properties.circuitBreakerSleepWindowInMilliseconds().get()) {
      // We push the 'circuitOpenedTime' ahead by 'sleepWindow' since we have allowed one request to try.
      // If it succeeds the circuit will be closed, otherwise another singleTest will be allowed at the end of the 'sleepWindow'.
      if (circuitOpenedOrLastTestedTime.compareAndSet(timeCircuitOpenedOrWasLastTested, System.currentTimeMillis())) {
        // if this returns true that means we set the time so we'll return true to allow the singleTest
        // if it returned false it means another thread raced us and allowed the singleTest before we did
        return true;
      }
    }
    return false;
  }

At Seconds If statement, It change circuitOpenedOrLastTestedTime.

I think this is bug. I just call query method, but it change status.

[mattrjacobs]

Thanks for the report @narusas . Tracing through the code, I think you're correct. Given that the HystrixCircuitBreaker.allowRequest is public, it should be possible to call that method without changing state. I'll work on a fix for this.

[andredasilvapinto]

+1.
I would also like to have the possibility to know if the CB is currently half-open without changing its internal state.

[mattrjacobs]

1568 addresses this issue. Can someone take a look to sanity-check?