Allow requesting fixed retry delay instead of exponential

Clients like ironicclient and swiftclient use fixed delay for their
build-in retry functionality. To replace it without changing behavior
we need a similar feature.

Change-Id: I1f9de98dae5719842f03d45e5a9d724199d5718b

keystoneauth1/adapter.py

@@ -103,6 +103,14 @@ class Adapter(object):
     :param int concurrency:
         How many simultaneous http requests this Adapter can be used for.
         (optional, defaults to None, which means no limit).
+    :param float connect_retry_delay:
+        Delay (in seconds) between two connect retries (if enabled).
+        By default exponential retry starting with 0.5 seconds up to
+        a maximum of 60 seconds is used.
+    :param float status_code_retry_delay:
+        Delay (in seconds) between two status code retries (if enabled).
+        By default exponential retry starting with 0.5 seconds up to
+        a maximum of 60 seconds is used.
     """
 
     client_name = None
@@ -119,6 +127,7 @@ class Adapter(object):
                  default_microversion=None, status_code_retries=None,
                  retriable_status_codes=None, raise_exc=None,
                  rate_limit=None, concurrency=None,
+                 connect_retry_delay=None, status_code_retry_delay=None,
                  ):
         if version and (min_version or max_version):
             raise TypeError(
@@ -148,6 +157,8 @@ class Adapter(object):
         self.default_microversion = default_microversion
         self.status_code_retries = status_code_retries
         self.retriable_status_codes = retriable_status_codes
+        self.connect_retry_delay = connect_retry_delay
+        self.status_code_retry_delay = status_code_retry_delay
         self.raise_exc = raise_exc
 
         self.global_request_id = global_request_id
@@ -195,10 +206,10 @@ class Adapter(object):
             kwargs.setdefault('auth', self.auth)
         if self.user_agent:
             kwargs.setdefault('user_agent', self.user_agent)
-        if self.connect_retries is not None:
+        for arg in ('connect_retries', 'status_code_retries',
-            kwargs.setdefault('connect_retries', self.connect_retries)
+                    'connect_retry_delay', 'status_code_retry_delay'):
-        if self.status_code_retries is not None:
+            if getattr(self, arg) is not None:
-            kwargs.setdefault('status_code_retries', self.status_code_retries)
+                kwargs.setdefault(arg, getattr(self, arg))
         if self.retriable_status_codes:
             kwargs.setdefault('retriable_status_codes',
                               self.retriable_status_codes)

keystoneauth1/loading/adapter.py

@@ -140,11 +140,27 @@ class Adapter(base.BaseLoader):
                                'connect-retries'),
                            help='The maximum number of retries that should be '
                                 'attempted for connection errors.'),
+                cfg.FloatOpt('connect-retry-delay',
+                             deprecated_opts=deprecated_opts.get(
+                                 'connect-retry-delay'),
+                             help='Delay (in seconds) between two retries '
+                                  'for connection errors. If not set, '
+                                  'exponential retry starting with 0.5 '
+                                  'seconds up to a maximum of 60 seconds '
+                                  'is used.'),
                 cfg.IntOpt('status-code-retries',
                            deprecated_opts=deprecated_opts.get(
                                'status-code-retries'),
                            help='The maximum number of retries that should be '
                                 'attempted for retriable HTTP status codes.'),
+                cfg.FloatOpt('status-code-retry-delay',
+                             deprecated_opts=deprecated_opts.get(
+                                 'status-code-retry-delay'),
+                             help='Delay (in seconds) between two retries '
+                                  'for retriable status codes. If not set, '
+                                  'exponential retry starting with 0.5 '
+                                  'seconds up to a maximum of 60 seconds '
+                                  'is used.'),
                 ]
         if include_deprecated:
             opts += [
@@ -271,7 +287,10 @@ def process_conf_options(confgrp, kwargs):
             "version is mutually exclusive with min_version and"
             " max_version")
     kwargs.setdefault('connect_retries', confgrp.connect_retries)
+    kwargs.setdefault('connect_retry_delay', confgrp.connect_retry_delay)
     kwargs.setdefault('status_code_retries', confgrp.status_code_retries)
+    kwargs.setdefault('status_code_retry_delay',
+                      confgrp.status_code_retry_delay)
 
 
 def register_argparse_arguments(*args, **kwargs):

keystoneauth1/session.py

@@ -51,6 +51,7 @@ DEFAULT_USER_AGENT = 'keystoneauth1/%s %s %s/%s' % (
 _LOG_CONTENT_TYPES = set(['application/json'])
 
 _MAX_RETRY_INTERVAL = 60.0
+_EXPONENTIAL_DELAY_START = 0.5
 
 # NOTE(efried): This is defined in oslo_middleware.request_id.INBOUND_HEADER,
 # but it didn't seem worth adding oslo_middleware to requirements just for that
@@ -239,6 +240,29 @@ class RequestTiming(object):
         self.elapsed = elapsed
 
 
+class _Retries(object):
+    __slots__ = ('_fixed_delay', '_current')
+
+    def __init__(self, fixed_delay=None):
+        self._fixed_delay = fixed_delay
+        self.reset()
+
+    def __next__(self):
+        value = self._current
+        if not self._fixed_delay:
+            self._current = min(value * 2, _MAX_RETRY_INTERVAL)
+        return value
+
+    def reset(self):
+        if self._fixed_delay:
+            self._current = self._fixed_delay
+        else:
+            self._current = _EXPONENTIAL_DELAY_START
+
+    # Python 2 compatibility
+    next = __next__
+
+
 class Session(object):
     """Maintains client communication state and common functionality.
 
