rubyist / circuitbreaker

Circuit Breakers in Go
MIT License
1.12k stars 109 forks source link

Store lastFailure as nanos since epoch rather than seconds #41

Closed a-robinson closed 7 years ago

a-robinson commented 7 years ago

This protects against races in state() where it hasn't actually been nextBackoff time since the last failure. Previously the breaker wouldn't back off in many cases where it should have.

For example, consider this set of events, assuming that the first Fail trips the breaker and that nextBackoff is set to 500 milliseconds: 0.5s: Fail() - sets lastFailure to 0, truncating the half second 0.6s: state() - returns halfopen because 0.6s-0.0s > 500ms nextBackoff

This is a case where state() should absolutely return that the breaker is open, but currently doesn't.

CockroachDB has seen problems caused by this in stress testing, and I don't see a reason not to use nanoseconds - they'll continue working until the year 2262.

rubyist commented 7 years ago

This looks good to me, thanks! I'll set a reminder to look at it again in 2262 😸

a-robinson commented 7 years ago

:)

Thanks!