From d7e4062a0e86790b90f46c6d9a65fb654a73d121 Mon Sep 17 00:00:00 2001 From: Scott Finlay Date: Thu, 3 Nov 2016 10:45:31 +0100 Subject: [PATCH 1/6] Adding the ability to specify a function to check if micro should retry a failed rpc call --- client/client.go | 2 ++ client/is_retriable.go | 22 ++++++++++++++++++++++ client/options.go | 19 +++++++++++++++---- client/rpc_client.go | 5 +++++ 4 files changed, 44 insertions(+), 4 deletions(-) create mode 100644 client/is_retriable.go diff --git a/client/client.go b/client/client.go index 723af2e0..5e8fe23d 100644 --- a/client/client.go +++ b/client/client.go @@ -68,6 +68,8 @@ var ( DefaultClient Client = newRpcClient() // DefaultBackoff is the default backoff function for retries DefaultBackoff = exponentialBackoff + // DefaultCheckIfRetriable is the default check-for-retry function for retries + DefaultCheckIfRetriable = AlwaysRetry // DefaultRetries is the default number of times a request is tried DefaultRetries = 1 // DefaultRequestTimeout is the default request timeout diff --git a/client/is_retriable.go b/client/is_retriable.go new file mode 100644 index 00000000..01fc8c47 --- /dev/null +++ b/client/is_retriable.go @@ -0,0 +1,22 @@ +package client + +import ( + "github.com/micro/go-micro/errors" +) + +type IsRetriableFunc func(err error) bool + +// always retry on error +func AlwaysRetry(err error) bool { + return true +} + +func Only500Errors(err error) bool { + errorData := errors.Parse(err.Error()) + + if(errorData.Code >= 500) { + return true + } + + return false +} \ No newline at end of file diff --git a/client/options.go b/client/options.go index 7751d879..b5104990 100644 --- a/client/options.go +++ b/client/options.go @@ -43,6 +43,8 @@ type CallOptions struct { // Backoff func Backoff BackoffFunc + // Check if retriable func + CheckIfRetriable IsRetriableFunc // Transport Dial Timeout DialTimeout time.Duration // Number of Call attempts @@ -73,10 +75,11 @@ func newOptions(options ...Option) Options { opts := Options{ Codecs: make(map[string]codec.NewCodec), CallOptions: CallOptions{ - Backoff: DefaultBackoff, - Retries: DefaultRetries, - RequestTimeout: DefaultRequestTimeout, - DialTimeout: transport.DefaultDialTimeout, + Backoff: DefaultBackoff, + CheckIfRetriable: DefaultCheckIfRetriable, + Retries: DefaultRetries, + RequestTimeout: DefaultRequestTimeout, + DialTimeout: transport.DefaultDialTimeout, }, PoolSize: DefaultPoolSize, PoolTTL: DefaultPoolTTL, @@ -221,6 +224,14 @@ func WithBackoff(fn BackoffFunc) CallOption { } } +// WithCheckIfRetriable is a CallOption which overrides that which +// set in Options.CallOptions +func WithCheckIfRetriable(fn IsRetriableFunc) CallOption { + return func(o *CallOptions) { + o.CheckIfRetriable = fn + } +} + // WithRetries is a CallOption which overrides that which // set in Options.CallOptions func WithRetries(i int) CallOption { diff --git a/client/rpc_client.go b/client/rpc_client.go index 6e78db4a..b621eae2 100644 --- a/client/rpc_client.go +++ b/client/rpc_client.go @@ -299,6 +299,11 @@ func (r *rpcClient) Call(ctx context.Context, request Request, response interfac if err == nil { return nil } + + if !callOpts.CheckIfRetriable(err) { + return err + } + gerr = err } } From 092d17a74e741b6b780364430ffbfff32387ff26 Mon Sep 17 00:00:00 2001 From: Scott Finlay Date: Mon, 7 Nov 2016 09:40:11 +0100 Subject: [PATCH 2/6] Adjusting names --- client/client.go | 4 ++-- client/is_retriable.go | 22 ---------------------- client/options.go | 10 +++++----- client/retry.go | 8 ++++++++ client/rpc_client.go | 2 +- 5 files changed, 16 insertions(+), 30 deletions(-) delete mode 100644 client/is_retriable.go create mode 100644 client/retry.go diff --git a/client/client.go b/client/client.go index 5e8fe23d..e7a8536d 100644 --- a/client/client.go +++ b/client/client.go @@ -68,8 +68,8 @@ var ( DefaultClient Client = newRpcClient() // DefaultBackoff is the default backoff function for retries DefaultBackoff = exponentialBackoff - // DefaultCheckIfRetriable is the default check-for-retry function for retries - DefaultCheckIfRetriable = AlwaysRetry + // DefaultRetry is the default check-for-retry function for retries + DefaultRetry = alwaysRetry // DefaultRetries is the default number of times a request is tried DefaultRetries = 1 // DefaultRequestTimeout is the default request timeout diff --git a/client/is_retriable.go b/client/is_retriable.go deleted file mode 100644 index 01fc8c47..00000000 --- a/client/is_retriable.go +++ /dev/null @@ -1,22 +0,0 @@ -package client - -import ( - "github.com/micro/go-micro/errors" -) - -type IsRetriableFunc func(err error) bool - -// always retry on error -func AlwaysRetry(err error) bool { - return true -} - -func Only500Errors(err error) bool { - errorData := errors.Parse(err.Error()) - - if(errorData.Code >= 500) { - return true - } - - return false -} \ No newline at end of file diff --git a/client/options.go b/client/options.go index b5104990..da89c237 100644 --- a/client/options.go +++ b/client/options.go @@ -44,7 +44,7 @@ type CallOptions struct { // Backoff func Backoff BackoffFunc // Check if retriable func - CheckIfRetriable IsRetriableFunc + Retry RetryFunc // Transport Dial Timeout DialTimeout time.Duration // Number of Call attempts @@ -76,7 +76,7 @@ func newOptions(options ...Option) Options { Codecs: make(map[string]codec.NewCodec), CallOptions: CallOptions{ Backoff: DefaultBackoff, - CheckIfRetriable: DefaultCheckIfRetriable, + Retry: DefaultRetry, Retries: DefaultRetries, RequestTimeout: DefaultRequestTimeout, DialTimeout: transport.DefaultDialTimeout, @@ -224,11 +224,11 @@ func WithBackoff(fn BackoffFunc) CallOption { } } -// WithCheckIfRetriable is a CallOption which overrides that which +// WithRetry is a CallOption which overrides that which // set in Options.CallOptions -func WithCheckIfRetriable(fn IsRetriableFunc) CallOption { +func WithRetry(fn RetryFunc) CallOption { return func(o *CallOptions) { - o.CheckIfRetriable = fn + o.Retry = fn } } diff --git a/client/retry.go b/client/retry.go new file mode 100644 index 00000000..df74abec --- /dev/null +++ b/client/retry.go @@ -0,0 +1,8 @@ +package client + +type RetryFunc func(err error) bool + +// always retry on error +func alwaysRetry(err error) bool { + return true +} diff --git a/client/rpc_client.go b/client/rpc_client.go index b621eae2..0ad84805 100644 --- a/client/rpc_client.go +++ b/client/rpc_client.go @@ -300,7 +300,7 @@ func (r *rpcClient) Call(ctx context.Context, request Request, response interfac return nil } - if !callOpts.CheckIfRetriable(err) { + if !callOpts.Retry(err) { return err } From a66bce0e4b8ca242a64882f6e6e12f55b8aa0443 Mon Sep 17 00:00:00 2001 From: Scott Finlay Date: Mon, 7 Nov 2016 17:10:40 +0100 Subject: [PATCH 3/6] Adjusting arguments and return value of retry function and adding new retry logic to stream --- client/retry.go | 8 +++++--- client/rpc_client.go | 9 +++++++-- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/client/retry.go b/client/retry.go index df74abec..59743980 100644 --- a/client/retry.go +++ b/client/retry.go @@ -1,8 +1,10 @@ package client -type RetryFunc func(err error) bool +import "context" + +type RetryFunc func(ctx context.Context, req Request, retryCount int, err error) (bool, error) // always retry on error -func alwaysRetry(err error) bool { - return true +func alwaysRetry(ctx context.Context, req Request, retryCount int, err error) (bool, error) { + return true, err } diff --git a/client/rpc_client.go b/client/rpc_client.go index 0ad84805..ede9e899 100644 --- a/client/rpc_client.go +++ b/client/rpc_client.go @@ -300,7 +300,7 @@ func (r *rpcClient) Call(ctx context.Context, request Request, response interfac return nil } - if !callOpts.Retry(err) { + if retry, err := callOpts.Retry(ctx, request, i, err); !retry { return err } @@ -405,7 +405,12 @@ func (r *rpcClient) Stream(ctx context.Context, request Request, opts ...CallOpt if rsp.err == nil { return rsp.stream, nil } - grr = rsp.err + + if retry, err := callOpts.Retry(ctx, request, i, rsp.err); !retry { + return nil, err + } + + grr = err } } From c6737ac64c6932c6c24fdaf9e252af238ea17d06 Mon Sep 17 00:00:00 2001 From: Scott Finlay Date: Mon, 7 Nov 2016 17:15:11 +0100 Subject: [PATCH 4/6] Adjusting import because of failed build --- client/retry.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/retry.go b/client/retry.go index 59743980..cdae7a6b 100644 --- a/client/retry.go +++ b/client/retry.go @@ -1,6 +1,6 @@ package client -import "context" +import "golang.org/x/net/context" type RetryFunc func(ctx context.Context, req Request, retryCount int, err error) (bool, error) From a6812ba3321e4041f869a88a84f6fc5ed036bf79 Mon Sep 17 00:00:00 2001 From: Scott Finlay Date: Mon, 7 Nov 2016 17:39:05 +0100 Subject: [PATCH 5/6] Adjusting the logic around the returned error from the retry function --- client/retry.go | 3 ++- client/rpc_client.go | 17 ++++++++++++++--- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/client/retry.go b/client/retry.go index cdae7a6b..06e2d40c 100644 --- a/client/retry.go +++ b/client/retry.go @@ -2,9 +2,10 @@ package client import "golang.org/x/net/context" +// note that returning either true or a non-nil error will result in the call not being retried type RetryFunc func(ctx context.Context, req Request, retryCount int, err error) (bool, error) // always retry on error func alwaysRetry(ctx context.Context, req Request, retryCount int, err error) (bool, error) { - return true, err + return true, nil } diff --git a/client/rpc_client.go b/client/rpc_client.go index ede9e899..1d6b743f 100644 --- a/client/rpc_client.go +++ b/client/rpc_client.go @@ -300,10 +300,16 @@ func (r *rpcClient) Call(ctx context.Context, request Request, response interfac return nil } - if retry, err := callOpts.Retry(ctx, request, i, err); !retry { + retry, rerr := callOpts.Retry(ctx, request, i, err) + if rerr != nil { + return rerr + } + + if !retry { return err } + gerr = err } } @@ -406,8 +412,13 @@ func (r *rpcClient) Stream(ctx context.Context, request Request, opts ...CallOpt return rsp.stream, nil } - if retry, err := callOpts.Retry(ctx, request, i, rsp.err); !retry { - return nil, err + retry, rerr := callOpts.Retry(ctx, request, i, rsp.err) + if rerr != nil { + return nil, rerr + } + + if !retry { + return nil, rsp.err } grr = err From 1ab59094ebb3030a846c76fcb3c51b7b3fb61a0b Mon Sep 17 00:00:00 2001 From: Scott Finlay Date: Mon, 7 Nov 2016 17:46:12 +0100 Subject: [PATCH 6/6] Fixing return value and gofmt --- client/options.go | 10 +++++----- client/rpc_client.go | 3 +-- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/client/options.go b/client/options.go index da89c237..0f53c8db 100644 --- a/client/options.go +++ b/client/options.go @@ -75,11 +75,11 @@ func newOptions(options ...Option) Options { opts := Options{ Codecs: make(map[string]codec.NewCodec), CallOptions: CallOptions{ - Backoff: DefaultBackoff, - Retry: DefaultRetry, - Retries: DefaultRetries, - RequestTimeout: DefaultRequestTimeout, - DialTimeout: transport.DefaultDialTimeout, + Backoff: DefaultBackoff, + Retry: DefaultRetry, + Retries: DefaultRetries, + RequestTimeout: DefaultRequestTimeout, + DialTimeout: transport.DefaultDialTimeout, }, PoolSize: DefaultPoolSize, PoolTTL: DefaultPoolTTL, diff --git a/client/rpc_client.go b/client/rpc_client.go index 1d6b743f..861ddc53 100644 --- a/client/rpc_client.go +++ b/client/rpc_client.go @@ -309,7 +309,6 @@ func (r *rpcClient) Call(ctx context.Context, request Request, response interfac return err } - gerr = err } } @@ -421,7 +420,7 @@ func (r *rpcClient) Stream(ctx context.Context, request Request, opts ...CallOpt return nil, rsp.err } - grr = err + grr = rsp.err } }