Skip to content

CircuitBreaker.allowRequest change state of isOpen #1541

Closed
@narusas

Description

@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.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions