connor4312 / cockatiel

🐦 A resilience and fault-handling library. Supports Backoffs, Retries, Circuit Breakers, Timeouts, Bulkhead Isolation, and Fallbacks.
MIT License
1.59k stars 50 forks source link

Circuit not going into half open state after duration #91

Closed AbiArya closed 4 months ago

AbiArya commented 5 months ago

Hey! I have a circuit breaker defined as

export const breakerObject = circuitBreaker(handleAll, {
  // Will go from open to halfopen after 10 seconds
  halfOpenAfter: 10 * 1000,
  breaker: new SamplingBreaker({
    // if 1% of requests fail, will open the circuit
    threshold: 0.01,
    // samples the %request fail rate over 30 seconds
    duration: 30 * 1000,
    // minimum number of requests per second needed before we can open the circuit.
    // this is so we don't open the circuit due to low request count when arc isnt running
    minimumRps: 1,
  }),
});

and I'm seeing the circuit switch to an open state properly, but then it stays in the open state and never goes into half open.

connor4312 commented 5 months ago

Could you create a complete small code sample so I can better diagnose your issue?

AbiArya commented 5 months ago

Sure, so I have my definition and logging in a file

circuitBreaker.ts

import { circuitBreaker, CircuitState, handleAll, SamplingBreaker } from 'cockatiel';
import {logger} from "../logger";

export const breakerObject = circuitBreaker(handleAll, {
  // Will go from open to halfopen after 10 seconds
  halfOpenAfter: 10 * 1000,
  breaker: new SamplingBreaker({
    // if 1% of requests fail, will open the circuit
    threshold: 0.01,
    // samples the %request fail rate over 30 seconds
    duration: 30 * 1000,
    // minimum number of requests per second needed before we can open the circuit.
    // this is so we don't open the circuit due to low request count when arc isnt running
    minimumRps: 1,
  }),
});

breakerObject.onStateChange((state) => {
  switch(state) {
    case CircuitState.Closed:
      logger.info('circuit breaker is once again closed');
      break;
    case CircuitState.HalfOpen:
      logger.info('circuit breaker is in the halfOpen state');
      break;
    case CircuitState.Open:
      logger.info('circuit breaker is in the Open state');
      break;
    default:
      logger.info('Unknown circuit breaker state: ' + state);
  }
});

Then I have 2 usages of it, one as a healthcheck endpoint in my server dbHealthCheck.ts

export function dbHealthcheck(req: Request, res: Response) {
  if (breakerObject.state === CircuitState.Closed || breakerObject.state === CircuitState.HalfOpen) {
    res.status(200);
    res.send({
      status: 'success',
    });
    // If we're in a state of open, we want to mark as unhealthy
  } else {
    logger.info('Breaker object state is ' + breakerObject.state);
    res.status(400);
    res.send({
      status: 'error',
      message: 'database status unhealthy',
    });
  }
}

and as usage around my database call

dbCall.ts

async function writeMetrics(
  metrics: Metric[]
): Promise<{ [x: string]: any; ErrorMessage: any; ErrorCode: string }[]> {
    const results = await Promise.allSettled(
    metrics.map((metric) =>
      breakerObject
        .execute(() => insertMetricConcurrently(metric))
        .catch((error) => ({ status: 'rejected', reason: error }))
    )
  );
  return results;
}

The function insertMetricConcurrently just returns a promise.resolve() for if a db call succeeds or a promise.reject() for if the db call fails. So the breaker goes into open easily enough, but in my healthcheck endpoint I see that the state stays open and never goes into half-open. Similarly, I never see an event fired for halfOpen state in circuitBreaker.ts.

ghost91- commented 4 months ago

This is mostly a misunderstanding of how this breaker implementation works. It doesn't close automatically after a certain time. It doesn't even go into the half open state automatically. It does so only when a request is performed after the duration has passed.

Maybe this helps with understanding: The half open state is only a very brief transient state: When the breaker is open and a request is made after duration has passed, the breaker goes into the half open state and performs the request. Depending on whether the request is successful or not, the breaker then goes into the open or closed state. So it is only in the half open state while the request is being performed.

Some other circuit breaker implementations (like Resilience4j) allow you to opt into automatically transitioning to half open, using a flag (automaticTransitionFromOpenToHalfOpenEnabled in the case of Resilience4j), but even there it is disabled by default.

Possibly, this could be implemented here, too, but that's rather a feature request (and change in intended behavior) than a bug, imo.

connor4312 commented 4 months ago

Sorry for not following up quickly, what ghost91 said is right. The circuit breaker doesn't do anything in a background task and only changes state in response to calls that come in. You should not guard calls from an open or half-open breaker, since doing so will ensure the breaker never tries to close again. You can instead try to make a call and catch any BrokenCircuitError. Let me know if you have other questions.