Lying as a valid coding pattern
time to read 2 min | 324 words

Davy Brion has an interesting post about Protecting Your Application From Remote Problems, which contains a really interesting implementation of CircuitBreaker.

One thing, though, bothered me about it. Let us look at the code:

private class OpenState : CircuitBreakerState
{
	private readonly Timer timer;

	public OpenState(CircuitBreaker circuitBreaker)
		: base(circuitBreaker)
	{
		timer = new Timer(circuitBreaker.timeout.TotalMilliseconds);
		timer.Elapsed += TimeoutHasBeenReached;
		timer.AutoReset = false;
		timer.Start();
	}

	private void TimeoutHasBeenReached(object sender, ElapsedEventArgs e)
	{
		circuitBreaker.MoveToHalfOpenState();
	}

	public override void ProtectedCodeIsAboutToBeCalled()
	{
		throw new OpenCircuitException();
	}
}

The code works correctly, but I… don’t like the implementation. Please note that this is based on more gut feeling than anything else.

The part that I don’t like is that it is doing too much work. The use of timer for the timeout seems like a waste to me. What I would rather do is something like this:

private class OpenState : CircuitBreakerState
{
	private readonly DateTime expiry;

	public OpenState(CircuitBreaker circuitBreaker)
		: base(circuitBreaker)
	{
		expiry = DateTime.UtcNow + circuitBreaker.timeout;
	}

	public override void ProtectedCodeIsAboutToBeCalled()
	{
  	     if(DateTime.UtcNow < expiry)
			throw new OpenCircuitException();
             circuitBreaker.MoveToHalfOpenState();
	}
}

As seen from outside, this behave in exactly the same fashion, while not actually requiring anything from the system when we are actually not touching the code that the circuit breaker protects. It also force this to be a lazy evaluation, so if we are actually doing some reporting on the state of the circuit breaker, we won’t see the status change until something actually make a call to it.

That may or may not be a consideration. But even if it is, I would rather do the state change in the reporting function than in a dedicated timer.

Tweet Share Share 12 comments
Tags: