From d4d5aee55cb6e75618fe970b8d7a039e717a7f54 Mon Sep 17 00:00:00 2001 From: hlts2 Date: Wed, 19 Oct 2022 10:24:30 +0900 Subject: [PATCH 1/7] bugfix cb half-open error handling Signed-off-by: hlts2 --- internal/circuitbreaker/breaker.go | 1 - internal/circuitbreaker/breaker_test.go | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/internal/circuitbreaker/breaker.go b/internal/circuitbreaker/breaker.go index d38dc47b8c..be54260983 100644 --- a/internal/circuitbreaker/breaker.go +++ b/internal/circuitbreaker/breaker.go @@ -109,7 +109,6 @@ func (b *breaker) isReady() (st State, err error) { // If this modulo is used, 1/2 of the requests will be error. And if an error occurs, mark as failures. cnt := b.count.Load().(*count) if cnt.Total()%2 == 0 { - cnt.onFail() return st, errors.ErrCircuitBreakerHalfOpenFlowLimitation } } diff --git a/internal/circuitbreaker/breaker_test.go b/internal/circuitbreaker/breaker_test.go index 0348d2637d..04be06f62c 100644 --- a/internal/circuitbreaker/breaker_test.go +++ b/internal/circuitbreaker/breaker_test.go @@ -362,8 +362,8 @@ func Test_breaker_isReady(t *testing.T) { return err } cnt := atCount.Load().(*count) - if got := cnt.Fails(); got != 1 { - return fmt.Errorf("failures is not equals. want: %d, but got: %d", 2, got) + if got := cnt.Fails(); got != 0 { + return fmt.Errorf("failures is not equals. want: %d, but got: %d", 0, got) } return nil }, From 0f4acbad33b7c4404f25dc1db5c610a2fd0e7ae4 Mon Sep 17 00:00:00 2001 From: hlts2 Date: Wed, 19 Oct 2022 10:31:22 +0900 Subject: [PATCH 2/7] refactor logic Signed-off-by: hlts2 --- internal/circuitbreaker/breaker.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/circuitbreaker/breaker.go b/internal/circuitbreaker/breaker.go index be54260983..944a321ea7 100644 --- a/internal/circuitbreaker/breaker.go +++ b/internal/circuitbreaker/breaker.go @@ -107,8 +107,7 @@ func (b *breaker) isReady() (st State, err error) { // For flow control in the "Half-Open" state. It is limited to 50%. // If this modulo is used, 1/2 of the requests will be error. And if an error occurs, mark as failures. - cnt := b.count.Load().(*count) - if cnt.Total()%2 == 0 { + if b.count.Load().(*count).Total()%2 == 0 { return st, errors.ErrCircuitBreakerHalfOpenFlowLimitation } } From ba3136a8a2853579b84bc85d2b49753d3dc921a0 Mon Sep 17 00:00:00 2001 From: hlts2 Date: Wed, 19 Oct 2022 11:39:12 +0900 Subject: [PATCH 3/7] add fail function test Signed-off-by: hlts2 --- internal/circuitbreaker/breaker_test.go | 160 ++++++++++++++---------- 1 file changed, 97 insertions(+), 63 deletions(-) diff --git a/internal/circuitbreaker/breaker_test.go b/internal/circuitbreaker/breaker_test.go index 04be06f62c..c31c6267a9 100644 --- a/internal/circuitbreaker/breaker_test.go +++ b/internal/circuitbreaker/breaker_test.go @@ -570,71 +570,105 @@ func Test_breaker_fail(t *testing.T) { want want checkFunc func(want) error beforeFunc func(*testing.T) - afterFunc func(*testing.T) + afterFunc func(*testing.T, *breaker) } defaultCheckFunc := func(w want) error { return nil } tests := []test{ - // TODO test cases - /* - { - name: "test_case_1", - fields: fields { - key: "", - count: nil, - tripped: 0, - closedErrRate: 0, - closedErrShouldTrip: nil, - halfOpenErrRate: 0, - halfOpenErrShouldTrip: nil, - minSamples: 0, - openTimeout: nil, - openExp: 0, - cloedRefreshTimeout: nil, - closedRefreshExp: 0, - }, - want: want{}, - checkFunc: defaultCheckFunc, - beforeFunc: func(t *testing.T,) { - t.Helper() - }, - afterFunc: func(t *testing.T,) { - t.Helper() - }, - }, - */ - - // TODO test cases - /* - func() test { - return test { - name: "test_case_2", - fields: fields { - key: "", - count: nil, - tripped: 0, - closedErrRate: 0, - closedErrShouldTrip: nil, - halfOpenErrRate: 0, - halfOpenErrShouldTrip: nil, - minSamples: 0, - openTimeout: nil, - openExp: 0, - cloedRefreshTimeout: nil, - closedRefreshExp: 0, - }, - want: want{}, - checkFunc: defaultCheckFunc, - beforeFunc: func(t *testing.T,) { - t.Helper() - }, - afterFunc: func(t *testing.T,) { - t.Helper() - }, - } - }(), - */ + func() test { + var atCount atomic.Value + atCount.Store(&count{ + successes: 10, + failures: 11, + }) + closedErrRate := float32(0.5) + minSamples := int64(20) + return test{ + name: "the current state change from Close to Open when the failure rate is higher", + fields: fields{ + key: "insertRPC", + count: atCount, + tripped: 0, + closedErrRate: closedErrRate, + closedRefreshExp: time.Now().Add(100 * time.Second).UnixNano(), + closedErrShouldTrip: NewRateTripper(closedErrRate, minSamples), + minSamples: minSamples, + }, + checkFunc: defaultCheckFunc, + afterFunc: func(t *testing.T, b *breaker) { + t.Helper() + if b.tripped == 0 { + t.Errorf("state did not change: %d", b.tripped) + } + if total := b.count.Load().(*count).Total(); total != 0 { + t.Errorf("count did not reset: %d", total) + } + }, + } + }(), + func() test { + var atCount atomic.Value + atCount.Store(&count{ + successes: 10, + failures: 11, + }) + halfOpenErrRate := float32(0.5) + minSamples := int64(20) + return test{ + name: "the current state change from HalfOpen to Open when the failure rate is higher", + fields: fields{ + key: "insertRPC", + count: atCount, + tripped: 1, + openExp: time.Now().Add(-100 * time.Second).UnixNano(), + halfOpenErrRate: halfOpenErrRate, + halfOpenErrShouldTrip: NewRateTripper(halfOpenErrRate, minSamples), + minSamples: minSamples, + }, + checkFunc: defaultCheckFunc, + afterFunc: func(t *testing.T, b *breaker) { + t.Helper() + if b.tripped == 0 { + t.Errorf("state changed: %d", b.tripped) + } + if total := b.count.Load().(*count).Total(); total != 0 { + t.Errorf("count did not reset: %d", total) + } + }, + } + }(), + func() test { + var atCount atomic.Value + atCount.Store(&count{ + successes: 10, + failures: 1, + }) + halfOpenErrRate := float32(0.5) + minSamples := int64(10) + return test{ + name: "the current HalfOpen state dot not change when the failure rate does not reached the setting value", + fields: fields{ + key: "insertRPC", + count: atCount, + tripped: 1, + openExp: time.Now().Add(-100 * time.Second).UnixNano(), + halfOpenErrRate: halfOpenErrRate, + halfOpenErrShouldTrip: NewRateTripper(halfOpenErrRate, minSamples), + minSamples: minSamples, + }, + checkFunc: defaultCheckFunc, + afterFunc: func(t *testing.T, b *breaker) { + t.Helper() + if b.tripped == 0 { + t.Errorf("state changed: %d", b.tripped) + } + if total := b.count.Load().(*count).Total(); total == 0 { + t.Errorf("count reseted: %d", total) + } + }, + } + }(), } for _, tc := range tests { @@ -645,9 +679,6 @@ func Test_breaker_fail(t *testing.T) { if test.beforeFunc != nil { test.beforeFunc(tt) } - if test.afterFunc != nil { - defer test.afterFunc(tt) - } checkFunc := test.checkFunc if test.checkFunc == nil { checkFunc = defaultCheckFunc @@ -666,6 +697,9 @@ func Test_breaker_fail(t *testing.T) { cloedRefreshTimeout: test.fields.cloedRefreshTimeout, closedRefreshExp: test.fields.closedRefreshExp, } + if test.afterFunc != nil { + defer test.afterFunc(tt, b) + } b.fail() if err := checkFunc(test.want); err != nil { From 13429d2ff9fb52959a6b3a0b3d3846630af7a648 Mon Sep 17 00:00:00 2001 From: hlts2 Date: Wed, 19 Oct 2022 13:39:51 +0900 Subject: [PATCH 4/7] add ignores counter for internal state error Signed-off-by: hlts2 --- internal/circuitbreaker/breaker.go | 3 ++- internal/circuitbreaker/counter.go | 6 ++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/internal/circuitbreaker/breaker.go b/internal/circuitbreaker/breaker.go index 944a321ea7..5d284258e2 100644 --- a/internal/circuitbreaker/breaker.go +++ b/internal/circuitbreaker/breaker.go @@ -69,6 +69,7 @@ func newBreaker(key string, opts ...BreakerOption) (*breaker, error) { // If the current breaker state is "Open", this function returns ErrCircuitBreakerOpenState. func (b *breaker) do(ctx context.Context, fn func(ctx context.Context) (val interface{}, err error)) (val interface{}, st State, err error) { if st, err := b.isReady(); err != nil { + b.count.Load().(*count).onIgnore() return nil, st, err } val, err = fn(ctx) @@ -121,7 +122,7 @@ func (b *breaker) success() { // halfOpenErrShouldTrip.ShouldTrip returns true when the sum of the number of successes and failures is greater than the b.minSamples and when the error rate is greater than the b.halfOpenErrRate. // In other words, if the error rate is less than the b.halfOpenErrRate, it can be judged that the success rate is high, so this function change to the "Close" state from "Half-Open". if st := b.currentState(); st == StateHalfOpen && - cnt.Total() >= b.minSamples && + cnt.Successes()+cnt.Fails() >= b.minSamples && !b.halfOpenErrShouldTrip.ShouldTrip(cnt) { log.Infof("the operation succeeded, circuit breaker state for '%s' changed,\tfrom: %s, to: %s", b.key, st.String(), StateClosed.String()) b.reset() diff --git a/internal/circuitbreaker/counter.go b/internal/circuitbreaker/counter.go index bc20e906c7..b769dd44f1 100644 --- a/internal/circuitbreaker/counter.go +++ b/internal/circuitbreaker/counter.go @@ -22,6 +22,7 @@ type Counter interface { } type count struct { + ignores int64 successes int64 failures int64 } @@ -46,9 +47,14 @@ func (c *count) onFail() { atomic.AddInt64(&c.failures, 1) } +func (c *count) onIgnore() { + atomic.AddInt64(&c.ignores, 1) +} + func (c *count) reset() { atomic.StoreInt64(&c.failures, 0) atomic.StoreInt64(&c.successes, 0) + atomic.StoreInt64(&c.ignores, 0) } var _ Counter = (*count)(nil) From 432296479861d653888bdc597ca2557863e431a7 Mon Sep 17 00:00:00 2001 From: hlts2 Date: Wed, 19 Oct 2022 14:08:52 +0900 Subject: [PATCH 5/7] fix counter type from atomic to pure pointer Signed-off-by: hlts2 --- internal/circuitbreaker/breaker.go | 32 +- internal/circuitbreaker/breaker_test.go | 71 ++-- internal/circuitbreaker/counter.go | 6 +- internal/circuitbreaker/counter_test.go | 419 ++++++++++++++++++++++-- 4 files changed, 446 insertions(+), 82 deletions(-) diff --git a/internal/circuitbreaker/breaker.go b/internal/circuitbreaker/breaker.go index 5d284258e2..4e29ab1eb8 100644 --- a/internal/circuitbreaker/breaker.go +++ b/internal/circuitbreaker/breaker.go @@ -24,9 +24,9 @@ import ( ) type breaker struct { - key string // breaker key for logging - count atomic.Value // type: *count - tripped int32 // tripped flag. when flag value is 1, breaker state is "Open" or "HalfOpen". + key string // breaker key for logging + count *count // type: *count + tripped int32 // tripped flag. when flag value is 1, breaker state is "Open" or "HalfOpen". closedErrRate float32 closedErrShouldTrip Tripper @@ -41,7 +41,8 @@ type breaker struct { func newBreaker(key string, opts ...BreakerOption) (*breaker, error) { b := &breaker{ - key: key, + key: key, + count: &count{}, } for _, opt := range append(defaultBreakerOpts, opts...) { if err := opt(b); err != nil { @@ -54,7 +55,6 @@ func newBreaker(key string, opts ...BreakerOption) (*breaker, error) { log.Warn(oerr) } } - b.count.Store(&count{}) if b.closedErrShouldTrip == nil { b.closedErrShouldTrip = NewRateTripper(b.closedErrRate, b.minSamples) @@ -69,7 +69,7 @@ func newBreaker(key string, opts ...BreakerOption) (*breaker, error) { // If the current breaker state is "Open", this function returns ErrCircuitBreakerOpenState. func (b *breaker) do(ctx context.Context, fn func(ctx context.Context) (val interface{}, err error)) (val interface{}, st State, err error) { if st, err := b.isReady(); err != nil { - b.count.Load().(*count).onIgnore() + b.count.onIgnore() return nil, st, err } val, err = fn(ctx) @@ -108,7 +108,7 @@ func (b *breaker) isReady() (st State, err error) { // For flow control in the "Half-Open" state. It is limited to 50%. // If this modulo is used, 1/2 of the requests will be error. And if an error occurs, mark as failures. - if b.count.Load().(*count).Total()%2 == 0 { + if b.count.Total()%2 == 0 { return st, errors.ErrCircuitBreakerHalfOpenFlowLimitation } } @@ -116,30 +116,28 @@ func (b *breaker) isReady() (st State, err error) { } func (b *breaker) success() { - cnt := b.count.Load().(*count) - cnt.onSuccess() + b.count.onSuccess() // halfOpenErrShouldTrip.ShouldTrip returns true when the sum of the number of successes and failures is greater than the b.minSamples and when the error rate is greater than the b.halfOpenErrRate. // In other words, if the error rate is less than the b.halfOpenErrRate, it can be judged that the success rate is high, so this function change to the "Close" state from "Half-Open". if st := b.currentState(); st == StateHalfOpen && - cnt.Successes()+cnt.Fails() >= b.minSamples && - !b.halfOpenErrShouldTrip.ShouldTrip(cnt) { + b.count.Successes()+b.count.Fails() >= b.minSamples && + !b.halfOpenErrShouldTrip.ShouldTrip(b.count) { log.Infof("the operation succeeded, circuit breaker state for '%s' changed,\tfrom: %s, to: %s", b.key, st.String(), StateClosed.String()) b.reset() } } func (b *breaker) fail() { - cnt := b.count.Load().(*count) - cnt.onFail() + b.count.onFail() var ok bool var st State switch st = b.currentState(); st { case StateHalfOpen: - ok = b.halfOpenErrShouldTrip.ShouldTrip(cnt) + ok = b.halfOpenErrShouldTrip.ShouldTrip(b.count) case StateClosed: - ok = b.closedErrShouldTrip.ShouldTrip(cnt) + ok = b.closedErrShouldTrip.ShouldTrip(b.count) default: return } @@ -171,14 +169,14 @@ func (b *breaker) reset() { atomic.StoreInt32(&b.tripped, 0) atomic.StoreInt64(&b.openExp, 0) atomic.StoreInt64(&b.closedRefreshExp, time.Now().Add(b.cloedRefreshTimeout).UnixNano()) - b.count.Load().(*count).reset() + b.count.reset() } func (b *breaker) trip() { atomic.StoreInt32(&b.tripped, 1) atomic.StoreInt64(&b.openExp, time.Now().Add(b.openTimeout).UnixNano()) atomic.StoreInt64(&b.closedRefreshExp, 0) - b.count.Load().(*count).reset() + b.count.reset() } func (b *breaker) isTripped() (ok bool) { diff --git a/internal/circuitbreaker/breaker_test.go b/internal/circuitbreaker/breaker_test.go index c31c6267a9..4ba1202be2 100644 --- a/internal/circuitbreaker/breaker_test.go +++ b/internal/circuitbreaker/breaker_test.go @@ -17,7 +17,6 @@ import ( "context" "fmt" "reflect" - "sync/atomic" "testing" "time" @@ -124,7 +123,7 @@ func Test_breaker_do(t *testing.T) { } type fields struct { key string - count atomic.Value + count *count tripped int32 closedErrRate float32 closedErrShouldTrip Tripper @@ -273,7 +272,7 @@ func Test_breaker_do(t *testing.T) { func Test_breaker_isReady(t *testing.T) { type fields struct { key string - count atomic.Value + count *count tripped int32 closedErrRate float32 closedErrShouldTrip Tripper @@ -323,17 +322,16 @@ func Test_breaker_isReady(t *testing.T) { } }(), func() test { - var atCount atomic.Value - atCount.Store(&count{ + cnt := &count{ successes: 1, - }) + } return test{ name: "return the StateHalfOpen and nil when the current state is HalfOpen", fields: fields{ key: "insertRPC", tripped: 1, openExp: time.Now().Add(-100 * time.Second).UnixNano(), - count: atCount, + count: cnt, }, want: want{ wantSt: StateHalfOpen, @@ -343,15 +341,14 @@ func Test_breaker_isReady(t *testing.T) { } }(), func() test { - var atCount atomic.Value - atCount.Store(&count{}) + cnt := &count{} return test{ name: "return the StateHalfOpen and error when the current state is HalfOpen but the flow is being limited", fields: fields{ key: "insertRPC", tripped: 1, openExp: time.Now().Add(-100 * time.Second).UnixNano(), - count: atCount, + count: cnt, }, want: want{ wantSt: StateHalfOpen, @@ -361,7 +358,6 @@ func Test_breaker_isReady(t *testing.T) { if err := defaultCheckFunc(w, s, err); err != nil { return err } - cnt := atCount.Load().(*count) if got := cnt.Fails(); got != 0 { return fmt.Errorf("failures is not equals. want: %d, but got: %d", 0, got) } @@ -427,7 +423,7 @@ func Test_breaker_isReady(t *testing.T) { func Test_breaker_success(t *testing.T) { type fields struct { key string - count atomic.Value + count *count tripped int32 closedErrRate float32 closedErrShouldTrip Tripper @@ -453,18 +449,17 @@ func Test_breaker_success(t *testing.T) { } tests := []test{ func() test { - var atCount atomic.Value - atCount.Store(&count{ + cnt := &count{ successes: 10, failures: 10, - }) + } halfOpenErrRate := float32(0.5) minSamples := int64(10) return test{ name: "the current state change from HalfOpen to Close when the success rate is higher", fields: fields{ key: "insertRPC", - count: atCount, + count: cnt, tripped: 1, openExp: time.Now().Add(-100 * time.Second).UnixNano(), halfOpenErrRate: halfOpenErrRate, @@ -481,18 +476,17 @@ func Test_breaker_success(t *testing.T) { } }(), func() test { - var atCount atomic.Value - atCount.Store(&count{ + cnt := &count{ successes: 10, failures: 11, - }) + } halfOpenErrRate := float32(0.5) minSamples := int64(10) return test{ name: "the current state do not change from HalfOpen to Close when the success rate is less", fields: fields{ key: "insertRPC", - count: atCount, + count: cnt, tripped: 1, openExp: time.Now().Add(-100 * time.Second).UnixNano(), halfOpenErrRate: halfOpenErrRate, @@ -551,7 +545,7 @@ func Test_breaker_success(t *testing.T) { func Test_breaker_fail(t *testing.T) { type fields struct { key string - count atomic.Value + count *count tripped int32 closedErrRate float32 closedErrShouldTrip Tripper @@ -577,18 +571,17 @@ func Test_breaker_fail(t *testing.T) { } tests := []test{ func() test { - var atCount atomic.Value - atCount.Store(&count{ + cnt := &count{ successes: 10, failures: 11, - }) + } closedErrRate := float32(0.5) minSamples := int64(20) return test{ name: "the current state change from Close to Open when the failure rate is higher", fields: fields{ key: "insertRPC", - count: atCount, + count: cnt, tripped: 0, closedErrRate: closedErrRate, closedRefreshExp: time.Now().Add(100 * time.Second).UnixNano(), @@ -601,25 +594,24 @@ func Test_breaker_fail(t *testing.T) { if b.tripped == 0 { t.Errorf("state did not change: %d", b.tripped) } - if total := b.count.Load().(*count).Total(); total != 0 { + if total := cnt.Total(); total != 0 { t.Errorf("count did not reset: %d", total) } }, } }(), func() test { - var atCount atomic.Value - atCount.Store(&count{ + cnt := &count{ successes: 10, failures: 11, - }) + } halfOpenErrRate := float32(0.5) minSamples := int64(20) return test{ name: "the current state change from HalfOpen to Open when the failure rate is higher", fields: fields{ key: "insertRPC", - count: atCount, + count: cnt, tripped: 1, openExp: time.Now().Add(-100 * time.Second).UnixNano(), halfOpenErrRate: halfOpenErrRate, @@ -632,25 +624,24 @@ func Test_breaker_fail(t *testing.T) { if b.tripped == 0 { t.Errorf("state changed: %d", b.tripped) } - if total := b.count.Load().(*count).Total(); total != 0 { + if total := b.count.Total(); total != 0 { t.Errorf("count did not reset: %d", total) } }, } }(), func() test { - var atCount atomic.Value - atCount.Store(&count{ + cnt := &count{ successes: 10, failures: 1, - }) + } halfOpenErrRate := float32(0.5) minSamples := int64(10) return test{ name: "the current HalfOpen state dot not change when the failure rate does not reached the setting value", fields: fields{ key: "insertRPC", - count: atCount, + count: cnt, tripped: 1, openExp: time.Now().Add(-100 * time.Second).UnixNano(), halfOpenErrRate: halfOpenErrRate, @@ -663,7 +654,7 @@ func Test_breaker_fail(t *testing.T) { if b.tripped == 0 { t.Errorf("state changed: %d", b.tripped) } - if total := b.count.Load().(*count).Total(); total == 0 { + if total := b.count.Total(); total == 0 { t.Errorf("count reseted: %d", total) } }, @@ -712,7 +703,7 @@ func Test_breaker_fail(t *testing.T) { func Test_breaker_currentState(t *testing.T) { type fields struct { key string - count atomic.Value + count *count tripped int32 closedErrRate float32 closedErrShouldTrip Tripper @@ -844,7 +835,7 @@ func Test_breaker_currentState(t *testing.T) { func Test_breaker_reset(t *testing.T) { type fields struct { key string - count atomic.Value + count *count tripped int32 closedErrRate float32 closedErrShouldTrip Tripper @@ -971,7 +962,7 @@ func Test_breaker_reset(t *testing.T) { func Test_breaker_trip(t *testing.T) { type fields struct { key string - count atomic.Value + count *count tripped int32 closedErrRate float32 closedErrShouldTrip Tripper @@ -1098,7 +1089,7 @@ func Test_breaker_trip(t *testing.T) { func Test_breaker_isTripped(t *testing.T) { type fields struct { key string - count atomic.Value + count *count tripped int32 closedErrRate float32 closedErrShouldTrip Tripper diff --git a/internal/circuitbreaker/counter.go b/internal/circuitbreaker/counter.go index b769dd44f1..d5fc58b39f 100644 --- a/internal/circuitbreaker/counter.go +++ b/internal/circuitbreaker/counter.go @@ -35,8 +35,12 @@ func (c *count) Fails() (n int64) { return atomic.LoadInt64(&c.failures) } +func (c *count) Ignores() (n int64) { + return atomic.LoadInt64(&c.ignores) +} + func (c *count) Total() (n int64) { - return c.Successes() + c.Fails() + return c.Successes() + c.Fails() + c.Ignores() } func (c *count) onSuccess() { diff --git a/internal/circuitbreaker/counter_test.go b/internal/circuitbreaker/counter_test.go index 025bc909b4..2a55b288d4 100644 --- a/internal/circuitbreaker/counter_test.go +++ b/internal/circuitbreaker/counter_test.go @@ -17,12 +17,13 @@ import ( "reflect" "testing" - "github.com/vdaas/vald/internal/errors" + "github.com/pkg/errors" "github.com/vdaas/vald/internal/test/goleak" ) func Test_count_Successes(t *testing.T) { type fields struct { + ignores int64 successes int64 failures int64 } @@ -34,8 +35,8 @@ func Test_count_Successes(t *testing.T) { fields fields want want checkFunc func(want, int64) error - beforeFunc func() - afterFunc func() + beforeFunc func(*testing.T) + afterFunc func(*testing.T) } defaultCheckFunc := func(w want, gotN int64) error { if !reflect.DeepEqual(gotN, w.wantN) { @@ -49,11 +50,18 @@ func Test_count_Successes(t *testing.T) { { name: "test_case_1", fields: fields { + ignores: 0, successes: 0, failures: 0, }, want: want{}, checkFunc: defaultCheckFunc, + beforeFunc: func(t *testing.T,) { + t.Helper() + }, + afterFunc: func(t *testing.T,) { + t.Helper() + }, }, */ @@ -63,11 +71,18 @@ func Test_count_Successes(t *testing.T) { return test { name: "test_case_2", fields: fields { + ignores: 0, successes: 0, failures: 0, }, want: want{}, checkFunc: defaultCheckFunc, + beforeFunc: func(t *testing.T,) { + t.Helper() + }, + afterFunc: func(t *testing.T,) { + t.Helper() + }, } }(), */ @@ -79,16 +94,17 @@ func Test_count_Successes(t *testing.T) { tt.Parallel() defer goleak.VerifyNone(tt, goleak.IgnoreCurrent()) if test.beforeFunc != nil { - test.beforeFunc() + test.beforeFunc(tt) } if test.afterFunc != nil { - defer test.afterFunc() + defer test.afterFunc(tt) } checkFunc := test.checkFunc if test.checkFunc == nil { checkFunc = defaultCheckFunc } c := &count{ + ignores: test.fields.ignores, successes: test.fields.successes, failures: test.fields.failures, } @@ -97,12 +113,14 @@ func Test_count_Successes(t *testing.T) { if err := checkFunc(test.want, gotN); err != nil { tt.Errorf("error = %v", err) } + }) } } func Test_count_Fails(t *testing.T) { type fields struct { + ignores int64 successes int64 failures int64 } @@ -114,8 +132,8 @@ func Test_count_Fails(t *testing.T) { fields fields want want checkFunc func(want, int64) error - beforeFunc func() - afterFunc func() + beforeFunc func(*testing.T) + afterFunc func(*testing.T) } defaultCheckFunc := func(w want, gotN int64) error { if !reflect.DeepEqual(gotN, w.wantN) { @@ -129,11 +147,18 @@ func Test_count_Fails(t *testing.T) { { name: "test_case_1", fields: fields { + ignores: 0, successes: 0, failures: 0, }, want: want{}, checkFunc: defaultCheckFunc, + beforeFunc: func(t *testing.T,) { + t.Helper() + }, + afterFunc: func(t *testing.T,) { + t.Helper() + }, }, */ @@ -143,11 +168,18 @@ func Test_count_Fails(t *testing.T) { return test { name: "test_case_2", fields: fields { + ignores: 0, successes: 0, failures: 0, }, want: want{}, checkFunc: defaultCheckFunc, + beforeFunc: func(t *testing.T,) { + t.Helper() + }, + afterFunc: func(t *testing.T,) { + t.Helper() + }, } }(), */ @@ -159,16 +191,17 @@ func Test_count_Fails(t *testing.T) { tt.Parallel() defer goleak.VerifyNone(tt, goleak.IgnoreCurrent()) if test.beforeFunc != nil { - test.beforeFunc() + test.beforeFunc(tt) } if test.afterFunc != nil { - defer test.afterFunc() + defer test.afterFunc(tt) } checkFunc := test.checkFunc if test.checkFunc == nil { checkFunc = defaultCheckFunc } c := &count{ + ignores: test.fields.ignores, successes: test.fields.successes, failures: test.fields.failures, } @@ -177,23 +210,220 @@ func Test_count_Fails(t *testing.T) { if err := checkFunc(test.want, gotN); err != nil { tt.Errorf("error = %v", err) } + + }) + } +} + +func Test_count_Ignores(t *testing.T) { + type fields struct { + ignores int64 + successes int64 + failures int64 + } + type want struct { + wantN int64 + } + type test struct { + name string + fields fields + want want + checkFunc func(want, int64) error + beforeFunc func(*testing.T) + afterFunc func(*testing.T) + } + defaultCheckFunc := func(w want, gotN int64) error { + if !reflect.DeepEqual(gotN, w.wantN) { + return errors.Errorf("got: \"%#v\",\n\t\t\t\twant: \"%#v\"", gotN, w.wantN) + } + return nil + } + tests := []test{ + // TODO test cases + /* + { + name: "test_case_1", + fields: fields { + ignores: 0, + successes: 0, + failures: 0, + }, + want: want{}, + checkFunc: defaultCheckFunc, + beforeFunc: func(t *testing.T,) { + t.Helper() + }, + afterFunc: func(t *testing.T,) { + t.Helper() + }, + }, + */ + + // TODO test cases + /* + func() test { + return test { + name: "test_case_2", + fields: fields { + ignores: 0, + successes: 0, + failures: 0, + }, + want: want{}, + checkFunc: defaultCheckFunc, + beforeFunc: func(t *testing.T,) { + t.Helper() + }, + afterFunc: func(t *testing.T,) { + t.Helper() + }, + } + }(), + */ + } + + for _, tc := range tests { + test := tc + t.Run(test.name, func(tt *testing.T) { + tt.Parallel() + defer goleak.VerifyNone(tt, goleak.IgnoreCurrent()) + if test.beforeFunc != nil { + test.beforeFunc(tt) + } + if test.afterFunc != nil { + defer test.afterFunc(tt) + } + checkFunc := test.checkFunc + if test.checkFunc == nil { + checkFunc = defaultCheckFunc + } + c := &count{ + ignores: test.fields.ignores, + successes: test.fields.successes, + failures: test.fields.failures, + } + + gotN := c.Ignores() + if err := checkFunc(test.want, gotN); err != nil { + tt.Errorf("error = %v", err) + } + + }) + } +} + +func Test_count_Total(t *testing.T) { + type fields struct { + ignores int64 + successes int64 + failures int64 + } + type want struct { + wantN int64 + } + type test struct { + name string + fields fields + want want + checkFunc func(want, int64) error + beforeFunc func(*testing.T) + afterFunc func(*testing.T) + } + defaultCheckFunc := func(w want, gotN int64) error { + if !reflect.DeepEqual(gotN, w.wantN) { + return errors.Errorf("got: \"%#v\",\n\t\t\t\twant: \"%#v\"", gotN, w.wantN) + } + return nil + } + tests := []test{ + // TODO test cases + /* + { + name: "test_case_1", + fields: fields { + ignores: 0, + successes: 0, + failures: 0, + }, + want: want{}, + checkFunc: defaultCheckFunc, + beforeFunc: func(t *testing.T,) { + t.Helper() + }, + afterFunc: func(t *testing.T,) { + t.Helper() + }, + }, + */ + + // TODO test cases + /* + func() test { + return test { + name: "test_case_2", + fields: fields { + ignores: 0, + successes: 0, + failures: 0, + }, + want: want{}, + checkFunc: defaultCheckFunc, + beforeFunc: func(t *testing.T,) { + t.Helper() + }, + afterFunc: func(t *testing.T,) { + t.Helper() + }, + } + }(), + */ + } + + for _, tc := range tests { + test := tc + t.Run(test.name, func(tt *testing.T) { + tt.Parallel() + defer goleak.VerifyNone(tt, goleak.IgnoreCurrent()) + if test.beforeFunc != nil { + test.beforeFunc(tt) + } + if test.afterFunc != nil { + defer test.afterFunc(tt) + } + checkFunc := test.checkFunc + if test.checkFunc == nil { + checkFunc = defaultCheckFunc + } + c := &count{ + ignores: test.fields.ignores, + successes: test.fields.successes, + failures: test.fields.failures, + } + + gotN := c.Total() + if err := checkFunc(test.want, gotN); err != nil { + tt.Errorf("error = %v", err) + } + }) } } func Test_count_onSuccess(t *testing.T) { type fields struct { + ignores int64 successes int64 failures int64 } - type want struct{} + type want struct { + } type test struct { name string fields fields want want checkFunc func(want) error - beforeFunc func() - afterFunc func() + beforeFunc func(*testing.T) + afterFunc func(*testing.T) } defaultCheckFunc := func(w want) error { return nil @@ -204,11 +434,18 @@ func Test_count_onSuccess(t *testing.T) { { name: "test_case_1", fields: fields { + ignores: 0, successes: 0, failures: 0, }, want: want{}, checkFunc: defaultCheckFunc, + beforeFunc: func(t *testing.T,) { + t.Helper() + }, + afterFunc: func(t *testing.T,) { + t.Helper() + }, }, */ @@ -218,11 +455,18 @@ func Test_count_onSuccess(t *testing.T) { return test { name: "test_case_2", fields: fields { + ignores: 0, successes: 0, failures: 0, }, want: want{}, checkFunc: defaultCheckFunc, + beforeFunc: func(t *testing.T,) { + t.Helper() + }, + afterFunc: func(t *testing.T,) { + t.Helper() + }, } }(), */ @@ -234,16 +478,17 @@ func Test_count_onSuccess(t *testing.T) { tt.Parallel() defer goleak.VerifyNone(tt, goleak.IgnoreCurrent()) if test.beforeFunc != nil { - test.beforeFunc() + test.beforeFunc(tt) } if test.afterFunc != nil { - defer test.afterFunc() + defer test.afterFunc(tt) } checkFunc := test.checkFunc if test.checkFunc == nil { checkFunc = defaultCheckFunc } c := &count{ + ignores: test.fields.ignores, successes: test.fields.successes, failures: test.fields.failures, } @@ -258,17 +503,19 @@ func Test_count_onSuccess(t *testing.T) { func Test_count_onFail(t *testing.T) { type fields struct { + ignores int64 successes int64 failures int64 } - type want struct{} + type want struct { + } type test struct { name string fields fields want want checkFunc func(want) error - beforeFunc func() - afterFunc func() + beforeFunc func(*testing.T) + afterFunc func(*testing.T) } defaultCheckFunc := func(w want) error { return nil @@ -279,11 +526,18 @@ func Test_count_onFail(t *testing.T) { { name: "test_case_1", fields: fields { + ignores: 0, successes: 0, failures: 0, }, want: want{}, checkFunc: defaultCheckFunc, + beforeFunc: func(t *testing.T,) { + t.Helper() + }, + afterFunc: func(t *testing.T,) { + t.Helper() + }, }, */ @@ -293,11 +547,18 @@ func Test_count_onFail(t *testing.T) { return test { name: "test_case_2", fields: fields { + ignores: 0, successes: 0, failures: 0, }, want: want{}, checkFunc: defaultCheckFunc, + beforeFunc: func(t *testing.T,) { + t.Helper() + }, + afterFunc: func(t *testing.T,) { + t.Helper() + }, } }(), */ @@ -309,16 +570,17 @@ func Test_count_onFail(t *testing.T) { tt.Parallel() defer goleak.VerifyNone(tt, goleak.IgnoreCurrent()) if test.beforeFunc != nil { - test.beforeFunc() + test.beforeFunc(tt) } if test.afterFunc != nil { - defer test.afterFunc() + defer test.afterFunc(tt) } checkFunc := test.checkFunc if test.checkFunc == nil { checkFunc = defaultCheckFunc } c := &count{ + ignores: test.fields.ignores, successes: test.fields.successes, failures: test.fields.failures, } @@ -331,19 +593,113 @@ func Test_count_onFail(t *testing.T) { } } +func Test_count_onIgnore(t *testing.T) { + type fields struct { + ignores int64 + successes int64 + failures int64 + } + type want struct { + } + type test struct { + name string + fields fields + want want + checkFunc func(want) error + beforeFunc func(*testing.T) + afterFunc func(*testing.T) + } + defaultCheckFunc := func(w want) error { + return nil + } + tests := []test{ + // TODO test cases + /* + { + name: "test_case_1", + fields: fields { + ignores: 0, + successes: 0, + failures: 0, + }, + want: want{}, + checkFunc: defaultCheckFunc, + beforeFunc: func(t *testing.T,) { + t.Helper() + }, + afterFunc: func(t *testing.T,) { + t.Helper() + }, + }, + */ + + // TODO test cases + /* + func() test { + return test { + name: "test_case_2", + fields: fields { + ignores: 0, + successes: 0, + failures: 0, + }, + want: want{}, + checkFunc: defaultCheckFunc, + beforeFunc: func(t *testing.T,) { + t.Helper() + }, + afterFunc: func(t *testing.T,) { + t.Helper() + }, + } + }(), + */ + } + + for _, tc := range tests { + test := tc + t.Run(test.name, func(tt *testing.T) { + tt.Parallel() + defer goleak.VerifyNone(tt, goleak.IgnoreCurrent()) + if test.beforeFunc != nil { + test.beforeFunc(tt) + } + if test.afterFunc != nil { + defer test.afterFunc(tt) + } + checkFunc := test.checkFunc + if test.checkFunc == nil { + checkFunc = defaultCheckFunc + } + c := &count{ + ignores: test.fields.ignores, + successes: test.fields.successes, + failures: test.fields.failures, + } + + c.onIgnore() + if err := checkFunc(test.want); err != nil { + tt.Errorf("error = %v", err) + } + }) + } +} + func Test_count_reset(t *testing.T) { type fields struct { + ignores int64 successes int64 failures int64 } - type want struct{} + type want struct { + } type test struct { name string fields fields want want checkFunc func(want) error - beforeFunc func() - afterFunc func() + beforeFunc func(*testing.T) + afterFunc func(*testing.T) } defaultCheckFunc := func(w want) error { return nil @@ -354,11 +710,18 @@ func Test_count_reset(t *testing.T) { { name: "test_case_1", fields: fields { + ignores: 0, successes: 0, failures: 0, }, want: want{}, checkFunc: defaultCheckFunc, + beforeFunc: func(t *testing.T,) { + t.Helper() + }, + afterFunc: func(t *testing.T,) { + t.Helper() + }, }, */ @@ -368,11 +731,18 @@ func Test_count_reset(t *testing.T) { return test { name: "test_case_2", fields: fields { + ignores: 0, successes: 0, failures: 0, }, want: want{}, checkFunc: defaultCheckFunc, + beforeFunc: func(t *testing.T,) { + t.Helper() + }, + afterFunc: func(t *testing.T,) { + t.Helper() + }, } }(), */ @@ -384,16 +754,17 @@ func Test_count_reset(t *testing.T) { tt.Parallel() defer goleak.VerifyNone(tt, goleak.IgnoreCurrent()) if test.beforeFunc != nil { - test.beforeFunc() + test.beforeFunc(tt) } if test.afterFunc != nil { - defer test.afterFunc() + defer test.afterFunc(tt) } checkFunc := test.checkFunc if test.checkFunc == nil { checkFunc = defaultCheckFunc } c := &count{ + ignores: test.fields.ignores, successes: test.fields.successes, failures: test.fields.failures, } From 63923a9ae5b9c2cc23a03606cc98679336461c0b Mon Sep 17 00:00:00 2001 From: hlts2 Date: Wed, 19 Oct 2022 14:17:17 +0900 Subject: [PATCH 6/7] fix counter interface Signed-off-by: hlts2 --- internal/circuitbreaker/counter.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/circuitbreaker/counter.go b/internal/circuitbreaker/counter.go index d5fc58b39f..9ca08a4ea7 100644 --- a/internal/circuitbreaker/counter.go +++ b/internal/circuitbreaker/counter.go @@ -19,6 +19,7 @@ type Counter interface { Total() int64 Successes() int64 Fails() int64 + Ignores() int64 } type count struct { From 41ec6cacfbba39b3c85c0c4e629d2b4052eba06f Mon Sep 17 00:00:00 2001 From: hlts2 Date: Wed, 19 Oct 2022 14:47:17 +0900 Subject: [PATCH 7/7] make format Signed-off-by: hlts2 --- internal/circuitbreaker/counter_test.go | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/internal/circuitbreaker/counter_test.go b/internal/circuitbreaker/counter_test.go index 2a55b288d4..eff9933470 100644 --- a/internal/circuitbreaker/counter_test.go +++ b/internal/circuitbreaker/counter_test.go @@ -17,7 +17,7 @@ import ( "reflect" "testing" - "github.com/pkg/errors" + "github.com/vdaas/vald/internal/errors" "github.com/vdaas/vald/internal/test/goleak" ) @@ -113,7 +113,6 @@ func Test_count_Successes(t *testing.T) { if err := checkFunc(test.want, gotN); err != nil { tt.Errorf("error = %v", err) } - }) } } @@ -210,7 +209,6 @@ func Test_count_Fails(t *testing.T) { if err := checkFunc(test.want, gotN); err != nil { tt.Errorf("error = %v", err) } - }) } } @@ -307,7 +305,6 @@ func Test_count_Ignores(t *testing.T) { if err := checkFunc(test.want, gotN); err != nil { tt.Errorf("error = %v", err) } - }) } } @@ -404,7 +401,6 @@ func Test_count_Total(t *testing.T) { if err := checkFunc(test.want, gotN); err != nil { tt.Errorf("error = %v", err) } - }) } } @@ -415,8 +411,7 @@ func Test_count_onSuccess(t *testing.T) { successes int64 failures int64 } - type want struct { - } + type want struct{} type test struct { name string fields fields @@ -507,8 +502,7 @@ func Test_count_onFail(t *testing.T) { successes int64 failures int64 } - type want struct { - } + type want struct{} type test struct { name string fields fields @@ -599,8 +593,7 @@ func Test_count_onIgnore(t *testing.T) { successes int64 failures int64 } - type want struct { - } + type want struct{} type test struct { name string fields fields @@ -691,8 +684,7 @@ func Test_count_reset(t *testing.T) { successes int64 failures int64 } - type want struct { - } + type want struct{} type test struct { name string fields fields