Skip to content

[IMP] Make the network retries configurable, with jitter #3

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
hugosantosred merged 1 commit into from
Aug 31, 2026
Merged

[IMP] Make the network retries configurable, with jitter #3

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 47 additions & 0 deletions README.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,53 @@ A real life example: applying a patch
shell_command_after:
- git am "$(git format-patch -1 XXXXXX -o ../patches)"

Network retries
---------------

Network git commands (``fetch``, ``pull``, ``push``, ``ls-remote`` and
``clone``) are retried when they fail, with an exponential backoff. This
helps with flaky networks and, above all, with forges that rate limit us:
GitHub answers with an HTTP 401 (which git reports as
``could not read Username for 'https://github.com'``) when too many
unauthenticated git operations come from the same IP address.

The default is 4 retries, waiting 15 seconds before the first one and
doubling the wait each time (15s, 30s, 60s, 120s), with a random jitter of
+/- 20% so that concurrent aggregations do not retry all at the same time
and get rate limited together again.

It can be tuned per repository:

.. code-block:: yaml

./product_attribute:
remotes:
oca: https://github.com/OCA/product-attribute.git
merges:
- oca 16.0
target: oca aggregated_branch_name
retry:
max_retries: 6
delay: 30
backoff_factor: 2
jitter: 0.2

Set ``max_retries`` to ``0`` to disable the retries, and ``backoff_factor``
to ``1`` for a constant delay. Note that the merge check done while loading
the configuration file is never retried, to avoid a very long startup when
the remote is unreachable.

The same parameters are available on the command line, where they take
precedence over the configuration file, which is handy in a CI environment:

.. code-block:: bash

$ gitaggregate -c repos.yaml --retry-max 6 --retry-delay 30

.. code-block:: bash

$ gitaggregate -c repos.yaml --retry-max 0

Command line Usage
==================

Expand Down
30 changes: 24 additions & 6 deletions git_aggregator/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,20 @@
import yaml

from ._compat import string_types
from .repo import Repo, ishex
from .repo import RETRY_KEYS, Repo, ishex
from .exception import ConfigException

log = logging.getLogger(__name__)


def get_repos(config, force=False, skip_merge_check=False):
def get_repos(config, force=False, skip_merge_check=False, retry=None):
"""Return a :py:obj:`list` list of repos from config file.
:param config: the repos config in :py:class:`dict` format.
:param bool force: Force aggregate dirty repos or not.
:param bool skip_merge_check: True to skip the merge check for non existing refs
in remotes.
:param retry: optional dict of retry parameters taking precedence over
the `retry` section of each repo in the config file.
:type config: dict
:rtype: list
"""
Expand All @@ -35,8 +37,20 @@ def get_repos(config, force=False, skip_merge_check=False):
'apply_patch': repo_data.get('apply_patch', False),
'skip_repo_init': repo_data.get('skip_repo_init', False),
}
if 'retry' in repo_data:
repo_dict['retry'] = repo_data['retry']
repo_retry = dict(repo_data.get('retry') or {})
unknown_keys = set(repo_retry) - set(RETRY_KEYS)
if unknown_keys:
raise ConfigException(
'%s: Unknown retry parameter(s) %s. Valid ones are %s.' % (
directory,
', '.join(sorted(unknown_keys)),
', '.join(RETRY_KEYS),
)
)
# Command line parameters win over the config file ones
repo_retry.update(retry or {})
if repo_retry:
repo_dict['retry'] = repo_retry
remote_names = set()
if 'remotes' in repo_data:
repo_dict['remotes'] = []
Expand Down Expand Up @@ -176,7 +190,8 @@ def get_repos(config, force=False, skip_merge_check=False):


