[anaconda][rhel7-branch][PATCH] Retry package repository metadata downloads (#1177366)
Vratislav Podzimek
vpodzime at redhat.com
Thu Jun 11 11:43:14 UTC 2015
On Thu, 2015-06-11 at 13:11 +0200, Martin Kolman wrote:
> Anaconda already retries package downloads and package set population,
> but until now it did not retry package repository metadata downloads.
>
> So retry metadata downloads up to 10 times per repository with a
> progressively longer delay between retries. This should make
> network installations on unreliable networks more likely to succeed.
>
> Resolves: rhbz#1177366
> Signed-off-by: Martin Kolman <mkolman at redhat.com>
> ---
> pyanaconda/packaging/yumpayload.py | 36 ++++++++++++++++++++++++++++--------
> 1 file changed, 28 insertions(+), 8 deletions(-)
>
> diff --git a/pyanaconda/packaging/yumpayload.py b/pyanaconda/packaging/yumpayload.py
> index 6574f5e..127ca4e 100644
> --- a/pyanaconda/packaging/yumpayload.py
> +++ b/pyanaconda/packaging/yumpayload.py
> @@ -68,7 +68,7 @@ from pyanaconda.constants import BASE_REPO_NAME, DRACUT_ISODIR, INSTALL_TREE, IS
> from pyanaconda.flags import flags
>
> from pyanaconda import iutil
> -from pyanaconda.iutil import ProxyString, ProxyStringError
> +from pyanaconda.iutil import ProxyString, ProxyStringError, xprogressive_delay
> from pyanaconda.i18n import _
> from pyanaconda.nm import nm_is_connected
> from pyanaconda.product import isFinal
> @@ -90,6 +90,8 @@ YUM_PLUGINS = ["fastestmirror", "langpacks"]
> YUM_REPOS_DIR = "/etc/yum.repos.d/"
> BASE_REPO_NAMES = [BASE_REPO_NAME] + PackagePayload.DEFAULT_REPOS
>
> +MAX_METADATA_DOWNLOAD_RETRIES = 10
> +
> import inspect
> import threading
> _private_yum_lock = threading.RLock()
> @@ -629,13 +631,31 @@ reposdir=%s
> with _yum_lock:
> repo = self._yum.repos.getRepo(repo_id)
> if repo.enabled:
> - try:
> - log.info("gathering repo metadata for %s", repo_id)
> - self._getRepoMetadata(repo)
> - except PayloadError as e:
> - log.error("failed to grab repo metadata for %s: %s",
> - repo_id, e)
> - self.disableRepo(repo_id)
> + # retry metadata downloads with a progressively longer pause,
> + # so that unattended installations on unreliable networks have
> + # a higher chance of finishing successfully
> + xdelay = xprogressive_delay()
> + for retry_count in xrange(0, MAX_METADATA_DOWNLOAD_RETRIES + 1):
> + # retry count == 0 -> first attempt
> + # retry count > 0 -> retry
> + if retry_count:
If you use 'if retry_count > 0' here, you can throw away the comment
above it while keeping things clear, I think. If you feel like it, you
can add the '# first attempt' comment to the inner block of the 'if'.
> + time.sleep(next(xdelay)) # wait a bit before retry
> + try:
> + log.info("gathering repo metadata for %s", repo_id)
> + self._getRepoMetadata(repo)
> + log.info("gathered repo metadata for %s", repo_id)
> + break
> + except PayloadError as e:
> + log.error("failed to grab repo metadata for %s: %s", repo_id, e)
> + if retry_count < MAX_METADATA_DOWNLOAD_RETRIES:
> + # retry
> + log.error("retrying metadata download for repo %s, retrying (%d/%d)",
> + repo_id, retry_count + 1, MAX_METADATA_DOWNLOAD_RETRIES)
This doesn't seem to me like an error message. log.info() would make
more sense to me here.
> + else:
> + # run out of retries
> + log.error("metadata download for repo %s failed after %d retries",
> + repo_id, retry_count)
> + self.disableRepo(repo_id)
> else:
> log.info("skipping disabled repo %s", repo_id)
Other than the minor things above this looks good to me.
--
Vratislav Podzimek
Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic
More information about the anaconda-patches
mailing list