Repository: Azure/WALinuxAgent
Pull request #2065 (2020-11-05 00:32:32)

Revert "Fixed delays for HTTP retries rather than exponential delays (#1967)"
<!-- DO NOT DELETE THIS TEMPLATE -->

## Description

Revert #1967
This reverts commit 0991c3a.

<!--
Please add an informative description that covers that changes made by the pull request. 
This checklist is used to make sure that common issues in a pull request are addressed.
This will expedite the process of getting your pull request merged and avoid extra work on your part to fix issues discovered during the review process.
-->

---

### PR information
- [ ] The title of the PR is clear and informative.
- [ ] There are a small number of commits, each of which has an informative message. This means that previously merged commits do not appear in the history of the PR. For information on cleaning up the commits in your pull request, [see this page](https://github.com/Azure/azure-powershell/blob/master/documentation/development-docs/cleaning-up-commits.md).
- [ ] Except for special cases involving multiple contributors, the PR is started from a fork of the main repository, not a branch.
- [ ] If applicable, the PR references the bug/issue that it fixes in the description.
- [ ] New Unit tests were added for the changes made and Travis.CI is passing.

### Quality of Code and Contribution Guidelines
- [ ] I have read the [contribution guidelines](https://github.com/Azure/WALinuxAgent/blob/master/.github/CONTRIBUTING.md).


Patch:
diff --git a/azurelinuxagent/common/utils/restutil.py b/azurelinuxagent/common/utils/restutil.py
index e2378be9..286b1dda 100644
--- a/azurelinuxagent/common/utils/restutil.py
+++ b/azurelinuxagent/common/utils/restutil.py
@@ -35,8 +35,8 @@ from azurelinuxagent.common.version import PY_VERSION_MAJOR, AGENT_NAME, GOAL_ST
 
 SECURE_WARNING_EMITTED = False
 
-DEFAULT_RETRIES = 3
-DELAY_IN_SECONDS = 5
+DEFAULT_RETRIES = 6
+DELAY_IN_SECONDS = 1
 
 THROTTLE_RETRIES = 25
 THROTTLE_DELAY_IN_SECONDS = 1
@@ -135,6 +135,13 @@ class IOErrorCounter(object):
         IOErrorCounter._protocol_endpoint = endpoint
 
 
+def _compute_delay(retry_attempt=1, delay=DELAY_IN_SECONDS):
+    fib = (1, 1)
+    for _ in range(retry_attempt):
+        fib = (fib[1], fib[0]+fib[1])
+    return delay*fib[1]
+
+
 def _is_retry_status(status, retry_codes=RETRY_CODES): # pylint: disable=W0102
     return status in retry_codes
 
@@ -383,8 +390,12 @@ def http_request(method, # pylint: disable=R0913,R0912,W0102
             # 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, 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
+            # -- 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)
 
             logger.verbose("[HTTP Retry] "
                         "Attempt {0} of {1} will delay {2} seconds: {3}", 


Test patch:
diff --git a/tests/utils/test_rest_util.py b/tests/utils/test_rest_util.py
index d9c827b2..c0d1a4ec 100644
--- a/tests/utils/test_rest_util.py
+++ b/tests/utils/test_rest_util.py
@@ -17,14 +17,11 @@
 
 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
 
 
@@ -581,28 +578,27 @@ class TestHttpOperations(AgentTestCase): # pylint: disable=too-many-public-metho
         self.assertEqual(2, _http_request.call_count)
         self.assertEqual(1, _sleep.call_count)
 
-    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")
+    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))  # pylint: disable=protected-access
+                    for i in range(restutil.DEFAULT_RETRIES)],
+            _sleep.call_args_list)
 
     @patch("time.sleep")
     @patch("azurelinuxagent.common.utils.restutil._http_request")