Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Encapsulate Circuit Breaker Success Condition in a Half-Open State #63

Open
empire opened this issue Aug 14, 2024 · 0 comments
Open

Encapsulate Circuit Breaker Success Condition in a Half-Open State #63

empire opened this issue Aug 14, 2024 · 0 comments

Comments

@empire
Copy link

empire commented Aug 14, 2024

In the current implementation of the CircuitBreaker library, the logic for determining whether the circuit should transition from StateHalfOpen to StateClosed is directly embedded within the onSuccess method. Specifically, the condition cb.counts.ConsecutiveSuccesses >= cb.maxRequests is checked to decide if the circuit should close.

The current approach tightly couples the condition with the onSuccess method, making the code less modular and harder to maintain. It might be helpful to adjust or extend the logic for transitioning states.

Proposed Solution

To improve code modularity and maintainability, I propose encapsulating the condition cb.counts.ConsecutiveSuccesses >= cb.maxRequests into a dedicated method, such as shouldCloseCircuit(). This change would make the onSuccess method cleaner and the success condition easier to manage and modify.

Suggested Implementation

Here's how the refactored onSuccess method would look:

func (cb *CircuitBreaker) onSuccess(state State, now time.Time) {
    switch state {
    case StateClosed:
        cb.counts.onSuccess()
    case StateHalfOpen:
        cb.counts.onSuccess()
        if cb.shouldCloseCircuit(cb.counts) {
            cb.setState(StateClosed, now)
        }
    }
}

I'll be okay to send a pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant