This is an automated email from the ASF dual-hosted git repository. tomaz pushed a commit to branch understand-ai-intelligent-retry in repository https://gitbox.apache.org/repos/asf/libcloud.git
commit e86b374af0261d87f423147e40bb6d999edb7da1 Author: Tomaz Muraus <[email protected]> AuthorDate: Wed Nov 3 21:44:30 2021 +0100 Update retry function so we also respect "timeout" argument passed to the function and we don't try to retry on RateLimitReached errors if timeout has been exhausted. --- libcloud/test/test_connection.py | 18 ++++++++++++++++++ libcloud/utils/retry.py | 31 ++++++++++++++++--------------- 2 files changed, 34 insertions(+), 15 deletions(-) diff --git a/libcloud/test/test_connection.py b/libcloud/test/test_connection.py index 525f587..2848426 100644 --- a/libcloud/test/test_connection.py +++ b/libcloud/test/test_connection.py @@ -27,6 +27,7 @@ import libcloud.common.base from libcloud.test import unittest from libcloud.common.base import Connection, CertificateConnection +from libcloud.common.exceptions import RateLimitReachedError from libcloud.http import LibcloudBaseConnection from libcloud.http import LibcloudConnection from libcloud.http import SignedHTTPSAdapter @@ -452,6 +453,23 @@ class ConnectionClassTestCase(unittest.TestCase): self.assertGreater(mock_connect.call_count, 1, 'Retry logic failed') + def test_retry_rate_limit_error_timeout(self): + con = Connection() + con.connection = Mock() + connect_method = 'libcloud.common.base.Connection.request' + + with patch(connect_method) as mock_connect: + mock_connect.__name__ = 'mock_connect' + with self.assertRaises(RateLimitReachedError): + headers = {'retry-after': 0.2} + mock_connect.side_effect = RateLimitReachedError(headers=headers) + retry_request = Retry(timeout=0.4, retry_delay=0.1, + backoff=1) + retry_request(con.request)(action='/') + + self.assertEqual(mock_connect.call_count, 2, + 'Retry logic failed') + class CertificateConnectionClassTestCase(unittest.TestCase): def setUp(self): diff --git a/libcloud/utils/retry.py b/libcloud/utils/retry.py index dd62dbd..c45db86 100644 --- a/libcloud/utils/retry.py +++ b/libcloud/utils/retry.py @@ -43,7 +43,6 @@ class TransientSSLError(ssl.SSLError): DEFAULT_TIMEOUT = 30 # default retry timeout DEFAULT_DELAY = 1 # default sleep delay used in each iterator DEFAULT_BACKOFF = 1 # retry backup multiplier -DEFAULT_MAX_RATE_LIMIT_RETRIES = float("inf") # default max number of times to retry on rate limit RETRY_EXCEPTIONS = (RateLimitReachedError, socket.error, socket.gaierror, httplib.NotConnected, httplib.ImproperConnectionState, TransientSSLError) @@ -52,8 +51,7 @@ RETRY_EXCEPTIONS = (RateLimitReachedError, socket.error, socket.gaierror, class MinimalRetry: def __init__(self, retry_delay=DEFAULT_DELAY, - timeout=DEFAULT_TIMEOUT, backoff=DEFAULT_BACKOFF, - max_rate_limit_retries=DEFAULT_MAX_RATE_LIMIT_RETRIES): + timeout=DEFAULT_TIMEOUT, backoff=DEFAULT_BACKOFF): """ Wrapper around retrying that helps to handle common transient exceptions. This minimalistic version only retries SSL errors and rate limiting. @@ -61,9 +59,6 @@ class MinimalRetry: :param retry_delay: retry delay between the attempts. :param timeout: maximum time to wait. :param backoff: multiplier added to delay between attempts. - :param max_rate_limit_retries: The maximum number of retries to do when being rate limited by the server. - Set to `float("inf")` if retrying forever is desired. - Being rate limited does not count towards the timeout. :Example: @@ -77,15 +72,12 @@ class MinimalRetry: timeout = DEFAULT_TIMEOUT if backoff is None: backoff = DEFAULT_BACKOFF - if max_rate_limit_retries is None: - max_rate_limit_retries = DEFAULT_MAX_RATE_LIMIT_RETRIES timeout = max(timeout, 0) self.retry_delay = retry_delay self.timeout = timeout self.backoff = backoff - self.max_rate_limit_retries = max_rate_limit_retries def __call__(self, func): def transform_ssl_error(function, *args, **kwargs): @@ -101,21 +93,30 @@ class MinimalRetry: def retry_loop(*args, **kwargs): current_delay = self.retry_delay end = datetime.now() + timedelta(seconds=self.timeout) - number_rate_limited_retries = 0 while True: try: return transform_ssl_error(func, *args, **kwargs) except Exception as exc: - if isinstance(exc, RateLimitReachedError) and number_rate_limited_retries <= self.max_rate_limit_retries: + if isinstance(exc, RateLimitReachedError): + if datetime.now() >= end: + # We have exhausted retry timeout so we abort + # retrying + raise + _logger.debug("You are being rate limited, backing off...") - time.sleep(exc.retry_after) + + # NOTE: Retry after defaults to 0 in the + # RateLimitReachedError class so we a use more + # reasonable default in case that attribute is not + # present. This way we prevent busy waiting, etc. + retry_after = exc.retry_after if exc.retry_after else 2 + + time.sleep(retry_after) + # Reset retries if we're told to wait due to rate # limiting current_delay = self.retry_delay - end = datetime.now() + timedelta( - seconds=exc.retry_after + self.timeout) - number_rate_limited_retries += 1 elif datetime.now() >= end: raise elif self.should_retry(exc):
