[tor-commits] [sbws/master] fix: destination: Set maximum to retry a destination

juga at torproject.org juga at torproject.org
Wed Feb 19 18:50:54 UTC 2020


commit 25bb4476f23c34bc25cf36950924d91549b651ff
Author: juga0 <juga at riseup.net>
Date:   Wed Feb 19 16:31:07 2020 +0000

    fix: destination: Set maximum to retry a destination
    
    otherwise the time to retry a destination could increase too much,
    as it's being multiplied by 2, and a destination would not recover.
    
    Patch submitted by tom.
    
    Closes: #33033.
---
 sbws/globals.py         |  3 +++
 sbws/lib/destination.py | 45 +++++++++++++++++++++++++++++++--------------
 2 files changed, 34 insertions(+), 14 deletions(-)

diff --git a/sbws/globals.py b/sbws/globals.py
index 1b34730..3eb1aa8 100644
--- a/sbws/globals.py
+++ b/sbws/globals.py
@@ -138,6 +138,9 @@ NUM_DESTINATION_ATTEMPTS_STORED = 10
 # Because intermitent failures with CDN destinations, start trying again
 # after 5 min.
 DELTA_SECONDS_RETRY_DESTINATION = 60 * 5
+# No matter what, do not increase the wait time between destination reties
+# past this value.
+MAX_SECONDS_BETWEEN_DESTINATION_RETRIES = 60 * 60 * 3
 # Number of consecutive times a destination can fail before considering it
 # not functional.
 MAX_NUM_DESTINATION_FAILURES = 3
diff --git a/sbws/lib/destination.py b/sbws/lib/destination.py
index 3ccd94f..a53b8ca 100644
--- a/sbws/lib/destination.py
+++ b/sbws/lib/destination.py
@@ -11,6 +11,7 @@ import sbws.util.stem as stem_utils
 from ..globals import (
     MAX_NUM_DESTINATION_FAILURES,
     DELTA_SECONDS_RETRY_DESTINATION,
+    MAX_SECONDS_BETWEEN_DESTINATION_RETRIES,
     NUM_DESTINATION_ATTEMPTS_STORED,
     FACTOR_INCREMENT_DESTINATION_RETRY
     )
@@ -121,7 +122,7 @@ def connect_to_destination_over_circuit(dest, circ_id, session, cont, max_dl):
             '{} not {}'.format(requests.codes.ok, head.status_code)
     if 'content-length' not in head.headers:
         dest.add_failure()
-        return False, error_prefix + 'we except the header Content-Length '\
+        return False, error_prefix + 'we expect the header Content-Length '\
             'to exist in the response'
     content_length = int(head.headers['content-length'])
     if max_dl > content_length:
@@ -143,6 +144,7 @@ class Destination:
     def __init__(self, url, max_dl, verify,
                  max_num_failures=MAX_NUM_DESTINATION_FAILURES,
                  delta_seconds_retry=DELTA_SECONDS_RETRY_DESTINATION,
+                 max_seconds_between_retries=MAX_SECONDS_BETWEEN_DESTINATION_RETRIES,
                  num_attempts_stored=NUM_DESTINATION_ATTEMPTS_STORED,
                  factor_increment_retry=FACTOR_INCREMENT_DESTINATION_RETRY):
         """Initalizes the Web server from which the data is downloaded.
@@ -169,6 +171,8 @@ class Destination:
         # Default delta time to try a destination that was not functional.
         self._default_delta_seconds_retry = delta_seconds_retry
         self._delta_seconds_retry = delta_seconds_retry
+        # A cap on the time to wait between destination retries.
+        self._max_seconds_between_retries = max_seconds_between_retries
         # Using a deque (FIFO) to do not grow forever and
         # to do not have to remove old attempts.
         # Store tuples of timestamp and whether the destination succed or not
@@ -201,20 +205,30 @@ class Destination:
         Increment the time a destination will be tried again by a ``factor``.
         """
         self._delta_seconds_retry *= factor or self._factor
-        log.info("Incremented the time to try destination %s to %s hours.",
-                 self.url, self._delta_seconds_retry / 60 / 60)
+        if self._delta_seconds_retry > self._max_seconds_between_retries:
+            self._delta_seconds_retry = self._max_seconds_between_retries
+            log.info("Incremented the time to try destination %s past the "
+                     "limit, capping it at %s hours.",
+                     self.url, self._delta_seconds_retry / 60 / 60)
+        else:
+            log.info("Incremented the time to try destination %s to %s hours.",
+                     self.url, self._delta_seconds_retry / 60 / 60)
 
-    def _is_last_try_old_enough(self, n=None):
+    def _get_last_try_in_seconds_ago(self):
         """
-        Return True if the last time it was used it was ``n`` seconds ago.
+        Return the delta between the last try and now, as positive seconds.
         """
         # Timestamp of the last attempt.
         last_time = self._attempts[-1][0]
+        return (datetime.datetime.utcnow() - last_time).total_seconds()
+
+    def _is_last_try_old_enough(self, n=None):
+        """
+        Return True if the last time it was used it was ``n`` seconds ago.
+        """
         # If the last attempt is older than _delta_seconds_retry, try again
-        return (datetime.datetime.utcnow()
-                - datetime.timedelta(seconds=self._delta_seconds_retry)
-                > last_time)
-        return False
+        return (self._get_last_try_in_seconds_ago() >
+                self._delta_seconds_retry)
 
     def is_functional(self):
         """Whether connections to a destination are failing or not.
@@ -238,18 +252,21 @@ class Destination:
         if self._are_last_attempts_failures():
             # The log here will appear in all the the queued
             #  relays and threads.
-            log.warning("The last %s times the destination %s failed."
-                        "Disabled for %s minutes.",
+            log.warning("The last %s times the destination %s failed. "
+                        "It last ran %s seconds ago. "
+                        "Disabled for %s seconds.",
                         self._max_num_failures, self.url,
-                        self._delta_seconds_retry / 60)
+                        self._get_last_try_in_seconds_ago(),
+                        self._delta_seconds_retry)
             log.warning("Please, add more destinations or increment the "
                         "number of maximum number of consecutive failures "
                         "in the configuration.")
             # It was not used for a while and the last time it was used
             # was long ago, then try again
             if self._is_last_try_old_enough():
-                log.info("The destination %s was not tried for %s hours, "
-                         "it is going to by tried again.")
+                log.info("The destination %s was not tried for %s seconds, "
+                         "it is going to by tried again.", self.url,
+                         self._get_last_try_in_seconds_ago())
                 # Set the next time to retry higher, in case this attempt fails
                 self._increment_time_to_retry()
                 return True





More information about the tor-commits mailing list