@@ -583,7 +607,9 @@ class Session(object):
                 allow=None, client_name=None, client_version=None,
                 microversion=None, microversion_service_type=None,
                 status_code_retries=0, retriable_status_codes=None,
-                rate_semaphore=None, global_request_id=None, **kwargs):
+                rate_semaphore=None, global_request_id=None,
+                connect_retry_delay=None, status_code_retry_delay=None,
+                **kwargs):
         """Send an HTTP request with the specified characteristics.
 
         Wrapper around `requests.Session.request` to handle tasks such as
@@ -673,6 +699,16 @@ class Session(object):
                                and rate limiting of requests. (optional,
                                defaults to no concurrency or rate control)
         :param global_request_id: Value for the X-Openstack-Request-Id header.
+        :param float connect_retry_delay: Delay (in seconds) between two
+                                          connect retries (if enabled).
+                                          By default exponential retry starting
+                                          with 0.5 seconds up to a maximum of
+                                          60 seconds is used.
+        :param float status_code_retry_delay: Delay (in seconds) between two
+                                              status code retries (if enabled).
+                                              By default exponential retry
+                                              starting with 0.5 seconds up to
+                                              a maximum of 60 seconds is used.
         :param kwargs: any other parameter that can be passed to
                        :meth:`requests.Session.request` (such as `headers`).
                        Except:
@@ -827,11 +863,15 @@ class Session(object):
         if redirect is None:
             redirect = self.redirect
 
+        connect_retry_delays = _Retries(connect_retry_delay)
+        status_code_retry_delays = _Retries(status_code_retry_delay)
+
         send = functools.partial(self._send_request,
                                  url, method, redirect, log, logger,
                                  split_loggers, connect_retries,
                                  status_code_retries, retriable_status_codes,
-                                 rate_semaphore)
+                                 rate_semaphore, connect_retry_delays,
+                                 status_code_retry_delays)
 
         try:
             connection_params = self.get_auth_connection_params(auth=auth)
@@ -920,7 +960,7 @@ class Session(object):
     def _send_request(self, url, method, redirect, log, logger, split_loggers,
                       connect_retries, status_code_retries,
                       retriable_status_codes, rate_semaphore,
-                      connect_retry_delay=0.5, status_code_retry_delay=0.5,
+                      connect_retry_delays, status_code_retry_delays,
                       **kwargs):
         # NOTE(jamielennox): We handle redirection manually because the
         # requests lib follows some browser patterns where it will redirect
@@ -962,12 +1002,10 @@ class Session(object):
             if connect_retries <= 0:
                 raise
 
+            delay = next(connect_retry_delays)
             logger.info('Failure: %(e)s. Retrying in %(delay).1fs.',
-                        {'e': e, 'delay': connect_retry_delay})
+                        {'e': e, 'delay': delay})
-            time.sleep(connect_retry_delay)
+            time.sleep(delay)
-
-            connect_retry_delay = min(connect_retry_delay * 2,
-                                      _MAX_RETRY_INTERVAL)
 
             return self._send_request(
                 url, method, redirect, log, logger, split_loggers,
@@ -975,7 +1013,8 @@ class Session(object):
                 retriable_status_codes=retriable_status_codes,
                 rate_semaphore=rate_semaphore,
                 connect_retries=connect_retries - 1,
-                connect_retry_delay=connect_retry_delay,
+                connect_retry_delays=connect_retry_delays,
+                status_code_retry_delays=status_code_retry_delays,
                 **kwargs)
 
         if log:
@@ -1000,14 +1039,18 @@ class Session(object):
                 logger.warning("Failed to redirect request to %s as new "
                                "location was not provided.", resp.url)
             else:
-                # NOTE(jamielennox): We don't pass through connect_retry_delay.
+                # NOTE(jamielennox): We don't keep increasing delays.
                 # This request actually worked so we can reset the delay count.
+                connect_retry_delays.reset()
+                status_code_retry_delays.reset()
                 new_resp = self._send_request(
                     location, method, redirect, log, logger, split_loggers,
                     rate_semaphore=rate_semaphore,
                     connect_retries=connect_retries,
                     status_code_retries=status_code_retries,
                     retriable_status_codes=retriable_status_codes,
+                    connect_retry_delays=connect_retry_delays,
+                    status_code_retry_delays=status_code_retry_delays,
                     **kwargs)
 
                 if not isinstance(new_resp.history, list):
@@ -1017,24 +1060,23 @@ class Session(object):
         elif (resp.status_code in retriable_status_codes and
               status_code_retries > 0):
 
+            delay = next(status_code_retry_delays)
             logger.info('Retriable status code %(code)s. Retrying in '
                         '%(delay).1fs.',
-                        {'code': resp.status_code,
+                        {'code': resp.status_code, 'delay': delay})
-                         'delay': status_code_retry_delay})
+            time.sleep(delay)
-            time.sleep(status_code_retry_delay)
 
-            status_code_retry_delay = min(status_code_retry_delay * 2,
+            # NOTE(jamielennox): We don't keep increasing connection delays.
-                                          _MAX_RETRY_INTERVAL)
-
-            # NOTE(jamielennox): We don't pass through connect_retry_delay.
             # This request actually worked so we can reset the delay count.
+            connect_retry_delays.reset()
             return self._send_request(
                 url, method, redirect, log, logger, split_loggers,
                 connect_retries=connect_retries,
                 status_code_retries=status_code_retries - 1,
                 retriable_status_codes=retriable_status_codes,
                 rate_semaphore=rate_semaphore,
-                status_code_retry_delay=status_code_retry_delay,
+                connect_retry_delays=connect_retry_delays,
+                status_code_retry_delays=status_code_retry_delays,
                 **kwargs)
 
         return resp

keystoneauth1/tests/unit/loading/test_adapter.py

@@ -158,19 +158,24 @@ class ConfLoadingTests(utils.TestCase):
         self.conf_fx.config(
             service_type='type', service_name='name',
             connect_retries=3, status_code_retries=5,
+            connect_retry_delay=0.5, status_code_retry_delay=2.0,
             group=self.GROUP)
         adap = loading.load_adapter_from_conf_options(
             self.conf_fx.conf, self.GROUP, session='session', auth='auth')
         self.assertEqual('type', adap.service_type)
         self.assertEqual('name', adap.service_name)
         self.assertEqual(3, adap.connect_retries)
+        self.assertEqual(0.5, adap.connect_retry_delay)
         self.assertEqual(5, adap.status_code_retries)
+        self.assertEqual(2.0, adap.status_code_retry_delay)
 
     def test_get_conf_options(self):
         opts = loading.get_adapter_conf_options()
         for opt in opts:
             if opt.name.endswith('-retries'):
                 self.assertIsInstance(opt, cfg.IntOpt)
+            elif opt.name.endswith('-retry-delay'):
+                self.assertIsInstance(opt, cfg.FloatOpt)
             elif opt.name != 'valid-interfaces':
                 self.assertIsInstance(opt, cfg.StrOpt)
             else:
@@ -179,7 +184,8 @@ class ConfLoadingTests(utils.TestCase):
                           'interface', 'valid-interfaces',
                           'region-name', 'endpoint-override', 'version',
                           'min-version', 'max-version', 'connect-retries',
-                          'status-code-retries'},
+                          'status-code-retries', 'connect-retry-delay',
+                          'status-code-retry-delay'},
                          {opt.name for opt in opts})
 
     def test_get_conf_options_undeprecated(self):
@@ -187,6 +193,8 @@ class ConfLoadingTests(utils.TestCase):
         for opt in opts:
             if opt.name.endswith('-retries'):
                 self.assertIsInstance(opt, cfg.IntOpt)
+            elif opt.name.endswith('-retry-delay'):
+                self.assertIsInstance(opt, cfg.FloatOpt)
             elif opt.name != 'valid-interfaces':
                 self.assertIsInstance(opt, cfg.StrOpt)
             else:
@@ -194,7 +202,8 @@ class ConfLoadingTests(utils.TestCase):
         self.assertEqual({'service-type', 'service-name', 'valid-interfaces',
                           'region-name', 'endpoint-override', 'version',
                           'min-version', 'max-version', 'connect-retries',
-                          'status-code-retries'},
+                          'status-code-retries', 'connect-retry-delay',
+                          'status-code-retry-delay'},
                          {opt.name for opt in opts})
 
     def test_deprecated(self):

keystoneauth1/tests/unit/test_session.py

@@ -458,6 +458,25 @@ class SessionTests(utils.TestCase):
         self.assertThat(self.requests_mock.request_history,
                         matchers.HasLength(retries + 1))
 
+    def test_connect_retries_fixed_delay(self):
+        self.stub_url('GET', exc=requests.exceptions.Timeout())
+
+        session = client_session.Session()
+        retries = 3
+
+        with mock.patch('time.sleep') as m:
+            self.assertRaises(exceptions.ConnectTimeout,
+                              session.get,
+                              self.TEST_URL, connect_retries=retries,
+                              connect_retry_delay=0.5)
+
+            self.assertEqual(retries, m.call_count)
+            m.assert_has_calls([mock.call(0.5)] * retries)
+
+        # we count retries so there will be one initial request + 3 retries
+        self.assertThat(self.requests_mock.request_history,
+                        matchers.HasLength(retries + 1))
+
     def test_http_503_retries(self):
         self.stub_url('GET', status_code=503)
 
@@ -514,6 +533,26 @@ class SessionTests(utils.TestCase):
         self.assertThat(self.requests_mock.request_history,
                         matchers.HasLength(1))
 
+    def test_http_status_retries_fixed_delay(self):
+        self.stub_url('GET', status_code=409)
+
+        session = client_session.Session()
+        retries = 3
+
+        with mock.patch('time.sleep') as m:
+            self.assertRaises(exceptions.Conflict,
+                              session.get,
+                              self.TEST_URL, status_code_retries=retries,
+                              status_code_retry_delay=0.5,
+                              retriable_status_codes=[503, 409])
+
+            self.assertEqual(retries, m.call_count)
+            m.assert_has_calls([mock.call(0.5)] * retries)
+
+        # we count retries so there will be one initial request + 3 retries
+        self.assertThat(self.requests_mock.request_history,
+                        matchers.HasLength(retries + 1))
+
     def test_http_status_retries_inverval_limit(self):
         self.stub_url('GET', status_code=409)
 

releasenotes/notes/retry-delay-68d0c0a1dffcf2fd.yaml

@@ -0,0 +1,6 @@
+---
+features:
+  - |
+    Allows configuring fixed retry delay for connection and status code retries
+    via the new parameters ``connect_retry_delay`` and
+    ``status_code_retry_delay`` accordingly.