def load_config(
config, expand_env=False, env_file=None, force=False, skip_merge_check=False):
config, expand_env=False, env_file=None, force=False,
skip_merge_check=False, retry=None):
"""Return repos from a directory and fnmatch. Not recursive.

:param config: paths to config file
Expand All @@ -188,6 +203,8 @@ def load_config(
:param bool force: True to aggregate even if repo is dirty.
:param bool skip_merge_check: True to skip the merge check for non existing refs
in remotes.
:param retry: optional dict of retry parameters taking precedence over
the `retry` section of each repo in the config file.
:returns: expanded config dict item
:rtype: iter(dict)
"""
Expand Down Expand Up @@ -221,4 +238,5 @@ def load_config(

conf = yaml.load(config, Loader=yaml.SafeLoader)

return get_repos(conf or {}, force, skip_merge_check=skip_merge_check)
return get_repos(
conf or {}, force, skip_merge_check=skip_merge_check, retry=retry)
73 changes: 70 additions & 3 deletions git_aggregator/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@

from .config import load_config
from .log import DebugLogFormatter, LogFormatter
from .repo import Repo
from .repo import DEFAULT_RETRY, RETRY_KEYS, Repo
from .utils import ThreadNameKeeper

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -132,6 +132,47 @@ def get_parser():
'Set `1` or less to disable multiprocessing (default).',
)

main_parser.add_argument(
'--retry-max',
dest='retry_max_retries',
default=None,
type=int,
help='Number of times a failing network git command (fetch, pull, '
'push, ls-remote, clone) is retried. Set `0` to disable '
'retries. Overrides the `retry` section of the configuration '
'file (default: %d).' % DEFAULT_RETRY['max_retries'],
)

main_parser.add_argument(
'--retry-delay',
dest='retry_delay',
default=None,
type=float,
help='Seconds to wait before the first retry (default: %d). '
'Each subsequent retry waits `--retry-backoff-factor` times '
'longer.' % DEFAULT_RETRY['delay'],
)

main_parser.add_argument(
'--retry-backoff-factor',
dest='retry_backoff_factor',
default=None,
type=float,
help='Multiplier applied to the delay of each retry (default: %d). '
'Set `1` for a constant delay.' % DEFAULT_RETRY['backoff_factor'],
)

main_parser.add_argument(
'--retry-jitter',
dest='retry_jitter',
default=None,
type=float,
help='Randomize each retry delay by +/- this ratio, between 0 and 1 '
'(default: %s). This avoids concurrent aggregations retrying at '
'the same time against a rate limiting remote. Set `0` to '
'disable.' % DEFAULT_RETRY['jitter'],
)

