diff --git a/core/breaker/breaker.go b/core/breaker/breaker.go index 9fc4a483..3da681cd 100644 --- a/core/breaker/breaker.go +++ b/core/breaker/breaker.go @@ -53,16 +53,19 @@ type ( // DoWithFallback runs the fallback if the Breaker rejects the request. // If a panic occurs in the request, the Breaker handles it as an error // and causes the same panic again. - DoWithFallback(req func() error, fallback func(err error) error) error + DoWithFallback(req func() error, fallback Fallback) error // DoWithFallbackAcceptable runs the given request if the Breaker accepts it. // DoWithFallbackAcceptable runs the fallback if the Breaker rejects the request. // If a panic occurs in the request, the Breaker handles it as an error // and causes the same panic again. // acceptable checks if it's a successful call, even if the error is not nil. - DoWithFallbackAcceptable(req func() error, fallback func(err error) error, acceptable Acceptable) error + DoWithFallbackAcceptable(req func() error, fallback Fallback, acceptable Acceptable) error } + // Fallback is the func to be called if the request is rejected. + Fallback func(err error) error + // Option defines the method to customize a Breaker. Option func(breaker *circuitBreaker) @@ -86,12 +89,12 @@ type ( internalThrottle interface { allow() (internalPromise, error) - doReq(req func() error, fallback func(err error) error, acceptable Acceptable) error + doReq(req func() error, fallback Fallback, acceptable Acceptable) error } throttle interface { allow() (Promise, error) - doReq(req func() error, fallback func(err error) error, acceptable Acceptable) error + doReq(req func() error, fallback Fallback, acceptable Acceptable) error } ) @@ -122,11 +125,11 @@ func (cb *circuitBreaker) DoWithAcceptable(req func() error, acceptable Acceptab return cb.throttle.doReq(req, nil, acceptable) } -func (cb *circuitBreaker) DoWithFallback(req func() error, fallback func(err error) error) error { +func (cb *circuitBreaker) DoWithFallback(req func() error, fallback Fallback) error { return cb.throttle.doReq(req, fallback, defaultAcceptable) } -func (cb *circuitBreaker) DoWithFallbackAcceptable(req func() error, fallback func(err error) error, +func (cb *circuitBreaker) DoWithFallbackAcceptable(req func() error, fallback Fallback, acceptable Acceptable) error { return cb.throttle.doReq(req, fallback, acceptable) } @@ -168,7 +171,7 @@ func (lt loggedThrottle) allow() (Promise, error) { }, lt.logError(err) } -func (lt loggedThrottle) doReq(req func() error, fallback func(err error) error, acceptable Acceptable) error { +func (lt loggedThrottle) doReq(req func() error, fallback Fallback, acceptable Acceptable) error { return lt.logError(lt.internalThrottle.doReq(req, fallback, func(err error) bool { accept := acceptable(err) if !accept && err != nil { diff --git a/core/breaker/breakers.go b/core/breaker/breakers.go index be55d55a..314965c4 100644 --- a/core/breaker/breakers.go +++ b/core/breaker/breakers.go @@ -22,14 +22,14 @@ func DoWithAcceptable(name string, req func() error, acceptable Acceptable) erro } // DoWithFallback calls Breaker.DoWithFallback on the Breaker with given name. -func DoWithFallback(name string, req func() error, fallback func(err error) error) error { +func DoWithFallback(name string, req func() error, fallback Fallback) error { return do(name, func(b Breaker) error { return b.DoWithFallback(req, fallback) }) } // DoWithFallbackAcceptable calls Breaker.DoWithFallbackAcceptable on the Breaker with given name. -func DoWithFallbackAcceptable(name string, req func() error, fallback func(err error) error, +func DoWithFallbackAcceptable(name string, req func() error, fallback Fallback, acceptable Acceptable) error { return do(name, func(b Breaker) error { return b.DoWithFallbackAcceptable(req, fallback, acceptable) diff --git a/core/breaker/googlebreaker.go b/core/breaker/googlebreaker.go index a8432e30..a15dfba8 100644 --- a/core/breaker/googlebreaker.go +++ b/core/breaker/googlebreaker.go @@ -60,8 +60,9 @@ func (b *googleBreaker) allow() (internalPromise, error) { }, nil } -func (b *googleBreaker) doReq(req func() error, fallback func(err error) error, acceptable Acceptable) error { +func (b *googleBreaker) doReq(req func() error, fallback Fallback, acceptable Acceptable) error { if err := b.accept(); err != nil { + b.markFailure() if fallback != nil { return fallback(err) } @@ -69,18 +70,19 @@ func (b *googleBreaker) doReq(req func() error, fallback func(err error) error, return err } + var success bool defer func() { - if e := recover(); e != nil { + // if req() panic, success is false, mark as failure + if success { + b.markSuccess() + } else { b.markFailure() - panic(e) } }() err := req() if acceptable(err) { - b.markSuccess() - } else { - b.markFailure() + success = true } return err diff --git a/core/breaker/googlebreaker_test.go b/core/breaker/googlebreaker_test.go index 7a8c65b8..85d76660 100644 --- a/core/breaker/googlebreaker_test.go +++ b/core/breaker/googlebreaker_test.go @@ -206,7 +206,7 @@ func BenchmarkGoogleBreakerAllow(b *testing.B) { breaker := getGoogleBreaker() b.ResetTimer() for i := 0; i <= b.N; i++ { - breaker.accept() + _ = breaker.accept() if i%2 == 0 { breaker.markSuccess() } else { @@ -215,6 +215,16 @@ func BenchmarkGoogleBreakerAllow(b *testing.B) { } } +func BenchmarkGoogleBreakerDoReq(b *testing.B) { + breaker := getGoogleBreaker() + b.ResetTimer() + for i := 0; i <= b.N; i++ { + _ = breaker.doReq(func() error { + return nil + }, nil, defaultAcceptable) + } +} + func markSuccess(b *googleBreaker, count int) { for i := 0; i < count; i++ { p, err := b.allow() diff --git a/core/breaker/nopbreaker.go b/core/breaker/nopbreaker.go index a1b2e7cc..cd030b63 100644 --- a/core/breaker/nopbreaker.go +++ b/core/breaker/nopbreaker.go @@ -24,12 +24,11 @@ func (b nopBreaker) DoWithAcceptable(req func() error, _ Acceptable) error { return req() } -func (b nopBreaker) DoWithFallback(req func() error, _ func(err error) error) error { +func (b nopBreaker) DoWithFallback(req func() error, _ Fallback) error { return req() } -func (b nopBreaker) DoWithFallbackAcceptable(req func() error, _ func(err error) error, - _ Acceptable) error { +func (b nopBreaker) DoWithFallbackAcceptable(req func() error, _ Fallback, _ Acceptable) error { return req() } diff --git a/core/stores/mon/collection_test.go b/core/stores/mon/collection_test.go index ac65bbd9..2adc27f0 100644 --- a/core/stores/mon/collection_test.go +++ b/core/stores/mon/collection_test.go @@ -603,11 +603,11 @@ func (d *dropBreaker) DoWithAcceptable(_ func() error, _ breaker.Acceptable) err return errDummy } -func (d *dropBreaker) DoWithFallback(_ func() error, _ func(err error) error) error { +func (d *dropBreaker) DoWithFallback(_ func() error, _ breaker.Fallback) error { return nil } -func (d *dropBreaker) DoWithFallbackAcceptable(_ func() error, _ func(err error) error, +func (d *dropBreaker) DoWithFallbackAcceptable(_ func() error, _ breaker.Fallback, _ breaker.Acceptable) error { return nil }