Skip to content

Commit d6cb41d

Browse files
committed
feat: Introduce retry mechanism for GET requests
Testing the SLO APIs pagination it was found that it's possible to run into the same timing issues as Upsert when listing existing configs. To handle cases where one reply returns a nextPage key/number of configs that does not match the one of the next reply, GET is now retried.
1 parent 708596b commit d6cb41d

File tree

2 files changed

+66
-3
lines changed

2 files changed

+66
-3
lines changed

pkg/rest/config_upload.go

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -400,7 +400,7 @@ func getExistingValuesFromEndpoint(client *http.Client, theApi api.Api, urlStrin
400400
if isPaginated, nextPage := isPaginatedResponse(objmap); isPaginated {
401401
parsedUrl = addNextPageQueryParams(theApi, parsedUrl, nextPage)
402402

403-
resp, err = get(client, parsedUrl.String(), apiToken)
403+
resp, err = getWithRetry(client, parsedUrl.String(), apiToken, 3, 5*time.Second)
404404

405405
if err != nil {
406406
return nil, err
@@ -418,6 +418,33 @@ func getExistingValuesFromEndpoint(client *http.Client, theApi api.Api, urlStrin
418418
return existingValues, nil
419419
}
420420

421+
// getWithRetry works similarly to retry does for PUT and POST
422+
// this method can be used for API calls we know to have occasional timing issues on GET - e.g. paginated queries that are impacted by replication lag, returning unequal amounts of objects/pages per node
423+
func getWithRetry(client *http.Client, url string, apiToken string, maxRetries int, timeout time.Duration) (Response, error) {
424+
resp, err := get(client, url, apiToken)
425+
426+
if err == nil && success(resp) {
427+
return resp, nil
428+
}
429+
430+
for i := 0; i < maxRetries; i++ {
431+
util.Log.Warn("Retrying failed GET request %s after error (HTTP %d): %w", url, resp.StatusCode, err)
432+
time.Sleep(timeout)
433+
resp, err = get(client, url, apiToken)
434+
if err == nil && success(resp) {
435+
return resp, err
436+
}
437+
}
438+
439+
var retryErr error
440+
if err != nil {
441+
retryErr = fmt.Errorf("GET request %s failed after %d retries: %w", url, maxRetries, err)
442+
} else {
443+
retryErr = fmt.Errorf("GET request %s failed after %d retries: (HTTP %d)!\n Response was: %s", url, maxRetries, resp.StatusCode, resp.Body)
444+
}
445+
return Response{}, retryErr
446+
}
447+
421448
func addQueryParamsForNonStandardApis(theApi api.Api, url *url.URL) *url.URL {
422449

423450
queryParams := url.Query()

pkg/rest/config_upload_test.go

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -471,15 +471,51 @@ func Test_GetObjectIdIfAlreadyExists_WorksCorrectlyForAddedQueryParameters(t *te
471471
{
472472
{"nextPageKey", "page42"},
473473
},
474+
{
475+
{"nextPageKey", "page42"},
476+
},
477+
{
478+
{"nextPageKey", "page42"},
479+
},
480+
{
481+
{"nextPageKey", "page42"},
482+
},
474483
},
475-
expectedApiCalls: 2,
484+
expectedApiCalls: 5,
476485
serverResponses: []testServerResponse{
477486
{200, `{ "nextPageKey": "page42", "values": [ {"id": "1", "name": "name1"} ] }`},
478-
{400, `epic fail`},
487+
{400, `epic fail`}, // fail paginated request
488+
{400, `epic fail`}, // still fail after retry
489+
{400, `epic fail`}, // still fail after 2nd retry
490+
{400, `epic fail`}, // still fail after 3rd retry
479491
},
480492
apiKey: "slo",
481493
expectError: true,
482494
},
495+
{
496+
name: "Retries on HTTP error on paginated request and returns eventual success",
497+
expectedQueryParamsPerApiCall: [][]testQueryParams{
498+
{},
499+
{
500+
{"nextPageKey", "page42"},
501+
},
502+
{
503+
{"nextPageKey", "page42"},
504+
},
505+
{
506+
{"nextPageKey", "page42"},
507+
},
508+
},
509+
expectedApiCalls: 4,
510+
serverResponses: []testServerResponse{
511+
{200, `{ "nextPageKey": "page42", "values": [ {"id": "1", "name": "name1"} ] }`},
512+
{400, `epic fail`}, // fail paginated request
513+
{400, `epic fail`}, // still fail after retry
514+
{200, `{ "values": [ {"id": "1", "name": "name1"} ] }`},
515+
},
516+
apiKey: "random-api", //not testing a real API, so this won't break if params are ever added to one
517+
expectError: false,
518+
},
483519
{
484520
name: "Sends correct param to get all SLOs",
485521
expectedQueryParamsPerApiCall: [][]testQueryParams{

0 commit comments

Comments
 (0)