Fixed delays for HTTP retries rather than exponential delays (Azure#1967

)

azurelinuxagent/common/utils/restutil.py

@@ -35,8 +35,8 @@
 
 SECURE_WARNING_EMITTED = False
 
-DEFAULT_RETRIES = 6
-DELAY_IN_SECONDS = 1
+DEFAULT_RETRIES = 3
+DELAY_IN_SECONDS = 5
 
 THROTTLE_RETRIES = 25
 THROTTLE_DELAY_IN_SECONDS = 1
@@ -135,13 +135,6 @@ def set_protocol_endpoint(endpoint=KNOWN_WIRESERVER_IP):
         IOErrorCounter._protocol_endpoint = endpoint
 
 
-def _compute_delay(retry_attempt=1, delay=DELAY_IN_SECONDS):
-    fib = (1, 1)
-    for n in range(retry_attempt):
-        fib = (fib[1], fib[0]+fib[1])
-    return delay*fib[1]
-
-
 def _is_retry_status(status, retry_codes=RETRY_CODES):
     return status in retry_codes
 
@@ -385,12 +378,8 @@ def http_request(method,
             # Compute the request delay
             # -- Use a fixed delay if the server ever rate-throttles the request
             #    (with a safe, minimum number of retry attempts)
-            # -- Otherwise, compute a delay that is the product of the next
-            #    item in the Fibonacci series and the initial delay value
-            delay = THROTTLE_DELAY_IN_SECONDS \
-                        if was_throttled \
-                        else _compute_delay(retry_attempt=attempt,
-                                            delay=retry_delay)
+            # -- Otherwise, use the retry_delay (fixed) with maximum of max_retry retries for the rest of the requests.
+            delay = THROTTLE_DELAY_IN_SECONDS if was_throttled else retry_delay
 
             logger.verbose("[HTTP Retry] "
                         "Attempt {0} of {1} will delay {2} seconds: {3}",

tests/utils/test_rest_util.py

@@ -17,11 +17,14 @@
 
 import os
 import unittest
+from datetime import datetime, timedelta
+from random import randint
 
 from azurelinuxagent.common.exception import HttpError, ResourceGoneError, InvalidContainerError
 import azurelinuxagent.common.utils.restutil as restutil
 from azurelinuxagent.common.utils.restutil import HTTP_USER_AGENT
 from azurelinuxagent.common.future import httpclient, ustr
+from tests.protocol.mocks import MockHttpResponse
 from tests.tools import AgentTestCase, call, Mock, MagicMock, patch
 
 
@@ -578,27 +581,28 @@ def test_http_request_retries_passed_status_codes(self, _http_request, _sleep):
         self.assertEqual(2, _http_request.call_count)
         self.assertEqual(1, _sleep.call_count)
 
-    @patch("time.sleep")
-    @patch("azurelinuxagent.common.utils.restutil._http_request")
-    def test_http_request_retries_with_fibonacci_delay(self, _http_request, _sleep):
-        # Ensure the code is not a throttle code
-        self.assertFalse(httpclient.BAD_GATEWAY in restutil.THROTTLE_CODES)
-
-        _http_request.side_effect = [
-                Mock(status=httpclient.BAD_GATEWAY)
-                    for i in range(restutil.DEFAULT_RETRIES)
-            ] + [Mock(status=httpclient.OK)]
-
-        restutil.http_get("https://foo.bar",
-                            max_retry=restutil.DEFAULT_RETRIES+1)
-
-        self.assertEqual(restutil.DEFAULT_RETRIES+1, _http_request.call_count)
-        self.assertEqual(restutil.DEFAULT_RETRIES, _sleep.call_count)
-        self.assertEqual(
-            [
-                call(restutil._compute_delay(i+1, restutil.DELAY_IN_SECONDS))
-                    for i in range(restutil.DEFAULT_RETRIES)],
-            _sleep.call_args_list)
+    def test_it_should_have_http_request_retries_with_linear_delay(self):
+
+        self.assertTrue(httpclient.BAD_GATEWAY in restutil.RETRY_CODES, "Ensure that the test params are correct")
+        retry_delay_in_sec = 0.05
+
+        for _ in range(3):
+            mock_resp = Mock(return_value=MockHttpResponse(status=httpclient.BAD_GATEWAY))
+            mock_conn = MagicMock(getresponse=mock_resp)
+            max_retry = randint(5, 10)
+            duration = None
+            with patch("azurelinuxagent.common.future.httpclient.HTTPConnection", return_value=mock_conn):
+                with self.assertRaises(HttpError):
+                    start_time = datetime.utcnow()
+                    restutil.http_get("http://foo.bar", retry_delay=retry_delay_in_sec, max_retry=max_retry)
+                duration = datetime.utcnow() - start_time
+
+            self.assertEqual(max_retry, mock_resp.call_count, "Did not Retry the required amount of times")
+            upper_bound = timedelta(seconds=retry_delay_in_sec * (max_retry + 2))
+            lower_bound = timedelta(seconds=retry_delay_in_sec * (max_retry - 2))
+            self.assertTrue(upper_bound >= duration >= lower_bound,
+                            "The total duration for request not in acceptable range. UB: {0}; LB: {1}; Actual: {2}".format(
+                                upper_bound, lower_bound, duration))
 
     @patch("time.sleep")
     @patch("azurelinuxagent.common.utils.restutil._http_request")