Limit interval between retries to 1 minute
Currently it grows exponentially, exceeding 1 hour after 15 retries. While we don't expect people to have so many retries, we should not let them shoot their legs. Change-Id: I01dfaa1c379340a0d41fcfdb07298fdef6110941
This commit is contained in:
parent
96559d6009
commit
34c005ae5f
@ -50,6 +50,8 @@ DEFAULT_USER_AGENT = 'keystoneauth1/%s %s %s/%s' % (
|
|||||||
# here and we'll add it to the list as required.
|
# here and we'll add it to the list as required.
|
||||||
_LOG_CONTENT_TYPES = set(['application/json'])
|
_LOG_CONTENT_TYPES = set(['application/json'])
|
||||||
|
|
||||||
|
_MAX_RETRY_INTERVAL = 60.0
|
||||||
|
|
||||||
|
|
||||||
def _construct_session(session_obj=None):
|
def _construct_session(session_obj=None):
|
||||||
# NOTE(morganfainberg): if the logic in this function changes be sure to
|
# NOTE(morganfainberg): if the logic in this function changes be sure to
|
||||||
@ -953,13 +955,16 @@ class Session(object):
|
|||||||
{'e': e, 'delay': connect_retry_delay})
|
{'e': e, 'delay': connect_retry_delay})
|
||||||
time.sleep(connect_retry_delay)
|
time.sleep(connect_retry_delay)
|
||||||
|
|
||||||
|
connect_retry_delay = min(connect_retry_delay * 2,
|
||||||
|
_MAX_RETRY_INTERVAL)
|
||||||
|
|
||||||
return self._send_request(
|
return self._send_request(
|
||||||
url, method, redirect, log, logger, split_loggers,
|
url, method, redirect, log, logger, split_loggers,
|
||||||
status_code_retries=status_code_retries,
|
status_code_retries=status_code_retries,
|
||||||
retriable_status_codes=retriable_status_codes,
|
retriable_status_codes=retriable_status_codes,
|
||||||
rate_semaphore=rate_semaphore,
|
rate_semaphore=rate_semaphore,
|
||||||
connect_retries=connect_retries - 1,
|
connect_retries=connect_retries - 1,
|
||||||
connect_retry_delay=connect_retry_delay * 2,
|
connect_retry_delay=connect_retry_delay,
|
||||||
**kwargs)
|
**kwargs)
|
||||||
|
|
||||||
if log:
|
if log:
|
||||||
@ -1007,6 +1012,9 @@ class Session(object):
|
|||||||
'delay': status_code_retry_delay})
|
'delay': status_code_retry_delay})
|
||||||
time.sleep(status_code_retry_delay)
|
time.sleep(status_code_retry_delay)
|
||||||
|
|
||||||
|
status_code_retry_delay = min(status_code_retry_delay * 2,
|
||||||
|
_MAX_RETRY_INTERVAL)
|
||||||
|
|
||||||
# NOTE(jamielennox): We don't pass through connect_retry_delay.
|
# NOTE(jamielennox): We don't pass through connect_retry_delay.
|
||||||
# This request actually worked so we can reset the delay count.
|
# This request actually worked so we can reset the delay count.
|
||||||
return self._send_request(
|
return self._send_request(
|
||||||
@ -1015,7 +1023,7 @@ class Session(object):
|
|||||||
status_code_retries=status_code_retries - 1,
|
status_code_retries=status_code_retries - 1,
|
||||||
retriable_status_codes=retriable_status_codes,
|
retriable_status_codes=retriable_status_codes,
|
||||||
rate_semaphore=rate_semaphore,
|
rate_semaphore=rate_semaphore,
|
||||||
status_code_retry_delay=status_code_retry_delay * 2,
|
status_code_retry_delay=status_code_retry_delay,
|
||||||
**kwargs)
|
**kwargs)
|
||||||
|
|
||||||
return resp
|
return resp
|
||||||
|
@ -440,6 +440,24 @@ class SessionTests(utils.TestCase):
|
|||||||
self.assertThat(self.requests_mock.request_history,
|
self.assertThat(self.requests_mock.request_history,
|
||||||
matchers.HasLength(retries + 1))
|
matchers.HasLength(retries + 1))
|
||||||
|
|
||||||
|
def test_connect_retries_interval_limit(self):
|
||||||
|
self.stub_url('GET', exc=requests.exceptions.Timeout())
|
||||||
|
|
||||||
|
session = client_session.Session()
|
||||||
|
retries = 20
|
||||||
|
|
||||||
|
with mock.patch('time.sleep') as m:
|
||||||
|
self.assertRaises(exceptions.ConnectTimeout,
|
||||||
|
session.get,
|
||||||
|
self.TEST_URL, connect_retries=retries)
|
||||||
|
|
||||||
|
self.assertEqual(retries, m.call_count)
|
||||||
|
# The interval maxes out at 60
|
||||||
|
m.assert_called_with(60.0)
|
||||||
|
|
||||||
|
self.assertThat(self.requests_mock.request_history,
|
||||||
|
matchers.HasLength(retries + 1))
|
||||||
|
|
||||||
def test_http_503_retries(self):
|
def test_http_503_retries(self):
|
||||||
self.stub_url('GET', status_code=503)
|
self.stub_url('GET', status_code=503)
|
||||||
|
|
||||||
@ -496,6 +514,26 @@ class SessionTests(utils.TestCase):
|
|||||||
self.assertThat(self.requests_mock.request_history,
|
self.assertThat(self.requests_mock.request_history,
|
||||||
matchers.HasLength(1))
|
matchers.HasLength(1))
|
||||||
|
|
||||||
|
def test_http_status_retries_inverval_limit(self):
|
||||||
|
self.stub_url('GET', status_code=409)
|
||||||
|
|
||||||
|
session = client_session.Session()
|
||||||
|
retries = 20
|
||||||
|
|
||||||
|
with mock.patch('time.sleep') as m:
|
||||||
|
self.assertRaises(exceptions.Conflict,
|
||||||
|
session.get,
|
||||||
|
self.TEST_URL, status_code_retries=retries,
|
||||||
|
retriable_status_codes=[503, 409])
|
||||||
|
|
||||||
|
self.assertEqual(retries, m.call_count)
|
||||||
|
# The interval maxes out at 60
|
||||||
|
m.assert_called_with(60.0)
|
||||||
|
|
||||||
|
# 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_uses_tcp_keepalive_by_default(self):
|
def test_uses_tcp_keepalive_by_default(self):
|
||||||
session = client_session.Session()
|
session = client_session.Session()
|
||||||
requests_session = session.session
|
requests_session = session.session
|
||||||
|
6
releasenotes/notes/retries-limit-dbaedcb3207934ae.yaml
Normal file
6
releasenotes/notes/retries-limit-dbaedcb3207934ae.yaml
Normal file
@ -0,0 +1,6 @@
|
|||||||
|
---
|
||||||
|
fixes:
|
||||||
|
- |
|
||||||
|
The retry interval for retries enabled by ``connect_retries`` and
|
||||||
|
``status_code_retries`` is now limited at 60 seconds. Previously it would
|
||||||
|
grow exponentially.
|
Loading…
Reference in New Issue
Block a user