main_parser.add_argument(
'--no-color',
dest='no_color',
Expand Down Expand Up @@ -182,6 +223,15 @@ def main():
argcomplete.autocomplete(parser, always_complete_options=False)

args = parser.parse_args()
if args.retry_max_retries is not None and args.retry_max_retries < 0:
parser.error('--retry-max must be a positive number')
if args.retry_delay is not None and args.retry_delay < 0:
parser.error('--retry-delay must be a positive number')
if args.retry_backoff_factor is not None and args.retry_backoff_factor < 1:
parser.error('--retry-backoff-factor must be greater than or equal 1')
if args.retry_jitter is not None and not 0 <= args.retry_jitter < 1:
parser.error('--retry-jitter must be greater than or equal 0 '
'and lower than 1')
if args.no_color:
colorama.init(strip=True)
if not args.command:
Expand All @@ -197,6 +247,20 @@ def main():
return 1


def get_retry_overrides(args):
"""Return the retry parameters explicitly set on the command line.

Parameters left out keep the value from the configuration file, or the
:data:`git_aggregator.repo.DEFAULT_RETRY` one.
"""
overrides = {}
for key in RETRY_KEYS:
value = getattr(args, 'retry_%s' % key, None)
if value is not None:
overrides[key] = value
return overrides


def match_dir(cwd, dirmatch=None):
if not dirmatch:
return True
Expand All @@ -208,7 +272,9 @@ def match_dir(cwd, dirmatch=None):
def load_aggregate(args):
"""Load YAML and JSON configs and begin creating / updating , aggregating
and pushing the repos (deprecated in favor or run())"""
repos = load_config(args.config, args.expand_env, args.env_file)
repos = load_config(
args.config, args.expand_env, args.env_file,
retry=get_retry_overrides(args))
dirmatch = args.dirmatch
for repo_dict in repos:
r = Repo(**repo_dict)
Expand Down Expand Up @@ -253,7 +319,8 @@ def run(args):
in args.command"""

repos = load_config(
args.config, args.expand_env, args.env_file, args.force)
args.config, args.expand_env, args.env_file, args.force,
retry=get_retry_overrides(args))

jobs = max(args.jobs, 1)
threads = []
Expand Down
47 changes: 40 additions & 7 deletions git_aggregator/repo.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
# https://github.com/anybox/anybox.recipe.odoo
import logging
import os
import random
import re
import shutil
import subprocess
Expand All @@ -16,7 +17,13 @@

FETCH_DEFAULTS = ("depth", "shallow-since", "shallow-exclude")
NETWORK_GIT_COMMANDS = ("fetch", "pull", "push", "ls-remote", "clone")
DEFAULT_RETRY = {"max_retries": 3, "delay": 5, "backoff_factor": 2}
DEFAULT_RETRY = {
"max_retries": 4,
"delay": 15,
"backoff_factor": 2,
"jitter": 0.2,
}
RETRY_KEYS = tuple(DEFAULT_RETRY)
logger = logging.getLogger(__name__)


Expand All @@ -28,6 +35,25 @@ def _is_network_command(cmd):
return False


def compute_retry_wait(attempt, delay, backoff_factor, jitter=0):
"""Return how many seconds to wait before the given retry attempt.

Exponential backoff, with an optional random jitter of +/- ``jitter``
(a ratio of the computed wait). The jitter keeps concurrent
aggregations from retrying in lockstep against a remote that is rate
limiting us, which would only get them limited again together.

>>> compute_retry_wait(0, 15, 2)
15
>>> compute_retry_wait(2, 15, 2)
60
"""
wait = delay * backoff_factor ** attempt
if jitter:
wait += wait * random.uniform(-jitter, jitter)
return max(wait, 0)


def ishex(s):
"""True iff given string is a valid hexadecimal number.
>>> ishex('deadbeef')
Expand Down Expand Up @@ -75,6 +101,11 @@ def __init__(self, cwd, remotes, merges, target,
:param bool skip_repo_init:
When ``True``, it will not clone the repository if it does not
exist.
:param retry:
Optional dict overriding :data:`DEFAULT_RETRY` for the network
git commands, with the keys ``max_retries``, ``delay``,
``backoff_factor`` and ``jitter``. Missing keys keep their
default value.
"""
self.cwd = cwd
self.remotes = remotes
Expand All @@ -90,10 +121,9 @@ def __init__(self, cwd, remotes, merges, target,
self.skip_dry_run = skip_dry_run
self.apply_patch = apply_patch
self.skip_repo_init = skip_repo_init
if retry is None:
self.retry = dict(DEFAULT_RETRY)
else:
self.retry = retry
self.retry = dict(DEFAULT_RETRY)
if retry:
self.retry.update(retry)

@property
def git_version(self):
Expand Down Expand Up @@ -189,6 +219,7 @@ def log_call(self, cmd, callwith=subprocess.check_call,
backoff_factor = self.retry.get(
"backoff_factor", DEFAULT_RETRY["backoff_factor"]
)
jitter = self.retry.get("jitter", DEFAULT_RETRY["jitter"])
last_exception = None
for attempt in range(max_retries + 1):
try:
Expand All @@ -199,9 +230,11 @@ def log_call(self, cmd, callwith=subprocess.check_call,
except Exception as exc:
last_exception = exc
if attempt < max_retries:
wait = delay * backoff_factor ** attempt
wait = compute_retry_wait(
attempt, delay, backoff_factor, jitter
)
logger.warning(
"%s> Retry %d/%d for %r in %ds...",
"%s> Retry %d/%d for %r in %.1fs...",
self.cwd, attempt + 1, max_retries, cmd, wait,
)
time.sleep(wait)
Expand Down
Loading
Loading