[Yum-devel] [PATCH] async urlgrab: send 'tries' to failure callback, reuse failed mirrors

James Antill james at fedoraproject.org
Wed Jul 18 18:41:50 UTC 2012


On Wed, 2012-07-18 at 17:19 +0200, Zdeněk Pavlas wrote:
> Make the removal of a failed mirror conditional, so it's in line
> with synchronous urlgrab.  Mirror failures are counted and the
> mirror selection always uses mirror with least failures.
> (this has higher priority than speed estimation)
> ---
>  urlgrabber/grabber.py |   16 ++++++++++------
>  urlgrabber/mirror.py  |    2 +-
>  2 files changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/urlgrabber/grabber.py b/urlgrabber/grabber.py
> index 057cbbe..344ba97 100644
> --- a/urlgrabber/grabber.py
> +++ b/urlgrabber/grabber.py
> @@ -2176,13 +2176,16 @@ def parallel_wait(meter = 'text'):
>                  continue
>  
>              if opts.mirror_group:
> -                mg, failed = opts.mirror_group
> +                mg, failed, removed = opts.mirror_group

 Is this an API visible change?

> +                failed[key] = failed.get(key, 0) + 1
> +                opts.tries = sum(failed.values())
>                  opts.mirror = key
>                  opts.exception = ug_err
> -                action = _run_callback(mg.failure_callback, opts)
> -                if not (action and action.get('fail')):
> +                action = _run_callback(mg.failure_callback, opts) or {}

 These "or {}" things are really easy to miss, creating a
_run_callback_dict() or just adding extra lines isn't that big a deal.

> +                if not action.get('fail', 0):
>                      # mask this mirror and retry
> -                    failed.add(key)
> +                    if action.get('remove', 1):
> +                        removed.add(key)
>                      _async_queue.append(opts)
>                      continue
>  
> @@ -2209,17 +2212,18 @@ def parallel_wait(meter = 'text'):
>                  perform()
>  
>              if opts.mirror_group:
> -                mg, failed = opts.mirror_group
> +                mg, failed, removed = opts.mirror_group
>  
>                  # find the best mirror
>                  best = None
>                  for mirror in mg.mirrors:
>                      key = mirror['mirror']
> -                    if key in failed: continue
> +                    if key in removed: continue
>  
>                      # estimate mirror speed
>                      speed = _TH.estimate(key)
>                      speed /= 1 + host_con.get(key, 0)
> +                    speed = -failed.get(key, 0), speed
>                      if best is None or speed > best_speed:
>                          best = mirror
>                          best_speed = speed

 Maybe at least comment this, that you are now using a tuple to order by
smallest number of fails and largest speed. Also it's only a single
extra line to do "best_speed = None" where best is initialized.

> diff --git a/urlgrabber/mirror.py b/urlgrabber/mirror.py
> index 675c2bf..83e68b6 100644
> --- a/urlgrabber/mirror.py
> +++ b/urlgrabber/mirror.py
> @@ -411,7 +411,7 @@ class MirrorGroup:
>          kw['filename'] = filename
>          if kw.get('async'):
>              # enable mirror failovers in async path
> -            kw['mirror_group'] = self, set()
> +            kw['mirror_group'] = self, {}, set()
>              kw['relative_url'] = url
>          else:
>              kw.pop('failfunc', None)




More information about the Yum-devel mailing list