From a13f676ec6f18cdbf8d5ace156726cf2ecd87cf3 Mon Sep 17 00:00:00 2001 From: Allen Ray Date: Wed, 17 Jan 2024 11:45:36 -0500 Subject: [PATCH] [3.5] backport: add backoff to client config Signed-off-by: Allen Ray --- client/v3/client.go | 19 +++++++++++++-- client/v3/client_test.go | 51 ++++++++++++++++++++++++++++++++++++++++ client/v3/config.go | 9 +++++++ 3 files changed, 77 insertions(+), 2 deletions(-) diff --git a/client/v3/client.go b/client/v3/client.go index efa44e890..8a2225b22 100644 --- a/client/v3/client.go +++ b/client/v3/client.go @@ -231,15 +231,30 @@ func (c *Client) dialSetupOpts(creds grpccredentials.TransportCredentials, dopts opts = append(opts, grpc.WithInsecure()) } + unaryMaxRetries := defaultUnaryMaxRetries + if c.cfg.MaxUnaryRetries > 0 { + unaryMaxRetries = c.cfg.MaxUnaryRetries + } + + backoffWaitBetween := defaultBackoffWaitBetween + if c.cfg.BackoffWaitBetween > 0 { + backoffWaitBetween = c.cfg.BackoffWaitBetween + } + + backoffJitterFraction := defaultBackoffJitterFraction + if c.cfg.BackoffJitterFraction > 0 { + backoffJitterFraction = c.cfg.BackoffJitterFraction + } + // Interceptor retry and backoff. // TODO: Replace all of clientv3/retry.go with RetryPolicy: // https://github.com/grpc/grpc-proto/blob/cdd9ed5c3d3f87aef62f373b93361cf7bddc620d/grpc/service_config/service_config.proto#L130 - rrBackoff := withBackoff(c.roundRobinQuorumBackoff(defaultBackoffWaitBetween, defaultBackoffJitterFraction)) + rrBackoff := withBackoff(c.roundRobinQuorumBackoff(backoffWaitBetween, backoffJitterFraction)) opts = append(opts, // Disable stream retry by default since go-grpc-middleware/retry does not support client streams. // Streams that are safe to retry are enabled individually. grpc.WithStreamInterceptor(c.streamClientInterceptor(withMax(0), rrBackoff)), - grpc.WithUnaryInterceptor(c.unaryClientInterceptor(withMax(defaultUnaryMaxRetries), rrBackoff)), + grpc.WithUnaryInterceptor(c.unaryClientInterceptor(withMax(unaryMaxRetries), rrBackoff)), ) return opts, nil diff --git a/client/v3/client_test.go b/client/v3/client_test.go index c6e7c7a97..b9d12c058 100644 --- a/client/v3/client_test.go +++ b/client/v3/client_test.go @@ -149,6 +149,57 @@ func TestDialNoTimeout(t *testing.T) { c.Close() } +func TestMaxUnaryRetries(t *testing.T) { + maxUnaryRetries := uint(10) + cfg := Config{ + Endpoints: []string{"127.0.0.1:12345"}, + MaxUnaryRetries: maxUnaryRetries, + } + c, err := NewClient(t, cfg) + if c == nil || err != nil { + t.Fatalf("new client with MaxUnaryRetries should succeed, got %v", err) + } + defer c.Close() + + if c.cfg.MaxUnaryRetries != maxUnaryRetries { + t.Fatalf("client MaxUnaryRetries should be %d, got %d", maxUnaryRetries, c.cfg.MaxUnaryRetries) + } +} + +func TestBackoff(t *testing.T) { + backoffWaitBetween := 100 * time.Millisecond + cfg := Config{ + Endpoints: []string{"127.0.0.1:12345"}, + BackoffWaitBetween: backoffWaitBetween, + } + c, err := NewClient(t, cfg) + if c == nil || err != nil { + t.Fatalf("new client with BackoffWaitBetween should succeed, got %v", err) + } + defer c.Close() + + if c.cfg.BackoffWaitBetween != backoffWaitBetween { + t.Fatalf("client BackoffWaitBetween should be %v, got %v", backoffWaitBetween, c.cfg.BackoffWaitBetween) + } +} + +func TestBackoffJitterFraction(t *testing.T) { + backoffJitterFraction := float64(0.9) + cfg := Config{ + Endpoints: []string{"127.0.0.1:12345"}, + BackoffJitterFraction: backoffJitterFraction, + } + c, err := NewClient(t, cfg) + if c == nil || err != nil { + t.Fatalf("new client with BackoffJitterFraction should succeed, got %v", err) + } + defer c.Close() + + if c.cfg.BackoffJitterFraction != backoffJitterFraction { + t.Fatalf("client BackoffJitterFraction should be %v, got %v", backoffJitterFraction, c.cfg.BackoffJitterFraction) + } +} + func TestIsHaltErr(t *testing.T) { if !isHaltErr(context.TODO(), fmt.Errorf("etcdserver: some etcdserver error")) { t.Errorf(`error prefixed with "etcdserver: " should be Halted by default`) diff --git a/client/v3/config.go b/client/v3/config.go index 335a28873..6f5b41978 100644 --- a/client/v3/config.go +++ b/client/v3/config.go @@ -88,5 +88,14 @@ type Config struct { // PermitWithoutStream when set will allow client to send keepalive pings to server without any active streams(RPCs). PermitWithoutStream bool `json:"permit-without-stream"` + // MaxUnaryRetries is the maximum number of retries for unary RPCs. + MaxUnaryRetries uint `json:"max-unary-retries"` + + // BackoffWaitBetween is the wait time before retrying an RPC. + BackoffWaitBetween time.Duration `json:"backoff-wait-between"` + + // BackoffJitterFraction is the jitter fraction to randomize backoff wait time. + BackoffJitterFraction float64 `json:"backoff-jitter-fraction"` + // TODO: support custom balancer picker }