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

state and CircuitBreaker store #3

Open
mansoor-omrani opened this issue May 3, 2016 · 7 comments
Open

state and CircuitBreaker store #3

mansoor-omrani opened this issue May 3, 2016 · 7 comments

Comments

@mansoor-omrani
Copy link

mansoor-omrani commented May 3, 2016

Hi Alexander.

I think the code that employs a CircuitBreaker should be stateful. I mean it has to preserve CircuitBreaker object in some state store.

I suggest to add something like ICircuitBreakerStore interface that eases using CircuitBreaker in a stateless application such as a Web Application.

public interface ICircuitBreakerStore
{
    void Set(ICircuitBreaker c, object state);
    ICircuitBreaker Get(object state);
}

For example the following could be an implementation that uses GetData()/SetData() methods in AppDomain to preserve the state:

public class AppDomainCircuitBreakerStore: ICircuitBreakerStore
{
   protected Dictionary<object, ICircuitBreaker> store;

   public AppDomainCircuitBreakerStore()
   {
      store = AppDomain.CurrentDomain.GetData("circuit_breaker_store") as Dictionary<object, ICircuitBreaker>;

      if (store == null)
      {
         store = new Dictionary<object, ICircuitBreaker>();

         AppDomain.CurrentDomain.SetData("circuit_breaker_store", store);
      }
   }
   public void Set(ICircuitBreaker c, object state)
   {
      store.Add(state, c);
   }
    public ICircuitBreaker Get(object state)
   {
      return store[state];
   }
}
@alexandrnikitin
Copy link
Owner

Hi Mansoor! Actually a good idea. I didn't think about that and didn't have a need for but it could be useful. I'm on vacation atm and I'll get back to it as soon as I can!

@Caldas
Copy link

Caldas commented Mar 29, 2019

Hey @alexandrnikitin do you intend to improve it?

@alexandrnikitin
Copy link
Owner

Hi @Caldas,

I'm not sure about it. Frankly I've never seen that use case in few years. I incline to close it to keep the code simple. The code is simple right now and easy to adjust for specific use cases.

@mansoor-omrani
Copy link
Author

Hi @alexandrnikitin

The reason I proposed for something like a CircuitBreaker store was that I believe the same CircuitBreaker object should open the circuit when it is closed. If we don't store CircuitBreaker object somewhere and try to instantiate it each time, the method we gave to it will be executed each time and the idea of CircuitBreaker will be annulled

I did not read the details of your CircuitBreaker class. If you already have implemented a state mechanism somewhere in your class, yes, there won't be a need for a CircuitBreaker store. Nevertheless, for more control and openness (based on O principle in SOLID), I still believe the implementation of storing CircuitBreaker should be extracted out of the CircuitBreaker class since it is a separate concern. It should be implemented using an abstraction such ICircuitBreakerStore that I proposed.

@alexandrnikitin
Copy link
Owner

@mansoor-omrani Yes, at the moment CircuitBreaker encapsulates the state in _currentState field. Yes, you need to use same instance per service. CircuitBreaker is thread-safe. Probably the easiest option is to use an IoC container to manage lifecycle of circuit breaker. I'm still not convinced that offloading the state to a separate class/interface will solve the shared state problem.

@mansoor-omrani
Copy link
Author

@alexandrnikitin I understand what you mean. My humble suggestion is to move the concern of storing state out of circuit breaker into another class, if possible.

This makes circuit breaker lighter somehow. It won't be dependent to any state.

Storing the state is a concern that another class should handle. This is following SOLID principles (Single Responsibility, Open for extension, Closed for modification).

Nevertheless, it's all your choice. You own the circuit breaker code and your wisdom and understanding is better than us as weather applying this change is a correct decision or not, since it might break compatibility somewhere.

@harvey-green
Copy link

I'm late to this, but I wanted to leave a comment. The Circuit Breaker is really solving a problem of misbehaving services - how it does this is not really the concern of the user of the Circuit Breaker. The assumption is that you'd create the circuit breaker at the same time you instantiate the service that your protecting, and that it always starts off in the closed state. So I don't see the need of the user of the Circuit Breaker to be able to access its internal state (e.g. for serialization or storage) ... its state is really an implementation detail.

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

No branches or pull requests

4 participants