From 0721ddfdbad7bcce61060283bca64ac847464302 Mon Sep 17 00:00:00 2001 From: Castulo Martinez Date: Wed, 30 Jan 2019 00:13:33 +0000 Subject: [PATCH] Adding configurable download retries This commit adds two new options: -r, --max-retries: maximum number of retries for download failures -d, --retry-delay: initial delay between download retries With these two is possible to increase the number of retries in case the user is in a non reliable network, or to turn off retries intirely (by setting --max-retries to 0). The --retry-delay will also allow more flexibility for unreliable connections, and will make testing easier since the --retry-delat can also be set to 0. Closes #322 Signed-off-by: Castulo Martinez --- src/curl.c | 31 ++++++++++++++++++++++++------- src/curl_async.c | 8 ++++---- src/globals.c | 22 +++++++++++++++++++++- src/swupd.h | 7 ++++--- src/swupd_curl.h | 2 +- 5 files changed, 54 insertions(+), 16 deletions(-) diff --git a/src/curl.c b/src/curl.c index 1be15678..8a2281a1 100644 --- a/src/curl.c +++ b/src/curl.c @@ -567,7 +567,7 @@ static int retry_download_loop(const char *url, char *filename, struct curl_file { int current_retry = 0; - int sleep_time = RETRY_DELAY; + int sleep_time = retry_delay; int strategy; int ret; @@ -591,17 +591,34 @@ static int retry_download_loop(const char *url, char *filename, struct curl_file case RETRY_NOW: /* if we have reached the retry limit just return the failure, * if not try again immediately */ - if (current_retry <= MAX_TRIES) { - continue; + if (max_retries) { + if (current_retry <= max_retries) { + fprintf(stderr, "Retry #%d downloading from %s\n", current_retry, url); + continue; + } else { + fprintf(stderr, "Maximum number of retries reached\n"); + } + } else { + fprintf(stderr, "Download retries is disabled\n"); } return ret; case RETRY_WITH_DELAY: /* if we have reached the retry limit just return the failure, * if not, wait for the delay and try again */ - if (current_retry <= MAX_TRIES) { - sleep(sleep_time); - sleep_time *= DELAY_MULTIPLIER; - continue; + if (max_retries) { + if (current_retry <= max_retries) { + if (sleep_time) { + fprintf(stderr, "Waiting %d seconds before retrying the download\n", sleep_time); + } + fprintf(stderr, "Retry #%d downloading from %s\n", current_retry, url); + sleep(sleep_time); + sleep_time = (sleep_time * DELAY_MULTIPLIER) > MAX_DELAY ? MAX_DELAY : (sleep_time * DELAY_MULTIPLIER); + continue; + } else { + fprintf(stderr, "Maximum number of retries reached\n"); + } + } else { + fprintf(stderr, "Download retries is disabled\n"); } return ret; default: diff --git a/src/curl_async.c b/src/curl_async.c index 4c370e09..ad150d89 100644 --- a/src/curl_async.c +++ b/src/curl_async.c @@ -191,7 +191,7 @@ void *swupd_curl_parallel_download_start(size_t max_xfer) h->max_xfer = max_xfer; h->curl_hashmap = hashmap_new(SWUPD_CURL_HASH_BUCKETS, file_hash_cmp, file_hash_value); - h->retry_delay = RETRY_DELAY; + h->retry_delay = retry_delay; return h; error: @@ -265,7 +265,7 @@ static int perform_curl_io_and_complete(struct swupd_curl_parallel_handle *h, in //Check if user can handle errors if (!h->error_cb || h->error_cb(file->status, file->data)) { // Don't retry download if error was handled - file->retries = MAX_TRIES; + file->retries = max_retries; file->cb_retval = true; h->failed = list_prepend_data(h->failed, file); } else { @@ -533,7 +533,7 @@ int swupd_curl_parallel_download_end(void *handle, int *num_downloads) for (l = h->failed; l;) { struct multi_curl_file *file = l->data; - if (file->retries < MAX_TRIES && + if (file->retries < max_retries && file->status != DOWNLOAD_STATUS_WRITE_ERROR) { struct list *next; @@ -559,7 +559,7 @@ int swupd_curl_parallel_download_end(void *handle, int *num_downloads) } if (retry) { sleep(h->retry_delay); - h->retry_delay *= DELAY_MULTIPLIER; + h->retry_delay = (h->retry_delay * DELAY_MULTIPLIER) > MAX_DELAY ? MAX_DELAY : (h->retry_delay * DELAY_MULTIPLIER); } } diff --git a/src/globals.c b/src/globals.c index 612fdef7..e1fb7003 100644 --- a/src/globals.c +++ b/src/globals.c @@ -58,6 +58,8 @@ char *state_dir = NULL; int skip_diskspace_check = 0; bool keepcache = false; timelist *global_times = NULL; +int max_retries = 3; +int retry_delay = 10; /* NOTE: Today the content and version server urls are the same in * all cases. It is highly likely these will eventually differ, eg: @@ -565,6 +567,8 @@ static const struct option global_opts[] = { { "debug", no_argument, &log_level, LOG_DEBUG }, //TODO: -D option is deprecated. Remove that on a Major release { "", required_argument, 0, 'D' }, + { "max-retries", required_argument, 0, 'r' }, + { "retry-delay", required_argument, 0, 'd' }, { 0, 0, 0, 0 } }; @@ -636,6 +640,20 @@ static bool global_parse_opt(int opt, char *optarg) return false; } return true; + case 'r': + err = strtoi_err(optarg, &max_retries); + if (err < 0 || max_retries < 0) { + fprintf(stderr, "Invalid --max-retries argument: %s\n\n", optarg); + return false; + } + return true; + case 'd': + err = strtoi_err(optarg, &retry_delay); + if (err < 0 || retry_delay < 0 || retry_delay > 60) { + fprintf(stderr, "Invalid --retry-delay argument: %s (should be between 0 - %d seconds)\n\n", optarg, MAX_DELAY); + return false; + } + return true; default: return false; } @@ -681,7 +699,7 @@ void global_print_help(void) { fprintf(stderr, "Global Options:\n"); fprintf(stderr, " -h, --help Show help options\n"); - fprintf(stderr, " -p, --path=[PATH...] Use [PATH...] as the path to verify (eg: a chroot or btrfs subvol\n"); + fprintf(stderr, " -p, --path=[PATH...] Use [PATH...] as the path to verify (eg: a chroot or btrfs subvol)\n"); fprintf(stderr, " -u, --url=[URL] RFC-3986 encoded url for version string and content file downloads\n"); fprintf(stderr, " -P, --port=[port #] Port number to connect to at the url for version string and content file downloads\n"); fprintf(stderr, " -c, --contenturl=[URL] RFC-3986 encoded url for content file downloads\n"); @@ -696,6 +714,8 @@ void global_print_help(void) fprintf(stderr, " -N, --no-scripts Do not run the post-update scripts and boot update tool\n"); fprintf(stderr, " -b, --no-boot-update Do not install boot files to the boot partition (containers)\n"); fprintf(stderr, " -W, --max-parallel-downloads=[n] Set the maximum number of parallel downloads\n"); + fprintf(stderr, " -r, --max-retries Maximum number of retries for download failures\n"); + fprintf(stderr, " -d, --retry-delay Initial delay between download retries, this will be doubled for each retry\n"); fprintf(stderr, "\n"); } diff --git a/src/swupd.h b/src/swupd.h index 5b8d5cf6..9c4f5a0e 100644 --- a/src/swupd.h +++ b/src/swupd.h @@ -34,10 +34,9 @@ extern "C" { #define UNUSED_PARAM __attribute__((__unused__)) -/* values used for configuring downloads */ -#define MAX_TRIES 3 -#define RETRY_DELAY 10 +/* value used for configuring downloads */ #define DELAY_MULTIPLIER 2 +#define MAX_DELAY 60 #define SWUPD_HASH_DIRNAME "DIRECTORY" #define SWUPD_DEFAULTS "/usr/share/defaults/swupd/" @@ -187,6 +186,8 @@ extern char *bundle_to_add; extern char *state_dir; extern int skip_diskspace_check; extern timelist *global_times; +extern int max_retries; +extern int retry_delay; extern char *version_url; extern char *content_url; diff --git a/src/swupd_curl.h b/src/swupd_curl.h index 20a696d9..e5f5fb01 100644 --- a/src/swupd_curl.h +++ b/src/swupd_curl.h @@ -81,7 +81,7 @@ int swupd_curl_get_file_memory(const char *url, struct curl_file_data *file_data * Parameters: * - max_xfer: The maximum number of simultaneos downloads. * - * Parallel download handler will retry MAX_TRIES times to download each file, + * Parallel download handler will retry max_retries times to download each file, * ading a timeout between each try. * * Note: This function is non-blocking.