Skip to content

Commit 87ee407

Browse files
committed
Retry S3 download on 403 error
1 parent 187f872 commit 87ee407

File tree

7 files changed

+110
-27
lines changed

7 files changed

+110
-27
lines changed

Diff for: modules/ggdeploymentd/src/deployment_handler.c

+80-1
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#include <assert.h>
1515
#include <fcntl.h>
1616
#include <ggl/alloc.h>
17+
#include <ggl/backoff.h>
1718
#include <ggl/base64.h>
1819
#include <ggl/buffer.h>
1920
#include <ggl/bump_alloc.h>
@@ -409,6 +410,84 @@ static GglError get_tes_credentials(TesCredentials *tes_creds) {
409410
return GGL_ERR_OK;
410411
}
411412

413+
typedef struct CurlRequestRetryCtx {
414+
const char *url_for_sigv4_download;
415+
GglBuffer host;
416+
GglBuffer file_path;
417+
SigV4Details sigv4_details;
418+
419+
// reset response_data for next attempt
420+
GglError (*retry_cleanup_fn)(void *);
421+
void *response_data;
422+
423+
// Needed to propagate errors when retrying is impossible.
424+
GglError err;
425+
} DownloadRequestRetryCtx;
426+
427+
static GglError retry_download_wrapper(void *ctx) {
428+
DownloadRequestRetryCtx *retry_ctx = (DownloadRequestRetryCtx *) ctx;
429+
long http_response_code;
430+
431+
GglError ret = sigv4_download(
432+
retry_ctx->url_for_sigv4_download,
433+
retry_ctx->host,
434+
retry_ctx->file_path,
435+
*(int *) retry_ctx->response_data,
436+
retry_ctx->sigv4_details,
437+
&http_response_code
438+
);
439+
if (http_response_code == (long) 403) {
440+
GglError err = retry_ctx->retry_cleanup_fn(retry_ctx->response_data);
441+
GGL_LOGE(
442+
"Artifact download attempt failed with 403. Retrying with backoff."
443+
);
444+
if (err != GGL_ERR_OK) {
445+
retry_ctx->err = err;
446+
return GGL_ERR_OK;
447+
}
448+
return GGL_ERR_FAILURE;
449+
}
450+
if (ret != GGL_ERR_OK) {
451+
GGL_LOGE(
452+
"Artifact download attempt failed due to error: %d", ret
453+
454+
);
455+
retry_ctx->err = ret;
456+
return GGL_ERR_OK;
457+
}
458+
459+
retry_ctx->err = ret;
460+
return GGL_ERR_OK;
461+
}
462+
463+
static GglError retryable_download_request(
464+
const char *url_for_sigv4_download,
465+
GglBuffer host,
466+
GglBuffer file_path,
467+
int artifact_fd,
468+
SigV4Details sigv4_details
469+
) {
470+
DownloadRequestRetryCtx ctx
471+
= { .url_for_sigv4_download = url_for_sigv4_download,
472+
.host = host,
473+
.file_path = file_path,
474+
.sigv4_details = sigv4_details,
475+
.response_data = (void *) &artifact_fd,
476+
.retry_cleanup_fn = truncate_file,
477+
.err = GGL_ERR_OK };
478+
479+
GglError ret
480+
= ggl_backoff(3000, 64000, 3, retry_download_wrapper, (void *) &ctx);
481+
if (ret != GGL_ERR_OK) {
482+
GGL_LOGE("Artifact download attempt failed; retries exhausted.");
483+
return ret;
484+
}
485+
if (ctx.err != GGL_ERR_OK) {
486+
return ctx.err;
487+
}
488+
return GGL_ERR_OK;
489+
}
490+
412491
static GglError download_s3_artifact(
413492
GglBuffer scratch_buffer,
414493
GglUriInfo uri_info,
@@ -434,7 +513,7 @@ static GglError download_s3_artifact(
434513
return error;
435514
}
436515

437-
return sigv4_download(
516+
return retryable_download_request(
438517
(const char *) url_vec.buf.data,
439518
(GglBuffer) { .data = &scratch_buffer.data[start_loc],
440519
.len = end_loc - start_loc },

Diff for: modules/ggl-file/include/ggl/file.h

+2
Original file line numberDiff line numberDiff line change
@@ -87,4 +87,6 @@ static inline void cleanup_closedir(DIR **dirp) {
8787
}
8888
}
8989

90+
GglError truncate_file(void *response_data);
91+
9092
#endif

Diff for: modules/ggl-file/src/file.c

+15
Original file line numberDiff line numberDiff line change
@@ -578,3 +578,18 @@ GglError ggl_file_read_path(GglBuffer path, GglBuffer *content) {
578578

579579
return GGL_ERR_OK;
580580
}
581+
582+
GglError truncate_file(void *response_data) {
583+
int fd = *(int *) response_data;
584+
585+
int ret;
586+
do {
587+
ret = ftruncate(fd, 0);
588+
} while ((ret == -1) && (errno == EINTR));
589+
590+
if (ret == -1) {
591+
GGL_LOGE("Failed to truncate fd for write (errno=%d).", errno);
592+
return GGL_ERR_FAILURE;
593+
}
594+
return GGL_ERR_OK;
595+
}

Diff for: modules/ggl-http/include/ggl/http.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,8 @@ GglError sigv4_download(
9898
GglBuffer host,
9999
GglBuffer file_path,
100100
int fd,
101-
SigV4Details sigv4_details
101+
SigV4Details sigv4_details,
102+
long *http_response_code
102103
);
103104

104105
GglError gg_dataplane_call(

Diff for: modules/ggl-http/src/gghttp.c

+3-1
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,8 @@ GglError sigv4_download(
7878
GglBuffer host,
7979
GglBuffer file_path,
8080
int fd,
81-
SigV4Details sigv4_details
81+
SigV4Details sigv4_details,
82+
long *http_response_code
8283
) {
8384
CurlData curl_data = { 0 };
8485
GglError error = gghttplib_init_curl(&curl_data, url_for_sigv4_download);
@@ -153,6 +154,7 @@ GglError sigv4_download(
153154
long http_status_code = 0;
154155
curl_easy_getinfo(curl_data.curl, CURLINFO_HTTP_CODE, &http_status_code);
155156
GGL_LOGD("Return HTTP code: %ld", http_status_code);
157+
*http_response_code = http_status_code;
156158

157159
struct curl_header *type = NULL;
158160
curl_easy_header(

Diff for: modules/ggl-http/src/gghttp_util.c

-17
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,12 @@
99
#include <sys/types.h>
1010
#include <assert.h>
1111
#include <curl/curl.h>
12-
#include <errno.h>
1312
#include <ggl/backoff.h>
1413
#include <ggl/cleanup.h>
1514
#include <ggl/file.h>
1615
#include <ggl/log.h>
1716
#include <ggl/vector.h>
1817
#include <pthread.h>
19-
#include <unistd.h>
2018
#include <stdbool.h>
2119
#include <stdlib.h>
2220

@@ -109,21 +107,6 @@ static GglError clear_buffer(void *response_data) {
109107
return GGL_ERR_OK;
110108
}
111109

112-
static GglError truncate_file(void *response_data) {
113-
int fd = *(int *) response_data;
114-
115-
int ret;
116-
do {
117-
ret = ftruncate(fd, 0);
118-
} while ((ret == -1) && (errno == EINTR));
119-
120-
if (ret == -1) {
121-
GGL_LOGE("Failed to truncate fd for write (errno=%d).", errno);
122-
return GGL_ERR_FAILURE;
123-
}
124-
return GGL_ERR_OK;
125-
}
126-
127110
static GglError curl_request_retry_wrapper(void *ctx) {
128111
CurlRequestRetryCtx *retry_ctx = (CurlRequestRetryCtx *) ctx;
129112
CurlData *curl_data = retry_ctx->curl_data;

Diff for: test_modules/s3-get-test/src/entry.c

+8-7
Original file line numberDiff line numberDiff line change
@@ -112,20 +112,21 @@ GglError run_s3_test(char *region, char *bucket, char *key, char *file_path) {
112112
if (request_ret != GGL_ERR_OK) {
113113
return GGL_ERR_FAILURE;
114114
}
115+
long http_response_code;
115116

116117
request_ret = sigv4_download(
117118
url_buffer,
118119
(GglBuffer) { .data = host_vec.buf.data, .len = host_vec.buf.len },
119120
ggl_buffer_from_null_term(key),
120121
fd,
121-
(SigV4Details) {
122-
.aws_region = ggl_buffer_from_null_term(region),
123-
.aws_service = GGL_STR("s3"),
124-
.access_key_id = aws_access_key_id->buf,
125-
.secret_access_key = aws_secret_access_key->buf,
126-
.session_token = aws_session_token->buf,
127-
}
122+
(SigV4Details) { .aws_region = ggl_buffer_from_null_term(region),
123+
.aws_service = GGL_STR("s3"),
124+
.access_key_id = aws_access_key_id->buf,
125+
.secret_access_key = aws_secret_access_key->buf,
126+
.session_token = aws_session_token->buf },
127+
&http_response_code
128128
);
129+
(void) http_response_code;
129130
}
130131

131132
int fd = 0;

0 commit comments

Comments
 (0)