From 430649854045539fd713ddd622c0ad95cb489224 Mon Sep 17 00:00:00 2001 From: YangruiEmma Date: Mon, 18 Nov 2024 01:33:45 +0800 Subject: [PATCH] optimize(retry): optimize UpdatePolicy and add test cases to check invalid retry policy --- pkg/retry/backup_retryer.go | 20 +- pkg/retry/backup_test.go | 76 ++++++ pkg/retry/failure_retryer.go | 17 +- pkg/retry/failure_test.go | 107 +++++++++ pkg/retry/mixed_retryer.go | 19 +- pkg/retry/mixed_test.go | 93 ++++++++ pkg/retry/policy_test.go | 2 +- pkg/retry/retryer_test.go | 432 +++++++++++++++++++++++------------ 8 files changed, 590 insertions(+), 176 deletions(-) diff --git a/pkg/retry/backup_retryer.go b/pkg/retry/backup_retryer.go index 21980f57ca..84fe8a00ce 100644 --- a/pkg/retry/backup_retryer.go +++ b/pkg/retry/backup_retryer.go @@ -173,29 +173,27 @@ func (r *backupRetryer) Prepare(ctx context.Context, prevRI, retryRI rpcinfo.RPC // UpdatePolicy implements the Retryer interface. func (r *backupRetryer) UpdatePolicy(rp Policy) (err error) { + r.Lock() + defer r.Unlock() if !rp.Enable { - r.Lock() r.enable = rp.Enable - r.Unlock() return nil } if rp.BackupPolicy == nil || rp.Type != BackupType { err = errors.New("BackupPolicy is nil or retry type not match, cannot do update in backupRetryer") + r.errMsg = err.Error() + return err } - if err == nil && rp.BackupPolicy.RetryDelayMS == 0 { + if rp.BackupPolicy.RetryDelayMS == 0 { err = errors.New("invalid retry delay duration in backupRetryer") + r.errMsg = err.Error() + return err } - if err == nil { - err = checkStopPolicy(&rp.BackupPolicy.StopPolicy, maxBackupRetryTimes, r) - } - - r.Lock() - defer r.Unlock() - r.enable = rp.Enable - if err != nil { + if err = checkStopPolicy(&rp.BackupPolicy.StopPolicy, maxBackupRetryTimes, r); err != nil { r.errMsg = err.Error() return err } + r.enable = rp.Enable r.policy = rp.BackupPolicy r.retryDelay = time.Duration(rp.BackupPolicy.RetryDelayMS) * time.Millisecond return nil diff --git a/pkg/retry/backup_test.go b/pkg/retry/backup_test.go index ca09999883..76a4369d50 100644 --- a/pkg/retry/backup_test.go +++ b/pkg/retry/backup_test.go @@ -22,6 +22,7 @@ import ( "testing" "time" + "github.com/bytedance/sonic" "github.com/cloudwego/thriftgo/pkg/test" "github.com/cloudwego/kitex/pkg/rpcinfo" @@ -99,3 +100,78 @@ func TestBackupRetryWithRPCInfo(t *testing.T) { test.Assert(t, ri.To().Address().String() == "10.20.30.40:8888") test.Assert(t, atomic.LoadInt32(&callTimes) == 2) } + +func TestNewRetryer4BackupRequest(t *testing.T) { + t.Run("normal backup request", func(t *testing.T) { + jsonRet := `{"enable":true,"type":1, +"backup_policy":{"retry_delay_ms":10,"stop_policy":{"max_retry_times":2,"max_duration_ms":0,"disable_chain_stop":false,"ddl_stop":false,"cb_policy":{"error_rate":0.1,"min_sample":200}},"backoff_policy":{"backoff_type":"none"},"retry_same_node":false,"extra":"{\"not_retry_for_timeout\":false,\"rpc_retry_code\":{\"all_error_code\":false,\"error_codes\":[103,1204]},\"biz_retry_code\":{\"all_error_code\":false,\"error_codes\":[]}}","extra_struct":{"not_retry_for_timeout":false,"rpc_retry_code":{"all_error_code":false,"error_codes":[103,1204]},"biz_retry_code":{"all_error_code":false,"error_codes":[]}}}}` + var p Policy + err := sonic.UnmarshalString(jsonRet, &p) + test.Assert(t, err == nil, err) + + r, err := NewRetryer(p, nil, nil) + test.Assert(t, err == nil, err) + test.Assert(t, r.(*backupRetryer).enable) + }) + + t.Run("type is mixedRetry but policy is empty", func(t *testing.T) { + jsonRet := `{"enable":true,"type":2, "backup_policy":{}}` + var p Policy + err := sonic.UnmarshalString(jsonRet, &p) + test.Assert(t, err == nil, err) + + r, err := NewRetryer(p, nil, nil) + test.Assert(t, err != nil, err) + test.Assert(t, r == nil) + }) + + t.Run("type is backup but no policy", func(t *testing.T) { + jsonRet := `{"enable":true,"type":1}` + var p Policy + err := sonic.UnmarshalString(jsonRet, &p) + test.Assert(t, err == nil, err) + + r, err := NewRetryer(p, nil, nil) + test.Assert(t, err != nil, err) + test.Assert(t, r == nil) + }) + + t.Run("type is failure retry but policy is backupRequest", func(t *testing.T) { + jsonRet := `{"enable":true,"type":0, +"backup_policy":{"retry_delay_ms":10,"stop_policy":{"max_retry_times":2,"max_duration_ms":0,"disable_chain_stop":false,"ddl_stop":false,"cb_policy":{"error_rate":0.1,"min_sample":200}},"backoff_policy":{"backoff_type":"none"},"retry_same_node":false,"extra":"{\"not_retry_for_timeout\":false,\"rpc_retry_code\":{\"all_error_code\":false,\"error_codes\":[103,1204]},\"biz_retry_code\":{\"all_error_code\":false,\"error_codes\":[]}}","extra_struct":{"not_retry_for_timeout":false,"rpc_retry_code":{"all_error_code":false,"error_codes":[103,1204]},"biz_retry_code":{"all_error_code":false,"error_codes":[]}}}}` + var p Policy + err := sonic.UnmarshalString(jsonRet, &p) + test.Assert(t, err == nil, err) + + r, err := NewRetryer(p, nil, nil) + test.Assert(t, err != nil, err) + test.Assert(t, r == nil) + }) + + t.Run("type is backup but has multi-policies", func(t *testing.T) { + jsonRet := `{"enable":true,"type":1, +"backup_policy":{"retry_delay_ms":10,"stop_policy":{"max_retry_times":2,"max_duration_ms":0,"disable_chain_stop":false,"ddl_stop":false,"cb_policy":{"error_rate":0.1,"min_sample":200}},"backoff_policy":{"backoff_type":"none"},"retry_same_node":false,"extra":"{\"not_retry_for_timeout\":false,\"rpc_retry_code\":{\"all_error_code\":false,\"error_codes\":[103,1204]},\"biz_retry_code\":{\"all_error_code\":false,\"error_codes\":[]}}","extra_struct":{"not_retry_for_timeout":false,"rpc_retry_code":{"all_error_code":false,"error_codes":[103,1204]},"biz_retry_code":{"all_error_code":false,"error_codes":[]}}}, +"failure_policy":{"stop_policy":{"max_retry_times":2,"max_duration_ms":0,"disable_chain_stop":false,"ddl_stop":false,"cb_policy":{"error_rate":0.1,"min_sample":200}},"backoff_policy":{"backoff_type":"none"},"retry_same_node":false,"extra":"{\"not_retry_for_timeout\":false,\"rpc_retry_code\":{\"all_error_code\":false,\"error_codes\":[103,1204]},\"biz_retry_code\":{\"all_error_code\":false,\"error_codes\":[]}}","extra_struct":{"not_retry_for_timeout":false,"rpc_retry_code":{"all_error_code":false,"error_codes":[103,1204]},"biz_retry_code":{"all_error_code":false,"error_codes":[]}}} +}` + var p Policy + err := sonic.UnmarshalString(jsonRet, &p) + test.Assert(t, err == nil, err) + + r, err := NewRetryer(p, nil, nil) + test.Assert(t, err == nil, err) + test.Assert(t, r.(*backupRetryer).enable) + }) + + t.Run("type is backup but policy json has extra configuration", func(t *testing.T) { + jsonRet := `{"enable":true,"type":1, +"backup_policy":{"retry_delay_ms":10,"stop_policy":{"max_retry_times":2,"max_duration_ms":0,"disable_chain_stop":false,"ddl_stop":false,"cb_policy":{"error_rate":0.1,"min_sample":200}},"backoff_policy":{"backoff_type":"none"},"retry_same_node":false,"extra":"{\"not_retry_for_timeout\":false,\"rpc_retry_code\":{\"all_error_code\":false,\"error_codes\":[103,1204]},\"biz_retry_code\":{\"all_error_code\":false,\"error_codes\":[]}}","extra_struct":{"not_retry_for_timeout":false,"rpc_retry_code":{"all_error_code":false,"error_codes":[103,1204]},"biz_retry_code":{"all_error_code":false,"error_codes":[]}}} +}` + var p Policy + err := sonic.UnmarshalString(jsonRet, &p) + test.Assert(t, err == nil, err) + + r, err := NewRetryer(p, nil, nil) + test.Assert(t, err == nil, err) + test.Assert(t, r.(*backupRetryer).enable) + }) +} diff --git a/pkg/retry/failure_retryer.go b/pkg/retry/failure_retryer.go index 694855f0d0..99c93556ef 100644 --- a/pkg/retry/failure_retryer.go +++ b/pkg/retry/failure_retryer.go @@ -142,30 +142,27 @@ func (r *failureRetryer) Do(ctx context.Context, rpcCall RPCCallFunc, firstRI rp // UpdatePolicy implements the Retryer interface. func (r *failureRetryer) UpdatePolicy(rp Policy) (err error) { + r.Lock() + defer r.Unlock() if !rp.Enable { - r.Lock() r.enable = rp.Enable - r.Unlock() return nil } if rp.FailurePolicy == nil || rp.Type != FailureType { err = errors.New("FailurePolicy is nil or retry type not match, cannot do update in failureRetryer") + r.errMsg = err.Error() + return err } - if err == nil { - err = checkStopPolicy(&rp.FailurePolicy.StopPolicy, maxFailureRetryTimes, r) - } - r.Lock() - defer r.Unlock() - r.enable = rp.Enable - if err != nil { + if err = checkStopPolicy(&rp.FailurePolicy.StopPolicy, maxFailureRetryTimes, r); err != nil { r.errMsg = err.Error() return err } + r.enable = rp.Enable r.policy = rp.FailurePolicy r.setSpecifiedResultRetryIfNeeded(r.specifiedResultRetry, r.policy) if bo, e := initBackOff(rp.FailurePolicy.BackOffPolicy); e != nil { r.errMsg = fmt.Sprintf("failureRetryer update BackOffPolicy failed, err=%s", e.Error()) - klog.Warnf(r.errMsg) + klog.Warnf("KITEX: %s", r.errMsg) } else { r.backOff = bo } diff --git a/pkg/retry/failure_test.go b/pkg/retry/failure_test.go index 376342442e..86444e98d1 100644 --- a/pkg/retry/failure_test.go +++ b/pkg/retry/failure_test.go @@ -22,6 +22,7 @@ import ( "testing" "time" + "github.com/bytedance/sonic" "github.com/cloudwego/kitex/internal/test" "github.com/cloudwego/kitex/pkg/kerrors" "github.com/cloudwego/kitex/pkg/remote" @@ -272,6 +273,25 @@ func TestSpecifiedErrorRetry(t *testing.T) { test.Assert(t, ok) test.Assert(t, v == remoteTagValue) }) + + // case5: all error retry and trigger circuit breaker + t.Run("case5", func(t *testing.T) { + rc := NewRetryContainerWithPercentageLimit() + p := BuildFailurePolicy(NewFailurePolicyWithResultRetry(AllErrorRetry())) + ri = genRPCInfo() + + for i := 0; i < 10; i++ { + // failure rate is 50% + _, _, err := rc.WithRetryIfNeeded(ctx, &p, retryWithTransError(0, transErrCode), ri, nil) + if i < 5 { + // i < 5, total request < 10 + test.Assert(t, err == nil, err, i) + } else { + // the minSimple is 10, total request > 10 then trigger cb + test.Assert(t, err != nil) + } + } + }) } // test specified resp to retry @@ -691,3 +711,90 @@ var retryWithTransError = func(callTimes, transErrCode int32) RPCCallFunc { } } } + +func TestNewRetryer4FailureRetry(t *testing.T) { + t.Run("normal failure retry", func(t *testing.T) { + jsonRet := `{"enable":true,"type":0, +"failure_policy":{"stop_policy":{"max_retry_times":2,"max_duration_ms":0,"disable_chain_stop":false,"ddl_stop":false,"cb_policy":{"error_rate":0.1,"min_sample":200}},"backoff_policy":{"backoff_type":"none"},"retry_same_node":false,"extra":"{\"not_retry_for_timeout\":false,\"rpc_retry_code\":{\"all_error_code\":false,\"error_codes\":[103,1204]},\"biz_retry_code\":{\"all_error_code\":false,\"error_codes\":[]}}","extra_struct":{"not_retry_for_timeout":false,"rpc_retry_code":{"all_error_code":false,"error_codes":[103,1204]},"biz_retry_code":{"all_error_code":false,"error_codes":[]}}}}` + var p Policy + err := sonic.UnmarshalString(jsonRet, &p) + test.Assert(t, err == nil, err) + + r, err := NewRetryer(p, nil, nil) + test.Assert(t, err == nil, err) + test.Assert(t, r.(*failureRetryer).enable) + }) + + t.Run("type is failure retry but policy is empty", func(t *testing.T) { + jsonRet := `{"enable":true,"type":0, "failure_policy":{}}` + var p Policy + err := sonic.UnmarshalString(jsonRet, &p) + test.Assert(t, err == nil, err) + + r, err := NewRetryer(p, nil, nil) + test.Assert(t, err == nil, err) + test.Assert(t, r.(*failureRetryer).enable) + }) + + t.Run("type is failure retry but policy is mixed", func(t *testing.T) { + jsonRet := `{"enable":true,"type":0, +"mixed_policy":{"retry_delay_ms":10,"stop_policy":{"max_retry_times":2,"max_duration_ms":0,"disable_chain_stop":false,"ddl_stop":false,"cb_policy":{"error_rate":0.1,"min_sample":200}},"backoff_policy":{"backoff_type":"none"},"retry_same_node":false,"extra":"{\"not_retry_for_timeout\":false,\"rpc_retry_code\":{\"all_error_code\":false,\"error_codes\":[103,1204]},\"biz_retry_code\":{\"all_error_code\":false,\"error_codes\":[]}}","extra_struct":{"not_retry_for_timeout":false,"rpc_retry_code":{"all_error_code":false,"error_codes":[103,1204]},"biz_retry_code":{"all_error_code":false,"error_codes":[]}}}}` + var p Policy + err := sonic.UnmarshalString(jsonRet, &p) + test.Assert(t, err == nil, err) + + r, err := NewRetryer(p, nil, nil) + test.Assert(t, err != nil, err) + test.Assert(t, r == nil) + }) + + t.Run("type is failure retry but no policy", func(t *testing.T) { + jsonRet := `{"enable":true,"type":0}` + var p Policy + err := sonic.UnmarshalString(jsonRet, &p) + test.Assert(t, err == nil, err) + + r, err := NewRetryer(p, nil, nil) + test.Assert(t, err != nil, err) + test.Assert(t, r == nil) + }) + + t.Run("type is backup but policy is failure retry", func(t *testing.T) { + jsonRet := `{"enable":true,"type":1, +"failure_policy":{"stop_policy":{"max_retry_times":2,"max_duration_ms":0,"disable_chain_stop":false,"ddl_stop":false,"cb_policy":{"error_rate":0.1,"min_sample":200}},"backoff_policy":{"backoff_type":"none"},"retry_same_node":false,"extra":"{\"not_retry_for_timeout\":false,\"rpc_retry_code\":{\"all_error_code\":false,\"error_codes\":[103,1204]},\"biz_retry_code\":{\"all_error_code\":false,\"error_codes\":[]}}","extra_struct":{"not_retry_for_timeout":false,"rpc_retry_code":{"all_error_code":false,"error_codes":[103,1204]},"biz_retry_code":{"all_error_code":false,"error_codes":[]}}}}` + var p Policy + err := sonic.UnmarshalString(jsonRet, &p) + test.Assert(t, err == nil, err) + + r, err := NewRetryer(p, nil, nil) + test.Assert(t, err != nil, err) + test.Assert(t, r == nil) + }) + + t.Run("type is failure retry but has multi-policies", func(t *testing.T) { + jsonRet := `{"enable":true,"type":0, +"backup_policy":{"retry_delay_ms":10,"stop_policy":{"max_retry_times":2,"max_duration_ms":0,"disable_chain_stop":false,"ddl_stop":false,"cb_policy":{"error_rate":0.1,"min_sample":200}},"backoff_policy":{"backoff_type":"none"},"retry_same_node":false,"extra":"{\"not_retry_for_timeout\":false,\"rpc_retry_code\":{\"all_error_code\":false,\"error_codes\":[103,1204]},\"biz_retry_code\":{\"all_error_code\":false,\"error_codes\":[]}}","extra_struct":{"not_retry_for_timeout":false,"rpc_retry_code":{"all_error_code":false,"error_codes":[103,1204]},"biz_retry_code":{"all_error_code":false,"error_codes":[]}}}, +"failure_policy":{"stop_policy":{"max_retry_times":2,"max_duration_ms":0,"disable_chain_stop":false,"ddl_stop":false,"cb_policy":{"error_rate":0.1,"min_sample":200}},"backoff_policy":{"backoff_type":"none"},"retry_same_node":false,"extra":"{\"not_retry_for_timeout\":false,\"rpc_retry_code\":{\"all_error_code\":false,\"error_codes\":[103,1204]},\"biz_retry_code\":{\"all_error_code\":false,\"error_codes\":[]}}","extra_struct":{"not_retry_for_timeout":false,"rpc_retry_code":{"all_error_code":false,"error_codes":[103,1204]},"biz_retry_code":{"all_error_code":false,"error_codes":[]}}} +}` + var p Policy + err := sonic.UnmarshalString(jsonRet, &p) + test.Assert(t, err == nil, err) + + r, err := NewRetryer(p, nil, nil) + test.Assert(t, err == nil, err) + test.Assert(t, r.(*failureRetryer).enable) + }) + + t.Run("type is failure retry but policy json has retry_delay_ms", func(t *testing.T) { + jsonRet := `{"enable":true,"type":0, +"failure_policy":{"stop_policy":{"max_retry_times":2,"max_duration_ms":0,"disable_chain_stop":false,"ddl_stop":false,"cb_policy":{"error_rate":0.1,"min_sample":200}},"backoff_policy":{"backoff_type":"none"},"retry_same_node":false,"extra":"{\"not_retry_for_timeout\":false,\"rpc_retry_code\":{\"all_error_code\":false,\"error_codes\":[103,1204]},\"biz_retry_code\":{\"all_error_code\":false,\"error_codes\":[]}}","extra_struct":{"not_retry_for_timeout":false,"rpc_retry_code":{"all_error_code":false,"error_codes":[103,1204]},"biz_retry_code":{"all_error_code":false,"error_codes":[]}}} +}` + var p Policy + err := sonic.UnmarshalString(jsonRet, &p) + test.Assert(t, err == nil, err) + + r, err := NewRetryer(p, nil, nil) + test.Assert(t, err == nil, err) + test.Assert(t, r.(*failureRetryer).enable) + }) +} diff --git a/pkg/retry/mixed_retryer.go b/pkg/retry/mixed_retryer.go index 496f7061cd..8e5cb8e014 100644 --- a/pkg/retry/mixed_retryer.go +++ b/pkg/retry/mixed_retryer.go @@ -201,28 +201,27 @@ func (r *mixedRetryer) Do(ctx context.Context, rpcCall RPCCallFunc, firstRI rpci // UpdatePolicy implements the Retryer interface. func (r *mixedRetryer) UpdatePolicy(rp Policy) (err error) { + r.Lock() + defer r.Unlock() if !rp.Enable { - r.Lock() r.enable = rp.Enable - r.Unlock() return nil } if rp.MixedPolicy == nil || rp.Type != MixedType { err = errors.New("MixedPolicy is nil or retry type not match, cannot do update in mixedRetryer") + r.errMsg = err.Error() + return err } - if err == nil && rp.MixedPolicy.RetryDelayMS == 0 { + if rp.MixedPolicy.RetryDelayMS == 0 { err = errors.New("invalid retry delay duration in mixedRetryer") + r.errMsg = err.Error() + return err } - if err == nil { - err = checkStopPolicy(&rp.MixedPolicy.StopPolicy, maxMixRetryTimes, r) - } - r.Lock() - defer r.Unlock() - r.enable = rp.Enable - if err != nil { + if err = checkStopPolicy(&rp.MixedPolicy.StopPolicy, maxMixRetryTimes, r); err != nil { r.errMsg = err.Error() return err } + r.enable = rp.Enable r.policy = rp.MixedPolicy r.retryDelay = time.Duration(rp.MixedPolicy.RetryDelayMS) * time.Millisecond r.setSpecifiedResultRetryIfNeeded(r.specifiedResultRetry, &r.policy.FailurePolicy) diff --git a/pkg/retry/mixed_test.go b/pkg/retry/mixed_test.go index c8312e107f..ffb4a3cbbc 100644 --- a/pkg/retry/mixed_test.go +++ b/pkg/retry/mixed_test.go @@ -263,6 +263,25 @@ func TestMixedRetry(t *testing.T) { test.Assert(t, err == mockErr, err) test.Assert(t, !ok) }) + + // case5: all error retry and trigger circuit breaker + t.Run("case5", func(t *testing.T) { + rc := NewRetryContainerWithPercentageLimit() + p := BuildMixedPolicy(NewMixedPolicyWithResultRetry(100, AllErrorRetry())) + ri = genRPCInfo() + + for i := 0; i < 10; i++ { + // failure rate is 50% + _, _, err := rc.WithRetryIfNeeded(ctx, &p, retryWithTransError(0, transErrCode), ri, nil) + if i < 5 { + // i < 5, total request < 10 + test.Assert(t, err == nil, err, i) + } else { + // the minSimple is 10, total request > 10 then trigger cb + test.Assert(t, err != nil) + } + } + }) } // Assuming the first request returns at 300ms, the second request costs 150ms @@ -623,3 +642,77 @@ func BenchmarkMixedRetry(b *testing.B) { } }) } + +func TestNewRetryer4MixedRetry(t *testing.T) { + t.Run("normal mixed retry", func(t *testing.T) { + jsonRet := `{"enable":true,"type":2, +"mixed_policy":{"retry_delay_ms":10,"stop_policy":{"max_retry_times":2,"max_duration_ms":0,"disable_chain_stop":false,"ddl_stop":false,"cb_policy":{"error_rate":0.1,"min_sample":200}},"backoff_policy":{"backoff_type":"none"},"retry_same_node":false,"extra":"{\"not_retry_for_timeout\":false,\"rpc_retry_code\":{\"all_error_code\":false,\"error_codes\":[103,1204]},\"biz_retry_code\":{\"all_error_code\":false,\"error_codes\":[]}}","extra_struct":{"not_retry_for_timeout":false,"rpc_retry_code":{"all_error_code":false,"error_codes":[103,1204]},"biz_retry_code":{"all_error_code":false,"error_codes":[]}}}}` + var p Policy + err := sonic.UnmarshalString(jsonRet, &p) + test.Assert(t, err == nil, err) + + r, err := NewRetryer(p, nil, nil) + test.Assert(t, err == nil, err) + test.Assert(t, r.(*mixedRetryer).enable) + }) + + t.Run("type is mixedRetry but policy is empty", func(t *testing.T) { + jsonRet := `{"enable":true,"type":2, "mixed_policy":{}}` + var p Policy + err := sonic.UnmarshalString(jsonRet, &p) + test.Assert(t, err == nil, err) + + r, err := NewRetryer(p, nil, nil) + test.Assert(t, err != nil, err) + test.Assert(t, r == nil) + }) + + t.Run("type is mixedRetry but no policy", func(t *testing.T) { + jsonRet := `{"enable":true,"type":2}` + var p Policy + err := sonic.UnmarshalString(jsonRet, &p) + test.Assert(t, err == nil, err) + + r, err := NewRetryer(p, nil, nil) + test.Assert(t, err != nil, err) + test.Assert(t, r == nil) + }) + + t.Run("type is backupRequest but policy is mixedRetry", func(t *testing.T) { + jsonRet := `{"enable":true,"type":1, +"mixed_policy":{"retry_delay_ms":10,"stop_policy":{"max_retry_times":2,"max_duration_ms":0,"disable_chain_stop":false,"ddl_stop":false,"cb_policy":{"error_rate":0.1,"min_sample":200}},"backoff_policy":{"backoff_type":"none"},"retry_same_node":false,"extra":"{\"not_retry_for_timeout\":false,\"rpc_retry_code\":{\"all_error_code\":false,\"error_codes\":[103,1204]},\"biz_retry_code\":{\"all_error_code\":false,\"error_codes\":[]}}","extra_struct":{"not_retry_for_timeout":false,"rpc_retry_code":{"all_error_code":false,"error_codes":[103,1204]},"biz_retry_code":{"all_error_code":false,"error_codes":[]}}}}` + var p Policy + err := sonic.UnmarshalString(jsonRet, &p) + test.Assert(t, err == nil, err) + + r, err := NewRetryer(p, nil, nil) + test.Assert(t, err != nil, err) + test.Assert(t, r == nil) + }) + + t.Run("type is mixedRetry but policy is failure retry", func(t *testing.T) { + jsonRet := `{"enable":true,"type":2, +"failure_policy":{"stop_policy":{"max_retry_times":2,"max_duration_ms":0,"disable_chain_stop":false,"ddl_stop":false,"cb_policy":{"error_rate":0.1,"min_sample":200}},"backoff_policy":{"backoff_type":"none"},"retry_same_node":false,"extra":"{\"not_retry_for_timeout\":false,\"rpc_retry_code\":{\"all_error_code\":false,\"error_codes\":[103,1204]},\"biz_retry_code\":{\"all_error_code\":false,\"error_codes\":[]}}","extra_struct":{"not_retry_for_timeout":false,"rpc_retry_code":{"all_error_code":false,"error_codes":[103,1204]},"biz_retry_code":{"all_error_code":false,"error_codes":[]}}}}` + var p Policy + err := sonic.UnmarshalString(jsonRet, &p) + test.Assert(t, err == nil, err) + + r, err := NewRetryer(p, nil, nil) + test.Assert(t, err != nil, err) + test.Assert(t, r == nil) + }) + + t.Run("type is mixedRetry but has multi-policies", func(t *testing.T) { + jsonRet := `{"enable":true,"type":2, +"mixed_policy":{"retry_delay_ms":10,"stop_policy":{"max_retry_times":2,"max_duration_ms":0,"disable_chain_stop":false,"ddl_stop":false,"cb_policy":{"error_rate":0.1,"min_sample":200}},"backoff_policy":{"backoff_type":"none"},"retry_same_node":false,"extra":"{\"not_retry_for_timeout\":false,\"rpc_retry_code\":{\"all_error_code\":false,\"error_codes\":[103,1204]},\"biz_retry_code\":{\"all_error_code\":false,\"error_codes\":[]}}","extra_struct":{"not_retry_for_timeout":false,"rpc_retry_code":{"all_error_code":false,"error_codes":[103,1204]},"biz_retry_code":{"all_error_code":false,"error_codes":[]}}}, +"failure_policy":{"stop_policy":{"max_retry_times":2,"max_duration_ms":0,"disable_chain_stop":false,"ddl_stop":false,"cb_policy":{"error_rate":0.1,"min_sample":200}},"backoff_policy":{"backoff_type":"none"},"retry_same_node":false,"extra":"{\"not_retry_for_timeout\":false,\"rpc_retry_code\":{\"all_error_code\":false,\"error_codes\":[103,1204]},\"biz_retry_code\":{\"all_error_code\":false,\"error_codes\":[]}}","extra_struct":{"not_retry_for_timeout":false,"rpc_retry_code":{"all_error_code":false,"error_codes":[103,1204]},"biz_retry_code":{"all_error_code":false,"error_codes":[]}}} +}` + var p Policy + err := sonic.UnmarshalString(jsonRet, &p) + test.Assert(t, err == nil, err) + + r, err := NewRetryer(p, nil, nil) + test.Assert(t, err == nil, err) + test.Assert(t, r.(*mixedRetryer).enable) + }) +} diff --git a/pkg/retry/policy_test.go b/pkg/retry/policy_test.go index 9aec2e4bfc..cf0f4214ed 100644 --- a/pkg/retry/policy_test.go +++ b/pkg/retry/policy_test.go @@ -286,7 +286,7 @@ func TestRetryPolicyFailure(t *testing.T) { test.Assert(t, fr.policy.Equals(p.FailurePolicy)) } -// test policy equal +// test policy not equal func TestPolicyNotEqual(t *testing.T) { var p, policy Policy diff --git a/pkg/retry/retryer_test.go b/pkg/retry/retryer_test.go index 262cdabd53..da018d5930 100644 --- a/pkg/retry/retryer_test.go +++ b/pkg/retry/retryer_test.go @@ -19,6 +19,7 @@ package retry import ( "context" "reflect" + "strings" "sync" "sync/atomic" "testing" @@ -97,185 +98,271 @@ func TestNewRetryContainer(t *testing.T) { test.Assert(t, ok) _, allow = r.AllowRetry(context.Background()) test.Assert(t, !allow) +} - // backupPolicy is nil - rc = NewRetryContainer() - rc.NotifyPolicyChange(method, Policy{ - Enable: true, - Type: 1, +// test invalid policy +func TestRetryInvalidPolicyInit(t *testing.T) { + t.Run("type is mixedRetry but policy is not", func(t *testing.T) { + jsonRet := `{"enable":true,"type":1, +"mixed_policy":{"retry_delay_ms":10,"stop_policy":{"max_retry_times":2,"max_duration_ms":0,"disable_chain_stop":false,"ddl_stop":false,"cb_policy":{"error_rate":0.1,"min_sample":200}},"backoff_policy":{"backoff_type":"none"},"retry_same_node":false,"extra":"{\"not_retry_for_timeout\":false,\"rpc_retry_code\":{\"all_error_code\":false,\"error_codes\":[103,1204]},\"biz_retry_code\":{\"all_error_code\":false,\"error_codes\":[]}}","extra_struct":{"not_retry_for_timeout":false,"rpc_retry_code":{"all_error_code":false,"error_codes":[103,1204]},"biz_retry_code":{"all_error_code":false,"error_codes":[]}}} +}` + var p Policy + err := sonic.UnmarshalString(jsonRet, &p) + test.Assert(t, err == nil, err) + + rc := NewRetryContainerWithCB(nil, nil) + rc.NotifyPolicyChange(method, p) + test.Assert(t, strings.Contains(rc.msg, "BackupPolicy is nil or retry type not match"), rc.msg) }) - msg := "new retryer[test-Backup] failed, err=newBackupRetryer failed, err=BackupPolicy is nil or retry type not match, cannot do update in backupRetryer, at " - test.Assert(t, rc.msg[:len(msg)] == msg) - // backupPolicy config invalid - rc.NotifyPolicyChange(method, Policy{ - Enable: true, - Type: 1, - BackupPolicy: &BackupPolicy{ - RetryDelayMS: 0, - }, + t.Run("type is backupRequest but policy is not", func(t *testing.T) { + jsonRet := `{"enable":true,"type":1, +"mixed_policy":{"retry_delay_ms":10,"stop_policy":{"max_retry_times":2,"max_duration_ms":0,"disable_chain_stop":false,"ddl_stop":false,"cb_policy":{"error_rate":0.1,"min_sample":200}},"backoff_policy":{"backoff_type":"none"},"retry_same_node":false,"extra":"{\"not_retry_for_timeout\":false,\"rpc_retry_code\":{\"all_error_code\":false,\"error_codes\":[103,1204]},\"biz_retry_code\":{\"all_error_code\":false,\"error_codes\":[]}}","extra_struct":{"not_retry_for_timeout":false,"rpc_retry_code":{"all_error_code":false,"error_codes":[103,1204]},"biz_retry_code":{"all_error_code":false,"error_codes":[]}}}, +"failure_policy": {} +}` + var p Policy + err := sonic.UnmarshalString(jsonRet, &p) + test.Assert(t, err == nil, err) + + rc := NewRetryContainerWithCB(nil, nil) + rc.NotifyPolicyChange(method, p) + test.Assert(t, strings.Contains(rc.msg, "BackupPolicy is nil or retry type not match"), rc.msg) }) - msg = "new retryer[test-Backup] failed, err=newBackupRetryer failed, err=invalid retry delay duration in backupRetryer, at " - test.Assert(t, rc.msg[:len(msg)] == msg, rc.msg) - // backupPolicy cBPolicy config invalid - rc.NotifyPolicyChange(method, Policy{ - Enable: true, - Type: 1, - BackupPolicy: &BackupPolicy{ - RetryDelayMS: 100, - StopPolicy: StopPolicy{ - CBPolicy: CBPolicy{ - ErrorRate: 0.4, - }, - }, - }, + t.Run("type is mixedRetry but policy is empty", func(t *testing.T) { + jsonRet := `{"enable":true,"type":2}` + var p Policy + err := sonic.UnmarshalString(jsonRet, &p) + test.Assert(t, err == nil, err) + + rc := NewRetryContainerWithCB(nil, nil) + rc.NotifyPolicyChange(method, p) + test.Assert(t, strings.Contains(rc.msg, "MixedPolicy is nil or retry type not match"), rc.msg) }) - msg = "new retryer[test-Backup] at " - test.Assert(t, rc.msg[:len(msg)] == msg) - // failurePolicy config invalid - rc = NewRetryContainer() - rc.NotifyPolicyChange(method, Policy{ - Enable: true, - Type: 0, - FailurePolicy: &FailurePolicy{ - StopPolicy: StopPolicy{ - MaxRetryTimes: 6, + t.Run("type is failureRetry but policy is not", func(t *testing.T) { + jsonRet := `{"enable":true,"type":0, +"mixed_policy":{"retry_delay_ms":10,"stop_policy":{"max_retry_times":2,"max_duration_ms":0,"disable_chain_stop":false,"ddl_stop":false,"cb_policy":{"error_rate":0.1,"min_sample":200}},"backoff_policy":{"backoff_type":"none"},"retry_same_node":false,"extra":"{\"not_retry_for_timeout\":false,\"rpc_retry_code\":{\"all_error_code\":false,\"error_codes\":[103,1204]},\"biz_retry_code\":{\"all_error_code\":false,\"error_codes\":[]}}","extra_struct":{"not_retry_for_timeout":false,"rpc_retry_code":{"all_error_code":false,"error_codes":[103,1204]},"biz_retry_code":{"all_error_code":false,"error_codes":[]}}}}` + var p Policy + err := sonic.UnmarshalString(jsonRet, &p) + test.Assert(t, err == nil, err) + + rc := NewRetryContainerWithCB(nil, nil) + rc.NotifyPolicyChange(method, p) + test.Assert(t, strings.Contains(rc.msg, "FailurePolicy is nil or retry type not match"), rc.msg) + }) + + t.Run("failurePolicy is nil", func(t *testing.T) { + rc := NewRetryContainer() + rc.NotifyPolicyChange(method, Policy{ + Enable: true, + Type: 0, + }) + test.Assert(t, strings.Contains(rc.msg, "FailurePolicy is nil or retry type not match")) + }) + + t.Run("backupPolicy is nil", func(t *testing.T) { + cbs := newCBSuite(nil) + rc := NewRetryContainerWithCBStat(cbs.ServiceControl(), cbs.ServicePanel()) + rc.NotifyPolicyChange(method, Policy{ + Enable: true, + Type: 1, + }) + test.Assert(t, strings.Contains(rc.msg, "BackupPolicy is nil or retry type not match")) + }) + + t.Run("mixedPolicy is nil", func(t *testing.T) { + rc := NewRetryContainer() + rc.NotifyPolicyChange(method, Policy{ + Enable: true, + Type: 2, + }) + test.Assert(t, strings.Contains(rc.msg, "MixedPolicy is nil or retry type not match")) + }) + + t.Run("backupPolicy config invalid", func(t *testing.T) { + rc := NewRetryContainer() + rc.NotifyPolicyChange(method, Policy{ + Enable: true, + Type: 1, + BackupPolicy: &BackupPolicy{ + RetryDelayMS: 0, }, - }, + }) + msg := "new retryer[test-Backup] failed, err=newBackupRetryer failed, err=invalid retry delay duration in backupRetryer, at " + test.Assert(t, rc.msg[:len(msg)] == msg, rc.msg) }) - msg = "new retryer[test-Failure] failed, err=newfailureRetryer failed, err=invalid MaxRetryTimes[6], at " - test.Assert(t, rc.msg[:len(msg)] == msg, rc.msg) - // failurePolicy cBPolicy config invalid - rc = NewRetryContainer() - rc.NotifyPolicyChange(method, Policy{ - Enable: true, - Type: 0, - FailurePolicy: &FailurePolicy{ - StopPolicy: StopPolicy{ - MaxRetryTimes: 5, - CBPolicy: CBPolicy{ - ErrorRate: 0.4, + t.Run("backupPolicy config invalid", func(t *testing.T) { + rc := NewRetryContainerWithCB(nil, nil) + rc.NotifyPolicyChange(method, Policy{ + Enable: true, + Type: 1, + BackupPolicy: &BackupPolicy{ + RetryDelayMS: 100, + StopPolicy: StopPolicy{ + CBPolicy: CBPolicy{ + ErrorRate: 0.4, + }, }, }, - }, + }) + msg := "new retryer[test-Backup] at " + test.Assert(t, rc.msg[:len(msg)] == msg) }) - msg = "new retryer[test-Failure] at " - test.Assert(t, rc.msg[:len(msg)] == msg) - // failurePolicy backOffPolicy fixedBackOffType cfg is nil - rc = NewRetryContainerWithCB(nil, nil) - rc.NotifyPolicyChange(method, Policy{ - Enable: true, - Type: 0, - FailurePolicy: &FailurePolicy{ - StopPolicy: StopPolicy{ - MaxRetryTimes: 5, - }, - BackOffPolicy: &BackOffPolicy{ - BackOffType: FixedBackOffType, + t.Run("failurePolicy config invalid", func(t *testing.T) { + cbs := newCBSuite(nil) + rc := NewRetryContainerWithCBStat(cbs.ServiceControl(), cbs.ServicePanel()) + rc.NotifyPolicyChange(method, Policy{ + Enable: true, + Type: 0, + FailurePolicy: &FailurePolicy{ + StopPolicy: StopPolicy{ + MaxRetryTimes: 6, + }, }, - }, + }) + msg := "new retryer[test-Failure] failed, err=newfailureRetryer failed, err=invalid MaxRetryTimes[6], at " + test.Assert(t, rc.msg[:len(msg)] == msg, rc.msg) }) - msg = "new retryer[test-Failure] at " - test.Assert(t, rc.msg[:len(msg)] == msg) - // failurePolicy backOffPolicy fixedBackOffType cfg invalid - rc = NewRetryContainerWithCB(nil, nil) - rc.NotifyPolicyChange(method, Policy{ - Enable: true, - Type: 0, - FailurePolicy: &FailurePolicy{ - StopPolicy: StopPolicy{ - MaxRetryTimes: 5, - }, - BackOffPolicy: &BackOffPolicy{ - BackOffType: FixedBackOffType, - CfgItems: map[BackOffCfgKey]float64{ - FixMSBackOffCfgKey: 0, + t.Run("failurePolicy cBPolicy config invalid", func(t *testing.T) { + rc := NewRetryContainer() + rc.NotifyPolicyChange(method, Policy{ + Enable: true, + Type: 0, + FailurePolicy: &FailurePolicy{ + StopPolicy: StopPolicy{ + MaxRetryTimes: 5, + CBPolicy: CBPolicy{ + ErrorRate: 0.4, + }, }, }, - }, + }) + msg := "new retryer[test-Failure] at " + test.Assert(t, rc.msg[:len(msg)] == msg) }) - msg = "new retryer[test-Failure] at " - test.Assert(t, rc.msg[:len(msg)] == msg) - // failurePolicy backOffPolicy randomBackOffType cfg is nil - rc = NewRetryContainerWithCB(nil, nil) - rc.NotifyPolicyChange(method, Policy{ - Enable: true, - Type: 0, - FailurePolicy: &FailurePolicy{ - StopPolicy: StopPolicy{ - MaxRetryTimes: 5, - }, - BackOffPolicy: &BackOffPolicy{ - BackOffType: RandomBackOffType, + t.Run("failurePolicy backOffPolicy fixedBackOffType cfg is nil", func(t *testing.T) { + rc := NewRetryContainerWithCB(nil, nil) + rc.NotifyPolicyChange(method, Policy{ + Enable: true, + Type: 0, + FailurePolicy: &FailurePolicy{ + StopPolicy: StopPolicy{ + MaxRetryTimes: 5, + }, + BackOffPolicy: &BackOffPolicy{ + BackOffType: FixedBackOffType, + }, }, - }, + }) + msg := "new retryer[test-Failure] at " + test.Assert(t, rc.msg[:len(msg)] == msg) }) - msg = "new retryer[test-Failure] at " - test.Assert(t, rc.msg[:len(msg)] == msg) - // failurePolicy backOffPolicy randomBackOffType cfg invalid - rc = NewRetryContainerWithCB(nil, nil) - rc.NotifyPolicyChange(method, Policy{ - Enable: true, - Type: 0, - FailurePolicy: &FailurePolicy{ - StopPolicy: StopPolicy{ - MaxRetryTimes: 5, + t.Run("failurePolicy backOffPolicy fixedBackOffType cfg invalid", func(t *testing.T) { + rc := NewRetryContainerWithCB(nil, nil) + rc.NotifyPolicyChange(method, Policy{ + Enable: true, + Type: 0, + FailurePolicy: &FailurePolicy{ + StopPolicy: StopPolicy{ + MaxRetryTimes: 5, + }, + BackOffPolicy: &BackOffPolicy{ + BackOffType: FixedBackOffType, + CfgItems: map[BackOffCfgKey]float64{ + FixMSBackOffCfgKey: 0, + }, + }, }, - BackOffPolicy: &BackOffPolicy{ - BackOffType: RandomBackOffType, - CfgItems: map[BackOffCfgKey]float64{ - MinMSBackOffCfgKey: 20, - MaxMSBackOffCfgKey: 10, + }) + msg := "new retryer[test-Failure] at " + test.Assert(t, rc.msg[:len(msg)] == msg) + }) + + t.Run("failurePolicy backOffPolicy randomBackOffType cfg is nil", func(t *testing.T) { + rc := NewRetryContainerWithCB(nil, nil) + rc.NotifyPolicyChange(method, Policy{ + Enable: true, + Type: 0, + FailurePolicy: &FailurePolicy{ + StopPolicy: StopPolicy{ + MaxRetryTimes: 5, + }, + BackOffPolicy: &BackOffPolicy{ + BackOffType: RandomBackOffType, }, }, - }, + }) + msg := "new retryer[test-Failure] at " + test.Assert(t, rc.msg[:len(msg)] == msg) }) - msg = "new retryer[test-Failure] at " - test.Assert(t, rc.msg[:len(msg)] == msg) - // failurePolicy backOffPolicy randomBackOffType normal - rc = NewRetryContainerWithCB(nil, nil) - rc.NotifyPolicyChange(method, Policy{ - Enable: true, - Type: 0, - FailurePolicy: &FailurePolicy{ - StopPolicy: StopPolicy{ - MaxRetryTimes: 5, + t.Run("failurePolicy backOffPolicy randomBackOffType cfg invalid", func(t *testing.T) { + rc := NewRetryContainerWithCB(nil, nil) + rc.NotifyPolicyChange(method, Policy{ + Enable: true, + Type: 0, + FailurePolicy: &FailurePolicy{ + StopPolicy: StopPolicy{ + MaxRetryTimes: 5, + }, + BackOffPolicy: &BackOffPolicy{ + BackOffType: RandomBackOffType, + CfgItems: map[BackOffCfgKey]float64{ + MinMSBackOffCfgKey: 20, + MaxMSBackOffCfgKey: 10, + }, + }, }, - BackOffPolicy: &BackOffPolicy{ - BackOffType: RandomBackOffType, - CfgItems: map[BackOffCfgKey]float64{ - MinMSBackOffCfgKey: 10, - MaxMSBackOffCfgKey: 20, + }) + msg := "new retryer[test-Failure] at " + test.Assert(t, rc.msg[:len(msg)] == msg) + }) + + t.Run("failurePolicy backOffPolicy randomBackOffType normal", func(t *testing.T) { + rc := NewRetryContainerWithCB(nil, nil) + rc.NotifyPolicyChange(method, Policy{ + Enable: true, + Type: 0, + FailurePolicy: &FailurePolicy{ + StopPolicy: StopPolicy{ + MaxRetryTimes: 5, + }, + BackOffPolicy: &BackOffPolicy{ + BackOffType: RandomBackOffType, + CfgItems: map[BackOffCfgKey]float64{ + MinMSBackOffCfgKey: 10, + MaxMSBackOffCfgKey: 20, + }, }, }, - }, + }) + msg := "new retryer[test-Failure] at " + test.Assert(t, rc.msg[:len(msg)] == msg) }) - msg = "new retryer[test-Failure] at " - test.Assert(t, rc.msg[:len(msg)] == msg) - // test init invalid case - err := rc.Init(nil, nil) - test.Assert(t, err == nil, err) - err = rc.Init(map[string]Policy{Wildcard: { - Enable: true, - Type: 1, - BackupPolicy: &BackupPolicy{ - RetryDelayMS: 0, - }, - }}, nil) - test.Assert(t, err != nil, err) + t.Run("test init invalid case", func(t *testing.T) { + rc := NewRetryContainerWithCB(nil, nil) + err := rc.Init(nil, nil) + test.Assert(t, err == nil, err) + err = rc.Init(map[string]Policy{Wildcard: { + Enable: true, + Type: 1, + BackupPolicy: &BackupPolicy{ + RetryDelayMS: 0, + }, + }}, nil) + test.Assert(t, err != nil, err) - rc.DeletePolicy(method) - r = rc.getRetryer(context.Background(), genRPCInfo()) - test.Assert(t, r == nil) + rc.DeletePolicy(method) + r := rc.getRetryer(context.Background(), genRPCInfo()) + test.Assert(t, r == nil) + }) } // test container dump @@ -731,3 +818,60 @@ func BenchmarkPrepareRetryContext(b *testing.B) { } gctx = ctx } + +// v0.11.0 add type 2 - mixed retry, but the old version has bug that parse the data trigger panic +// json is +// {"enable":true,"type":2, +// "mixed_policy":{"retry_delay_ms":10,"stop_policy":{"max_retry_times":2,"max_duration_ms":0,"disable_chain_stop":false,"ddl_stop":false,"cb_policy":{"error_rate":0.1,"min_sample":200}},"backoff_policy":{"backoff_type":"none"},"retry_same_node":false,"extra":"{\"not_retry_for_timeout\":false,\"rpc_retry_code\":{\"all_error_code\":false,\"error_codes\":[103,1204]},\"biz_retry_code\":{\"all_error_code\":false,\"error_codes\":[]}}","extra_struct":{"not_retry_for_timeout":false,"rpc_retry_code":{"all_error_code":false,"error_codes":[103,1204]},"biz_retry_code":{"all_error_code":false,"error_codes":[]}}} +// } +// actually, the test cases has cover the bug +// this test case just to record the problem, and ignore it in the future +func TestBugfix4InvalidRetry(t *testing.T) { + // v0.11.0: newRetryer successful, mixed retry + t.Run("type=2, mixed retry", func(t *testing.T) { + jsonRet := `{"enable":true,"type":2, +"mixed_policy":{"retry_delay_ms":10,"stop_policy":{"max_retry_times":2,"max_duration_ms":0,"disable_chain_stop":false,"ddl_stop":false,"cb_policy":{"error_rate":0.1,"min_sample":200}},"backoff_policy":{"backoff_type":"none"},"retry_same_node":false,"extra":"{\"not_retry_for_timeout\":false,\"rpc_retry_code\":{\"all_error_code\":false,\"error_codes\":[103,1204]},\"biz_retry_code\":{\"all_error_code\":false,\"error_codes\":[]}}","extra_struct":{"not_retry_for_timeout":false,"rpc_retry_code":{"all_error_code":false,"error_codes":[103,1204]},"biz_retry_code":{"all_error_code":false,"error_codes":[]}}} +}` + var p Policy + err := sonic.UnmarshalString(jsonRet, &p) + test.Assert(t, err == nil, err) + + r, err := NewRetryer(p, nil, nil) + test.Assert(t, err == nil, err) + test.Assert(t, r.(*mixedRetryer).enable) + }) + + // expect + // v0.11.0: newRetryer successful, mixed retry + t.Run("type=2, mixed retry, but policy has empty failure_policy", func(t *testing.T) { + jsonRet := `{"enable":true,"type":2, +"mixed_policy":{"retry_delay_ms":10,"stop_policy":{"max_retry_times":2,"max_duration_ms":0,"disable_chain_stop":false,"ddl_stop":false,"cb_policy":{"error_rate":0.1,"min_sample":200}},"backoff_policy":{"backoff_type":"none"},"retry_same_node":false,"extra":"{\"not_retry_for_timeout\":false,\"rpc_retry_code\":{\"all_error_code\":false,\"error_codes\":[103,1204]},\"biz_retry_code\":{\"all_error_code\":false,\"error_codes\":[]}}","extra_struct":{"not_retry_for_timeout":false,"rpc_retry_code":{"all_error_code":false,"error_codes":[103,1204]},"biz_retry_code":{"all_error_code":false,"error_codes":[]}}}, +"failure_policy": {} +}` + var p Policy + err := sonic.UnmarshalString(jsonRet, &p) + test.Assert(t, err == nil, err) + + r, err := NewRetryer(p, nil, nil) + test.Assert(t, err == nil, err) + test.Assert(t, r.(*mixedRetryer).enable) + }) + + // expect + // v0.11.0: newRetryer successful, mixed retry + t.Run("type=3, unknown retry with unknown policy", func(t *testing.T) { + jsonRet := `{"enable":true,"type":3, +"new_policy":{"retry_delay_ms":10,"stop_policy":{"max_retry_times":2,"max_duration_ms":0,"disable_chain_stop":false,"ddl_stop":false,"cb_policy":{"error_rate":0.1,"min_sample":200}},"backoff_policy":{"backoff_type":"none"},"retry_same_node":false,"extra":"{\"not_retry_for_timeout\":false,\"rpc_retry_code\":{\"all_error_code\":false,\"error_codes\":[103,1204]},\"biz_retry_code\":{\"all_error_code\":false,\"error_codes\":[]}}","extra_struct":{"not_retry_for_timeout":false,"rpc_retry_code":{"all_error_code":false,"error_codes":[103,1204]},"biz_retry_code":{"all_error_code":false,"error_codes":[]}}}}` + var p Policy + err := sonic.UnmarshalString(jsonRet, &p) + test.Assert(t, err == nil, err) + + r, err := NewRetryer(p, nil, nil) + test.Assert(t, err != nil, err) + test.Assert(t, r == nil) + test.Assert(t, strings.Contains(err.Error(), "FailurePolicy is nil or retry type not match")) + }) +}