rate-limited backend: include retry context to propagated errors

* previously, ternimal backend errors gave no indication that rate
  limiter had intervened; raw 429 errors were returned as-is
* this patch wrap throttling errors with retry count and total backoff
* propagate enriched errors through backend methods

Signed-off-by: Tony Chen <a122774007@gmail.com>

Author: Nahemah1022

ais/rlbackend.go

@@ -6,13 +6,13 @@ package ais
 
 import (
 	"context"
+	"fmt"
 	"io"
 	"net/http"
 	"time"
 
 	"github.com/NVIDIA/aistore/cmn"
 	"github.com/NVIDIA/aistore/cmn/cos"
-	"github.com/NVIDIA/aistore/cmn/debug"
 	"github.com/NVIDIA/aistore/core"
 	"github.com/NVIDIA/aistore/core/meta"
 	"github.com/NVIDIA/aistore/stats"
@@ -44,6 +44,11 @@ type (
 		core.Backend
 		t *target
 	}
+	errBackendRetry struct {
+		err     error
+		retries int
+		total   time.Duration
+	}
 )
 
 // note: not counting stats
@@ -93,8 +98,8 @@ func (bp *rlbackend) GetObjReader(ctx context.Context, lom *core.LOM, offset, le
 		res = bp.Backend.GetObjReader(ctx, lom, offset, length)
 		return res.ErrCode, res.Err
 	}
-	total, code, e := bp.retry(ctx, arl, cb)
-	debug.Assertf(res.ErrCode == code && res.Err == e, "(%d, %v) vs (%d, %v)", res.ErrCode, res.Err, code, e)
+	var total time.Duration
+	total, res.ErrCode, res.Err = bp.retry(ctx, arl, cb)
 
 	// ditto
 	bp.stats(ctx, lom.Bck(), stats.RatelimGetRetryCount, stats.RatelimGetRetryLatencyTotal, total)
@@ -159,6 +164,7 @@ func (bp *rlbackend) acquire(bck *meta.Bck, verb string) (arl *cos.AdaptRateLim)
 }
 
 func (*rlbackend) retry(ctx context.Context, arl *cos.AdaptRateLim, cb func() (int, error)) (total time.Duration, ecode int, err error) {
+	var retries int
 	for total < cos.DfltRateMaxWait {
 		// reactive
 		sleep := arl.OnErr()
@@ -167,10 +173,14 @@ func (*rlbackend) retry(ctx context.Context, arl *cos.AdaptRateLim, cb func() (i
 			break
 		}
 		ecode, err = cb()
+		retries++
 		if err == nil || !cmn.IsErrTooManyRequests(err) {
 			break
 		}
 	}
+	if err != nil && cmn.IsErrTooManyRequests(err) {
+		err = &errBackendRetry{err: err, retries: retries, total: total}
+	}
 	return total, ecode, err
 }
 
@@ -187,3 +197,10 @@ func (bp *rlbackend) stats(ctx context.Context, bck *meta.Bck, count, latency st
 		cos.NamedVal64{Name: latency, Value: int64(total), VarLabs: vlabs},
 	)
 }
+
+func (e *errBackendRetry) Error() string {
+	return fmt.Sprintf("backend rate limiter: request still throttled after %d retry attempt%s and %v total backoff: %v",
+		e.retries, cos.Plural(e.retries), e.total, e.err)
+}
+
+func (e *errBackendRetry) Unwrap() error { return e.err }

ais/rlbackend_internal_test.go

@@ -24,6 +24,12 @@ type throttleBackend struct {
 	calls int
 }
 
+type throttleReaderBackend struct {
+	core.Backend
+	err   error
+	calls int
+}
+
 func (bp *throttleBackend) errOnce() error {
 	bp.calls++
 	if bp.calls == 1 {
@@ -48,6 +54,11 @@ func (bp *throttleBackend) GetObj(context.Context, *core.LOM, cmn.OWT, *http.Req
 	return 0, nil
 }
 
+func (bp *throttleReaderBackend) GetObjReader(context.Context, *core.LOM, int64, int64) core.GetReaderResult {
+	bp.calls++
+	return core.GetReaderResult{Err: bp.err, ErrCode: http.StatusTooManyRequests}
+}
+
 func TestRLBackendGetHead(t *testing.T) {
 	tests := []struct {
 		name string
@@ -129,3 +140,42 @@ func TestRLBackendHeadReturnsRetryError(t *testing.T) {
 	tassert.Fatalf(t, oa == nil, "expected no object attributes, got %v", oa)
 	tassert.Fatalf(t, backend.calls == 1, "expected no retry call, got %d calls", backend.calls)
 }
+
+func TestRLBackendGetObjReaderReturnsRetryError(t *testing.T) {
+	lom := core.AllocLOM("rate-limit-reader-retry-error")
+	defer core.FreeLOM(lom)
+	err := lom.InitBck(meta.NewBck(testBucket, apc.AIS, cmn.NsGlobal))
+	tassert.CheckFatal(t, err)
+
+	conf := &lom.Bck().Props.RateLimit.Backend
+	oldEnabled := conf.Enabled
+	conf.Enabled = true
+	defer func() { conf.Enabled = oldEnabled }()
+
+	arl, err := cos.NewAdaptRateLim(100, 1, time.Second)
+	tassert.CheckFatal(t, err)
+	key := lom.Bck().HashUname(http.MethodGet)
+	mockTarget.ratelim.Store(key, arl)
+	defer mockTarget.ratelim.Delete(key)
+
+	err429 := cmn.NewErrTooManyRequests(errors.New("throttled"), http.StatusTooManyRequests)
+	backend := &throttleReaderBackend{err: err429}
+	bp := &rlbackend{Backend: backend, t: mockTarget}
+	ctx, cancel := context.WithCancel(context.Background())
+	cancel()
+	res := bp.GetObjReader(ctx, lom, 0, 0)
+	tassert.Fatalf(t, errors.Is(res.Err, context.Canceled), "expected retry error %v, got %v", context.Canceled, res.Err)
+	tassert.Fatalf(t, res.ErrCode == 0, "expected retry code 0, got %d", res.ErrCode)
+	tassert.Fatalf(t, backend.calls == 1, "expected no retry call, got %d calls", backend.calls)
+}
+
+func TestRLBackendRetryErrorDetails(t *testing.T) {
+	err429 := cmn.NewErrTooManyRequests(errors.New("throttled"), http.StatusTooManyRequests)
+	var err error = &errBackendRetry{err: err429, retries: 3, total: 5 * time.Second}
+	var retryErr *errBackendRetry
+	tassert.Fatalf(t, errors.As(err, &retryErr), "expected backend retry error, got %T", err)
+	tassert.Fatalf(t, retryErr.retries == 3, "expected 3 retries, got %d", retryErr.retries)
+	tassert.Fatalf(t, retryErr.total == 5*time.Second, "expected 5s backoff, got %v", retryErr.total)
+	tassert.Fatalf(t, errors.Is(err, err429), "expected wrapped error %v, got %v", err429, err)
+	tassert.Fatalf(t, cmn.IsErrTooManyRequests(err), "expected throttling error, got %v", err)
+}