From 7baf1678810c090206a8bee21ccebb2ee498fe60 Mon Sep 17 00:00:00 2001 From: Robert Fairley Date: Thu, 5 Jul 2018 12:35:30 -0400 Subject: [PATCH] ostree/pull: Add network-retries command line option This exposes a way to specify from the command line the number of times to retry each download after a network error. If a negative value is given, then the default number of retries (5) is used. If 0 is given, then errors are returned without retrying. closes #1659 Closes: #1669 Approved by: jlebon --- bash/ostree | 1 + man/ostree-pull.xml | 8 ++++++++ src/ostree/ot-builtin-pull.c | 6 ++++++ tests/test-pull-repeated.sh | 38 +++++++++++++++++++++++++++++++++--- 4 files changed, 50 insertions(+), 3 deletions(-) diff --git a/bash/ostree b/bash/ostree index f3aef686..d7b54373 100644 --- a/bash/ostree +++ b/bash/ostree @@ -908,6 +908,7 @@ _ostree_pull() { --depth --http-header --localcache-repo -L + --network-retries --repo --subpath --update-frequency diff --git a/man/ostree-pull.xml b/man/ostree-pull.xml index 04ca4aaa..84d75be3 100644 --- a/man/ostree-pull.xml +++ b/man/ostree-pull.xml @@ -129,6 +129,14 @@ Boston, MA 02111-1307, USA. Traverse DEPTH parents (-1=infinite) (default: 0). + + + =N + + + Specifies how many times each download should be retried upon error (default: 5) + + diff --git a/src/ostree/ot-builtin-pull.c b/src/ostree/ot-builtin-pull.c index e2d1f09a..8c6569cf 100644 --- a/src/ostree/ot-builtin-pull.c +++ b/src/ostree/ot-builtin-pull.c @@ -44,6 +44,7 @@ static char* opt_cache_dir; static char* opt_append_user_agent; static int opt_depth = 0; static int opt_frequency = 0; +static int opt_network_retries = -1; static char* opt_url; static char** opt_localcache_repos; @@ -68,6 +69,7 @@ static GOptionEntry options[] = { { "url", 0, 0, G_OPTION_ARG_STRING, &opt_url, "Pull objects from this URL instead of the one from the remote config", NULL }, { "http-header", 0, 0, G_OPTION_ARG_STRING_ARRAY, &opt_http_headers, "Add NAME=VALUE as HTTP header to all requests", "NAME=VALUE" }, { "update-frequency", 0, 0, G_OPTION_ARG_INT, &opt_frequency, "Sets the update frequency, in milliseconds (0=1000ms) (default: 0)", "FREQUENCY" }, + { "network-retries", 0, 0, G_OPTION_ARG_INT, &opt_network_retries, "Specifies how many times each download should be retried upon error (default: 5)", "N"}, { "localcache-repo", 'L', 0, G_OPTION_ARG_FILENAME_ARRAY, &opt_localcache_repos, "Add REPO as local cache source for objects during this pull", "REPO" }, { "timestamp-check", 'T', 0, G_OPTION_ARG_NONE, &opt_timestamp_check, "Require fetched commits to have newer timestamps", NULL }, /* let's leave this hidden for now; we just need it for tests */ @@ -293,6 +295,10 @@ ostree_builtin_pull (int argc, char **argv, OstreeCommandInvocation *invocation, g_variant_builder_add (&builder, "{s@v}", "update-frequency", g_variant_new_variant (g_variant_new_uint32 (opt_frequency))); + + if (opt_network_retries >= 0) + g_variant_builder_add (&builder, "{s@v}", "n-network-retries", + g_variant_new_variant (g_variant_new_uint32 (opt_network_retries))); g_variant_builder_add (&builder, "{s@v}", "disable-static-deltas", g_variant_new_variant (g_variant_new_boolean (opt_disable_static_deltas))); diff --git a/tests/test-pull-repeated.sh b/tests/test-pull-repeated.sh index bfc7148a..306d48e5 100755 --- a/tests/test-pull-repeated.sh +++ b/tests/test-pull-repeated.sh @@ -23,7 +23,7 @@ set -euo pipefail . $(dirname $0)/libtest.sh -echo "1..2" +echo "1..4" COMMIT_SIGN="--gpg-homedir=${TEST_GPG_KEYHOME} --gpg-sign=${TEST_GPG_KEYID_1}" @@ -47,7 +47,20 @@ ${CMD_PREFIX} ostree --repo=repo rev-parse main popd echo "ok repeated pull after 500s" -# Now test from a repo which gives error 408 (request timeout) a lot of the time. +# Sanity check with no network retries and 408s given, pull should fail. +rm ostree-srv httpd repo -rf +setup_fake_remote_repo1 "archive" "${COMMIT_SIGN}" --random-408s=99 + +pushd ${test_tmpdir} +ostree_repo_init repo --mode=archive +${CMD_PREFIX} ostree --repo=repo remote add --set=gpg-verify=false origin $(cat httpd-address)/ostree/gnomerepo +assert_fail ${CMD_PREFIX} ostree --repo=repo pull --mirror origin --network-retries=0 main 2>err.txt +assert_file_has_content err.txt "\(408.*Request Timeout\)\|\(HTTP 408\)" + +popd +echo "ok no retries after a 408" + +# Test pulling a repo which gives error 408 (request timeout) a lot of the time. rm ostree-srv httpd repo -rf setup_fake_remote_repo1 "archive" "${COMMIT_SIGN}" --random-408s=50 @@ -55,7 +68,7 @@ pushd ${test_tmpdir} ostree_repo_init repo --mode=archive ${CMD_PREFIX} ostree --repo=repo remote add --set=gpg-verify=false origin $(cat httpd-address)/ostree/gnomerepo for x in $(seq 40); do - if ${CMD_PREFIX} ostree --repo=repo pull --mirror origin main 2>err.txt; then + if ${CMD_PREFIX} ostree --repo=repo pull --mirror origin --network-retries=2 main 2>err.txt; then echo "Success on iteration ${x}" break; fi @@ -67,3 +80,22 @@ ${CMD_PREFIX} ostree --repo=repo rev-parse main popd echo "ok repeated pull after 408s" + +# Test pulling a repo that gives 408s a lot of the time, with many network retries. +rm ostree-srv httpd repo -rf +setup_fake_remote_repo1 "archive" "${COMMIT_SIGN}" --random-408s=50 + +pushd ${test_tmpdir} +ostree_repo_init repo --mode=archive +${CMD_PREFIX} ostree --repo=repo remote add --set=gpg-verify=false origin $(cat httpd-address)/ostree/gnomerepo + +# Using 8 network retries gives error rate of <0.5%, when --random-408s=50 +if ${CMD_PREFIX} ostree --repo=repo pull --mirror origin --network-retries=8 main 2>err.txt; then + echo "Success with big number of network retries" +fi + +${CMD_PREFIX} ostree --repo=repo fsck +${CMD_PREFIX} ostree --repo=repo rev-parse main + +popd +echo "ok big number of retries with one 408"