Skip to content
This repository has been archived by the owner on Dec 10, 2024. It is now read-only.

Add an exponential backoff to the retry function #1993

Merged
merged 4 commits into from
Aug 26, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion gitlab.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"errors"
"fmt"
"io"
"math"
"math/rand"
"mime/multipart"
"net/http"
Expand Down Expand Up @@ -509,7 +510,7 @@ func (c *Client) retryHTTPBackoff(min, max time.Duration, attemptNum int, resp *
// min and max are mainly used for bounding the jitter that will be added to
// the reset time retrieved from the headers. But if the final wait time is
// less then min, min will be used instead.
func rateLimitBackoff(min, max time.Duration, _ int, resp *http.Response) time.Duration {
func rateLimitBackoff(min, max time.Duration, attemptNum int, resp *http.Response) time.Duration {
// rnd is used to generate pseudo-random numbers.
rnd := rand.New(rand.NewSource(time.Now().UnixNano()))

Expand All @@ -524,6 +525,10 @@ func rateLimitBackoff(min, max time.Duration, _ int, resp *http.Response) time.D
min = wait
}
}
} else {
// For each attempt without the header, back off an additiona 100% exponentially. With the default milliseconds
// being set to 100 for `min`, this makes the 5th retry wait 3.2 seconds (3,200 ms) by default
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use the suggested as it 1) fixes a typo 2) adds a missing period 3) keeps the comment length below 80 chars:

Suggested change
// For each attempt without the header, back off an additiona 100% exponentially. With the default milliseconds
// being set to 100 for `min`, this makes the 5th retry wait 3.2 seconds (3,200 ms) by default
// In case the RateLimit-Reset header is not set, back off an additional
// 100% exponentially. With the default milliseconds being set to 100 for
// `min`, this makes the 5th retry wait 3.2 seconds (3,200 ms) by default.

min = time.Duration(float64(min) * math.Pow(2, float64(attemptNum)))
}
}

Expand Down
36 changes: 36 additions & 0 deletions gitlab_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -418,3 +418,39 @@ func TestPaginationPopulatePageValuesKeyset(t *testing.T) {
}
}
}

func TestExponentialBackoffLogic(t *testing.T) {
// Can't use the default `setup` because it disabled the backoff
mux := http.NewServeMux()
server := httptest.NewServer(mux)
t.Cleanup(server.Close)
client, err := NewClient("",
WithBaseURL(server.URL),
)
if err != nil {
t.Fatalf("Failed to create client: %v", err)
}

// Create a method that returns 429
mux.HandleFunc("/api/v4/projects/1", func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, http.MethodGet)
w.WriteHeader(http.StatusTooManyRequests)
})

// Measure the time at the start of the test
start := time.Now()

// Send a request (which will get a bunch of 429s)
// None of the responses matter, so ignore them all
_, resp, _ := client.Projects.GetProject(1, nil)
end := time.Now()

// The test should run for _at least_ 3,200 milliseconds
duration := float64(end.Sub(start))
if duration < float64(3200*time.Millisecond) {
t.Fatal("Wait was shorter than expected. Expected a minimum of 5 retries taking 3200 milliseconds, got:", duration)
}
if resp.StatusCode != 429 {
t.Fatal("Expected to get a 429 code given the server is hard-coded to return this. Received instead:", resp.StatusCode)
}
}