[#XX] Support retryer

Add two strategy for PutBucketSettings request retryer: * exponential backoff (increasing up to `max_backoff` delays with jitter) * constant backoff (always the same `max_backoff` delay between requests) Signed-off-by: Denis Kirillov <d.kirillov@yadro.com>
2024-05-30 16:02:27 +03:00 · 2024-05-30 16:02:27 +03:00 · fe3452e170
parent 9f3ea470e6
commit fe3452e170
11 changed files with 235 additions and 6 deletions
--- a/api/handler/api.go
+++ b/api/handler/api.go
@ -47,6 +47,9 @@ type (
 		BypassContentEncodingInChunks() bool
 		MD5Enabled() bool
 		ACLEnabled() bool
+		RetryMaxAttempts() int
+		RetryMaxBackoff() time.Duration
+		RetryStrategy() RetryStrategy
 	}

 	FrostFSID interface {
@ -63,6 +66,13 @@ type (
 	}
 )

+type RetryStrategy string
+
+const (
+	RetryStrategyExponential = "exponential"
+	RetryStrategyConstant    = "constant"
+)
+
 var _ api.Handler = (*handler)(nil)

 // New creates new api.Handler using given logger and client.
--- a/api/handler/handlers_test.go
+++ b/api/handler/handlers_test.go
@ -132,6 +132,18 @@ func (c *configMock) ResolveNamespaceAlias(ns string) string {
 	return ns
 }

+func (c *configMock) RetryMaxAttempts() int {
+	return 1
+}
+
+func (c *configMock) RetryMaxBackoff() time.Duration {
+	return 0
+}
+
+func (c *configMock) RetryStrategy() RetryStrategy {
+	return RetryStrategyConstant
+}
+
 func prepareHandlerContext(t *testing.T) *handlerContext {
 	return prepareHandlerContextBase(t, layer.DefaultCachesConfigs(zap.NewExample()))
 }
--- a/api/handler/put.go
+++ b/api/handler/put.go
@ -26,12 +26,16 @@ import (
 	"git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/middleware"
 	"git.frostfs.info/TrueCloudLab/frostfs-s3-gw/creds/accessbox"
 	"git.frostfs.info/TrueCloudLab/frostfs-s3-gw/internal/logs"
+	"git.frostfs.info/TrueCloudLab/frostfs-s3-gw/pkg/retryer"
+	"git.frostfs.info/TrueCloudLab/frostfs-s3-gw/pkg/service/tree"
 	cid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/id"
 	"git.frostfs.info/TrueCloudLab/frostfs-sdk-go/eacl"
 	"git.frostfs.info/TrueCloudLab/frostfs-sdk-go/session"
 	"git.frostfs.info/TrueCloudLab/policy-engine/pkg/chain"
 	"git.frostfs.info/TrueCloudLab/policy-engine/schema/native"
 	"git.frostfs.info/TrueCloudLab/policy-engine/schema/s3"
+	"github.com/aws/aws-sdk-go-v2/aws"
+	"github.com/aws/aws-sdk-go-v2/aws/retry"
 	"github.com/nspcc-dev/neo-go/pkg/crypto/keys"
 	"go.uber.org/zap"
 )
@ -922,7 +926,10 @@ func (h *handler) createBucketHandlerPolicy(w http.ResponseWriter, r *http.Reque
 		sp.Settings.Versioning = data.VersioningEnabled
 	}

-	if err = h.obj.PutBucketSettings(ctx, sp); err != nil {
+	err = retryer.MakeWithRetry(ctx, func() error {
+		return h.obj.PutBucketSettings(ctx, sp)
+	}, h.putBucketSettingsRetryer())
+	if err != nil {
 		h.logAndSendError(w, "couldn't save bucket settings", reqInfo, err,
 			zap.String("container_id", bktInfo.CID.EncodeToString()))
 		return
@ -934,6 +941,27 @@ func (h *handler) createBucketHandlerPolicy(w http.ResponseWriter, r *http.Reque
 	}
 }

+func (h *handler) putBucketSettingsRetryer() aws.RetryerV2 {
+	return retry.NewStandard(func(options *retry.StandardOptions) {
+		options.MaxAttempts = h.cfg.RetryMaxAttempts()
+		options.MaxBackoff = h.cfg.RetryMaxBackoff()
+		if h.cfg.RetryStrategy() == RetryStrategyConstant {
+			options.Backoff = retry.NewExponentialJitterBackoff(options.MaxBackoff)
+		} else {
+			options.Backoff = retry.BackoffDelayerFunc(func(int, error) (time.Duration, error) {
+				return options.MaxBackoff, nil
+			})
+		}
+
+		options.Retryables = []retry.IsErrorRetryable{retry.IsErrorRetryableFunc(func(err error) aws.Ternary {
+			if stderrors.Is(err, tree.ErrNodeAccessDenied) {
+				return aws.TrueTernary
+			}
+			return aws.FalseTernary
+		})}
+	})
+}
+
 func (h *handler) createBucketHandlerACL(w http.ResponseWriter, r *http.Request) {
 	ctx := r.Context()
 	reqInfo := middleware.GetReqInfo(ctx)
--- a/cmd/s3-gw/app.go
+++ b/cmd/s3-gw/app.go
@ -111,6 +111,9 @@ type (
 		authorizedControlAPIKeys      [][]byte
 		policyDenyByDefault           bool
 		sourceIPHeader                string
+		retryMaxAttempts              int
+		retryMaxBackoff               time.Duration
+		retryStrategy                 handler.RetryStrategy
 	}

 	maxClientsConfig struct {
@ -238,6 +241,9 @@ func (s *appSettings) update(v *viper.Viper, log *zap.Logger, key *keys.PrivateK
 	s.setAuthorizedControlAPIKeys(append(fetchAuthorizedKeys(log, v), key.PublicKey()))
 	s.setPolicyDenyByDefault(v.GetBool(cfgPolicyDenyByDefault))
 	s.setSourceIPHeader(v.GetString(cfgSourceIPHeader))
+	s.setRetryMaxAttempts(fetchRetryMaxAttempts(v))
+	s.setRetryMaxBackoff(fetchRetryMaxBackoff(v))
+	s.setRetryStrategy(fetchRetryStrategy(v))
 }

 func (s *appSettings) updateNamespacesSettings(v *viper.Viper, log *zap.Logger) {
@ -451,6 +457,42 @@ func (s *appSettings) SourceIPHeader() string {
 	return s.sourceIPHeader
 }

+func (s *appSettings) setRetryMaxAttempts(maxAttempts int) {
+	s.mu.Lock()
+	s.retryMaxAttempts = maxAttempts
+	s.mu.Unlock()
+}
+
+func (s *appSettings) RetryMaxAttempts() int {
+	s.mu.RLock()
+	defer s.mu.RUnlock()
+	return s.retryMaxAttempts
+}
+
+func (s *appSettings) setRetryMaxBackoff(maxBackoff time.Duration) {
+	s.mu.Lock()
+	s.retryMaxBackoff = maxBackoff
+	s.mu.Unlock()
+}
+
+func (s *appSettings) RetryMaxBackoff() time.Duration {
+	s.mu.RLock()
+	defer s.mu.RUnlock()
+	return s.retryMaxBackoff
+}
+
+func (s *appSettings) setRetryStrategy(strategy handler.RetryStrategy) {
+	s.mu.Lock()
+	s.retryStrategy = strategy
+	s.mu.Unlock()
+}
+
+func (s *appSettings) RetryStrategy() handler.RetryStrategy {
+	s.mu.RLock()
+	defer s.mu.RUnlock()
+	return s.retryStrategy
+}
+
 func (a *App) initAPI(ctx context.Context) {
 	a.initLayer(ctx)
 	a.initHandler()
--- a/cmd/s3-gw/app_settings.go
+++ b/cmd/s3-gw/app_settings.go
@ -61,6 +61,10 @@ const (
 	defaultNamespace = ""

 	defaultReconnectInterval = time.Minute
+
+	defaultRetryMaxAttempts = 4
+	defaultRetryMaxBackoff  = 30 * time.Second
+	defaultRetryStrategy    = handler.RetryStrategyExponential
 )

 var (
@ -178,6 +182,11 @@ const ( // Settings.
 	cfgWebWriteTimeout      = "web.write_timeout"
 	cfgWebIdleTimeout       = "web.idle_timeout"

+	// Retry.
+	cfgRetryMaxAttempts = "retry.max_attempts"
+	cfgRetryMaxBackoff  = "retry.max_backoff"
+	cfgRetryStrategy    = "retry.strategy"
+
 	// Namespaces.
 	cfgNamespacesConfig = "namespaces.config"

@ -325,6 +334,33 @@ func fetchSoftMemoryLimit(cfg *viper.Viper) int64 {
 	return int64(softMemoryLimit)
 }

+func fetchRetryMaxAttempts(cfg *viper.Viper) int {
+	val := cfg.GetInt(cfgRetryMaxAttempts)
+	if val <= 0 {
+		val = defaultRetryMaxAttempts
+	}
+
+	return val
+}
+
+func fetchRetryMaxBackoff(cfg *viper.Viper) time.Duration {
+	val := cfg.GetDuration(cfgRetryMaxBackoff)
+	if val <= 0 {
+		val = defaultRetryMaxBackoff
+	}
+
+	return val
+}
+
+func fetchRetryStrategy(cfg *viper.Viper) handler.RetryStrategy {
+	val := handler.RetryStrategy(cfg.GetString(cfgRetryStrategy))
+	if val != handler.RetryStrategyExponential && val != handler.RetryStrategyConstant {
+		val = defaultRetryStrategy
+	}
+
+	return val
+}
+
 func fetchDefaultPolicy(l *zap.Logger, cfg *viper.Viper) netmap.PlacementPolicy {
 	var policy netmap.PlacementPolicy

@ -759,6 +795,10 @@ func newSettings() *viper.Viper {
 	// resolve
 	v.SetDefault(cfgResolveNamespaceHeader, defaultNamespaceHeader)

+	// retry
+	v.SetDefault(cfgRetryMaxAttempts, defaultRetryMaxAttempts)
+	v.SetDefault(cfgRetryMaxBackoff, defaultRetryMaxBackoff)
+
 	// Bind flags
 	if err := bindFlags(v, flags); err != nil {
 		panic(fmt.Errorf("bind flags: %w", err))
--- a/config/config.env
+++ b/config/config.env
@ -223,3 +223,12 @@ S3_GW_NAMESPACES_CONFIG=namespaces.json

 # Custom header to retrieve Source IP
 S3_GW_SOURCE_IP_HEADER=Source-Ip
+
+# Retry strategy configuration.
+# Max amount of request attempts. Currently only for updating bucket settings request.
+S3_GW_RETRY_MAX_ATTEMPTS=4
+# Max delay before next attempt.
+S3_GW_RETRY_MAX_BACKOFF=30s
+# Backoff strategy. `exponential` and `constant` are allowed.
+S3_GW_RETRY_STRATEGY=exponential
+
--- a/config/config.yaml
+++ b/config/config.yaml
@ -262,3 +262,12 @@ namespaces:

 # Custom header to retrieve Source IP
 source_ip_header: "Source-Ip"
+
+# Retry strategy configuration.
+retry:
+  # Max amount of request attempts. Currently only for updating bucket settings request.
+  max_attempts: 4
+  # Max delay before next attempt.
+  max_backoff: 30s
+  # Backoff strategy. `exponential` and `constant` are allowed.
+  strategy: exponential
--- a/docs/configuration.md
+++ b/docs/configuration.md
@ -193,6 +193,7 @@ There are some custom types used for brevity:
 | `policy`           | [Policy contract configuration](#policy-section)               |
 | `proxy`            | [Proxy contract configuration](#proxy-section)                 |
 | `namespaces`       | [Namespaces configuration](#namespaces-section)                |
+| `retry`            | [Retry configuration](#retry-section)                          |

 ### General section

@ -741,3 +742,21 @@ To override config values for default namespaces use namespace names that are pr
  }
 }
 ```
+
+# `retry` section
+
+Retry strategy configuration.
+
+```yaml
+retry:
+  max_attempts: 4
+  max_backoff: 30s
+  strategy: exponential
+```
+
+| Parameter     | Type       | SIGHUP reload | Default value | Description                                                                          |
+|---------------|------------|---------------|---------------|--------------------------------------------------------------------------------------|
+| `max_attemps` | `int`      | yes           | `4`           | Max amount of request attempts. Currently only for updating bucket settings request. |
+| `max_backoff` | `duration` | yes           | `30s`         | Max delay before next attempt.                                                       |
+| `strategy`    | `string`   | yes           | `exponential` | Backoff strategy. `exponential` and `constant` are allowed.                          |
+
--- a/go.mod
+++ b/go.mod
@ -3,13 +3,14 @@ module git.frostfs.info/TrueCloudLab/frostfs-s3-gw
 go 1.20

 require (
-	git.frostfs.info/TrueCloudLab/frostfs-api-go/v2 v2.16.1-0.20240327095603-491a47e7fe24
+	git.frostfs.info/TrueCloudLab/frostfs-api-go/v2 v2.16.1-0.20240516133103-0803bc6ded00
 	git.frostfs.info/TrueCloudLab/frostfs-contract v0.19.3-0.20240409115729-6eb492025bdd
 	git.frostfs.info/TrueCloudLab/frostfs-observability v0.0.0-20230531082742-c97d21411eb6
 	git.frostfs.info/TrueCloudLab/frostfs-sdk-go v0.0.0-20240402141549-3790142b10c7
 	git.frostfs.info/TrueCloudLab/policy-engine v0.0.0-20240527065402-303a81cdc6db
 	git.frostfs.info/TrueCloudLab/zapjournald v0.0.0-20240124114243-cb2e66427d02
 	github.com/aws/aws-sdk-go v1.44.6
+	github.com/aws/aws-sdk-go-v2 v1.18.1
 	github.com/bluele/gcache v0.0.2
 	github.com/go-chi/chi/v5 v5.0.8
 	github.com/google/uuid v1.3.1
@ -36,12 +37,15 @@ require (
 	google.golang.org/protobuf v1.33.0
 )

+replace git.frostfs.info/TrueCloudLab/frostfs-sdk-go => ../frostfs-sdk-go
+
 require (
 	git.frostfs.info/TrueCloudLab/frostfs-crypto v0.6.0 // indirect
 	git.frostfs.info/TrueCloudLab/hrw v1.2.1 // indirect
 	git.frostfs.info/TrueCloudLab/rfc6979 v0.4.0 // indirect
 	git.frostfs.info/TrueCloudLab/tzhash v1.8.0 // indirect
 	github.com/antlr4-go/antlr/v4 v4.13.0 // indirect
+	github.com/aws/smithy-go v1.13.5 // indirect
 	github.com/beorn7/perks v1.0.1 // indirect
 	github.com/cenkalti/backoff/v4 v4.2.1 // indirect
 	github.com/cespare/xxhash/v2 v2.2.0 // indirect
--- a/go.sum
+++ b/go.sum
@ -36,16 +36,14 @@ cloud.google.com/go/storage v1.8.0/go.mod h1:Wv1Oy7z6Yz3DshWRJFhqM/UCfaWIRTdp0RX
 cloud.google.com/go/storage v1.10.0/go.mod h1:FLPqc6j+Ki4BU591ie1oL6qBQGu2Bl/tZ9ullr3+Kg0=
 cloud.google.com/go/storage v1.14.0/go.mod h1:GrKmX003DSIwi9o29oFT7YDnHYwZoctc3fOKtUw0Xmo=
 dmitri.shuralyov.com/gpu/mtl v0.0.0-20190408044501-666a987793e9/go.mod h1:H6x//7gZCb22OMCxBHrMx7a5I7Hp++hsVxbQ4BYO7hU=
-git.frostfs.info/TrueCloudLab/frostfs-api-go/v2 v2.16.1-0.20240327095603-491a47e7fe24 h1:uIkl0mKWwDICUZTbNWZ38HLYDBI9rMgdAhYQWZ0C9iQ=
-git.frostfs.info/TrueCloudLab/frostfs-api-go/v2 v2.16.1-0.20240327095603-491a47e7fe24/go.mod h1:OBDSr+DqV1z4VDouoX3YMleNc4DPBVBWTG3WDT2PK1o=
+git.frostfs.info/TrueCloudLab/frostfs-api-go/v2 v2.16.1-0.20240516133103-0803bc6ded00 h1:Q3B9WtFh05AXhUFs/2CLvhh9tuFs/Zd/XemWBbuzvg8=
+git.frostfs.info/TrueCloudLab/frostfs-api-go/v2 v2.16.1-0.20240516133103-0803bc6ded00/go.mod h1:OBDSr+DqV1z4VDouoX3YMleNc4DPBVBWTG3WDT2PK1o=
 git.frostfs.info/TrueCloudLab/frostfs-contract v0.19.3-0.20240409115729-6eb492025bdd h1:fujTUMMn0wnpEKNDWLejFL916EPuaYD1MdZpk1ZokU8=
 git.frostfs.info/TrueCloudLab/frostfs-contract v0.19.3-0.20240409115729-6eb492025bdd/go.mod h1:F/fe1OoIDKr5Bz99q4sriuHDuf3aZefZy9ZsCqEtgxc=
 git.frostfs.info/TrueCloudLab/frostfs-crypto v0.6.0 h1:FxqFDhQYYgpe41qsIHVOcdzSVCB8JNSfPG7Uk4r2oSk=
 git.frostfs.info/TrueCloudLab/frostfs-crypto v0.6.0/go.mod h1:RUIKZATQLJ+TaYQa60X2fTDwfuhMfm8Ar60bQ5fr+vU=
 git.frostfs.info/TrueCloudLab/frostfs-observability v0.0.0-20230531082742-c97d21411eb6 h1:aGQ6QaAnTerQ5Dq5b2/f9DUQtSqPkZZ/bkMx/HKuLCo=
 git.frostfs.info/TrueCloudLab/frostfs-observability v0.0.0-20230531082742-c97d21411eb6/go.mod h1:W8Nn08/l6aQ7UlIbpF7FsQou7TVpcRD1ZT1KG4TrFhE=
-git.frostfs.info/TrueCloudLab/frostfs-sdk-go v0.0.0-20240402141549-3790142b10c7 h1:sjvYXV0WJAF4iNF3l0uhcN8zhXmpY1gYI0WyJpeFe6s=
-git.frostfs.info/TrueCloudLab/frostfs-sdk-go v0.0.0-20240402141549-3790142b10c7/go.mod h1:i0RKqiF4z3UOxLSNwhHw+cUz/JyYWuTRpnn9ere4Y3w=
 git.frostfs.info/TrueCloudLab/hrw v1.2.1 h1:ccBRK21rFvY5R1WotI6LNoPlizk7qSvdfD8lNIRudVc=
 git.frostfs.info/TrueCloudLab/hrw v1.2.1/go.mod h1:C1Ygde2n843yTZEQ0FP69jYiuaYV0kriLvP4zm8JuvM=
 git.frostfs.info/TrueCloudLab/policy-engine v0.0.0-20240527065402-303a81cdc6db h1:SVtRixp8gYn4orflpXaq3m7ET284kF8dogczIxbQRWs=
@ -64,6 +62,10 @@ github.com/antlr4-go/antlr/v4 v4.13.0 h1:lxCg3LAv+EUK6t1i0y1V6/SLeUi0eKEKdhQAlS8
 github.com/antlr4-go/antlr/v4 v4.13.0/go.mod h1:pfChB/xh/Unjila75QW7+VU4TSnWnnk9UTnmpPaOR2g=
 github.com/aws/aws-sdk-go v1.44.6 h1:Y+uHxmZfhRTLX2X3khkdxCoTZAyGEX21aOUHe1U6geg=
 github.com/aws/aws-sdk-go v1.44.6/go.mod h1:y4AeaBuwd2Lk+GepC1E9v0qOiTws0MIWAX4oIKwKHZo=
+github.com/aws/aws-sdk-go-v2 v1.18.1 h1:+tefE750oAb7ZQGzla6bLkOwfcQCEtC5y2RqoqCeqKo=
+github.com/aws/aws-sdk-go-v2 v1.18.1/go.mod h1:uzbQtefpm44goOPmdKyAlXSNcwlRgF3ePWVW6EtJvvw=
+github.com/aws/smithy-go v1.13.5 h1:hgz0X/DX0dGqTYpGALqXJoRKRj5oQ7150i5FdTePzO8=
+github.com/aws/smithy-go v1.13.5/go.mod h1:Tg+OJXh4MB2R/uN61Ko2f6hTZwB/ZYGOtib8J3gBHzA=
 github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM=
 github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw=
 github.com/bits-and-blooms/bitset v1.8.0 h1:FD+XqgOZDUxxZ8hzoBFuV9+cGWY9CslN6d5MS5JVb4c=
@ -168,6 +170,7 @@ github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/
 github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
 github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
 github.com/google/go-cmp v0.5.6/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
+github.com/google/go-cmp v0.5.8/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
 github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38=
 github.com/google/martian v2.1.0+incompatible/go.mod h1:9I4somxYTbIHy5NJKHRl3wXiIaQGbYVAs8BPL6v8lEs=
 github.com/google/martian/v3 v3.0.0/go.mod h1:y5Zk1BBys9G+gd6Jrk0W3cC1+ELVxBWuIGO+w/tUAp0=
--- a/pkg/retryer/retryer.go
+++ b/pkg/retryer/retryer.go
@ -0,0 +1,53 @@
+package retryer
+
+import (
+	"context"
+	"fmt"
+	"time"
+
+	"github.com/aws/aws-sdk-go-v2/aws"
+)
+
+func MakeWithRetry(ctx context.Context, fn func() error, retryer aws.RetryerV2) (err error) {
+	var attemptNum int
+
+	maxAttempts := retryer.MaxAttempts()
+
+	for {
+		attemptNum++
+
+		err = fn()
+
+		if !retryer.IsErrorRetryable(err) {
+			return err
+		}
+
+		if attemptNum >= maxAttempts {
+			return fmt.Errorf("max retry attempts exausted, max %d: %w", maxAttempts, err)
+		}
+
+		retryDelay, reqErr := retryer.RetryDelay(attemptNum, err)
+		fmt.Println(retryDelay.Milliseconds())
+		if reqErr != nil {
+			return reqErr
+		}
+
+		if reqErr = sleepWithContext(ctx, retryDelay); reqErr != nil {
+			return reqErr
+		}
+	}
+}
+
+func sleepWithContext(ctx context.Context, dur time.Duration) error {
+	t := time.NewTimer(dur)
+	defer t.Stop()
+
+	select {
+	case <-t.C:
+		break
+	case <-ctx.Done():
+		return ctx.Err()
+	}
+
+	return nil
+}