[tor-commits] [tor/maint-0.3.3] Remove all the old max_delay logic.

nickm at torproject.org nickm at torproject.org
Sat Mar 3 16:54:23 UTC 2018


commit 5b55e15707fd49079b9c127b339976c4f446e131
Author: Nick Mathewson <nickm at torproject.org>
Date:   Thu Jan 25 16:05:20 2018 -0500

    Remove all the old max_delay logic.
    
    We had tests for it, but it was always INT_MAX.
---
 src/or/directory.c  | 53 +++++++++++++++++------------------------------------
 src/or/directory.h  | 10 ++++------
 src/test/test_dir.c | 27 ++++++++++-----------------
 3 files changed, 31 insertions(+), 59 deletions(-)

diff --git a/src/or/directory.c b/src/or/directory.c
index 3d5321157..3dbed28e7 100644
--- a/src/or/directory.c
+++ b/src/or/directory.c
@@ -5362,17 +5362,14 @@ find_dl_schedule(const download_status_t *dls, const or_options_t *options)
   return NULL;
 }
 
-/** Decide which minimum and maximum delay step we want to use based on
+/** Decide which minimum delay step we want to use based on
  * descriptor type in <b>dls</b> and <b>options</b>.
  * Helper function for download_status_schedule_get_delay(). */
-STATIC void
-find_dl_min_and_max_delay(download_status_t *dls, const or_options_t *options,
-                          int *min, int *max)
+STATIC int
+find_dl_min_delay(download_status_t *dls, const or_options_t *options)
 {
   tor_assert(dls);
   tor_assert(options);
-  tor_assert(min);
-  tor_assert(max);
 
   /*
    * For now, just use the existing schedule config stuff and pick the
@@ -5380,10 +5377,7 @@ find_dl_min_and_max_delay(download_status_t *dls, const or_options_t *options,
    */
   const smartlist_t *schedule = find_dl_schedule(dls, options);
   tor_assert(schedule != NULL && smartlist_len(schedule) >= 2);
-  *min = *((int *)(smartlist_get(schedule, 0)));
-  /* Increment on failure schedules always use exponential backoff, but they
-   * have a smaller limit when they're deterministic */
-  *max = INT_MAX;
+  return *(int *)(smartlist_get(schedule, 0));
 }
 
 /** As next_random_exponential_delay() below, but does not compute a random
@@ -5421,49 +5415,39 @@ next_random_exponential_delay_range(int *low_bound_out,
  * zero); the <b>backoff_position</b> parameter is the number of times we've
  * generated a delay; and the <b>delay</b> argument is the most recently used
  * delay.
- *
- * Requires that delay is less than INT_MAX, and delay is in [0,max_delay].
  */
 STATIC int
 next_random_exponential_delay(int delay,
-                              int base_delay,
-                              int max_delay)
+                              int base_delay)
 {
   /* Check preconditions */
-  if (BUG(max_delay < 0))
-    max_delay = 0;
-  if (BUG(delay > max_delay))
-    delay = max_delay;
   if (BUG(delay < 0))
     delay = 0;
 
   if (base_delay < 1)
     base_delay = 1;
 
-  int low_bound=0, high_bound=max_delay;
+  int low_bound=0, high_bound=INT_MAX;
 
   next_random_exponential_delay_range(&low_bound, &high_bound,
                                       delay, base_delay);
 
-  int rand_delay = crypto_rand_int_range(low_bound, high_bound);
-
-  return MIN(rand_delay, max_delay);
+  return crypto_rand_int_range(low_bound, high_bound);
 }
 
-/** Find the current delay for dls based on min_delay/
- * max_delay. Requires that 0 <= min_delay <= max_delay <= INT_MAX.
+/** Find the current delay for dls based on min_delay.
  *
  * This function sets dls->next_attempt_at based on now, and returns the delay.
  * Helper for download_status_increment_failure and
  * download_status_increment_attempt. */
 STATIC int
 download_status_schedule_get_delay(download_status_t *dls,
-                                   int min_delay, int max_delay,
+                                   int min_delay,
                                    time_t now)
 {
   tor_assert(dls);
   /* If we're using random exponential backoff, we do need min/max delay */
-  tor_assert(min_delay >= 0 && max_delay >= min_delay);
+  tor_assert(min_delay >= 0);
 
   int delay = INT_MAX;
   uint8_t dls_schedule_position = (dls->increment_on
@@ -5482,7 +5466,7 @@ download_status_schedule_get_delay(download_status_t *dls,
 
     while (dls->last_backoff_position < dls_schedule_position) {
       /* Do one increment step */
-      delay = next_random_exponential_delay(delay, min_delay, max_delay);
+      delay = next_random_exponential_delay(delay, min_delay);
       /* Update our position */
       ++(dls->last_backoff_position);
     }
@@ -5493,7 +5477,6 @@ download_status_schedule_get_delay(download_status_t *dls,
 
   /* Clamp it within min/max if we have them */
   if (min_delay >= 0 && delay < min_delay) delay = min_delay;
-  if (max_delay != INT_MAX && delay > max_delay) delay = max_delay;
 
   /* Store it for next time */
   dls->last_backoff_position = dls_schedule_position;
@@ -5560,7 +5543,7 @@ download_status_increment_failure(download_status_t *dls, int status_code,
   (void) status_code; // XXXX no longer used.
   (void) server; // XXXX no longer used.
   int increment = -1;
-  int min_delay = 0, max_delay = INT_MAX;
+  int min_delay = 0;
 
   tor_assert(dls);
 
@@ -5585,9 +5568,8 @@ download_status_increment_failure(download_status_t *dls, int status_code,
 
     /* only return a failure retry time if this schedule increments on failures
      */
-    find_dl_min_and_max_delay(dls, get_options(), &min_delay, &max_delay);
-    increment = download_status_schedule_get_delay(dls,
-                                                   min_delay, max_delay, now);
+    min_delay = find_dl_min_delay(dls, get_options());
+    increment = download_status_schedule_get_delay(dls, min_delay, now);
   }
 
   download_status_log_helper(item, !dls->increment_on, "failed",
@@ -5618,7 +5600,7 @@ download_status_increment_attempt(download_status_t *dls, const char *item,
                                   time_t now)
 {
   int delay = -1;
-  int min_delay = 0, max_delay = INT_MAX;
+  int min_delay = 0;
 
   tor_assert(dls);
 
@@ -5638,9 +5620,8 @@ download_status_increment_attempt(download_status_t *dls, const char *item,
   if (dls->n_download_attempts < IMPOSSIBLE_TO_DOWNLOAD-1)
     ++dls->n_download_attempts;
 
-  find_dl_min_and_max_delay(dls, get_options(), &min_delay, &max_delay);
-  delay = download_status_schedule_get_delay(dls,
-                                             min_delay, max_delay, now);
+  min_delay = find_dl_min_delay(dls, get_options());
+  delay = download_status_schedule_get_delay(dls, min_delay, now);
 
   download_status_log_helper(item, dls->increment_on, "attempted",
                              "on failure", dls->n_download_attempts,
diff --git a/src/or/directory.h b/src/or/directory.h
index 6008d3a88..e223d51ae 100644
--- a/src/or/directory.h
+++ b/src/or/directory.h
@@ -254,7 +254,7 @@ MOCK_DECL(STATIC int, directory_handle_command_post,(dir_connection_t *conn,
                                                      const char *body,
                                                      size_t body_len));
 STATIC int download_status_schedule_get_delay(download_status_t *dls,
-                                              int min_delay, int max_delay,
+                                              int min_delay,
                                               time_t now);
 
 STATIC int handle_post_hs_descriptor(const char *url, const char *body);
@@ -265,13 +265,11 @@ STATIC int should_use_directory_guards(const or_options_t *options);
 STATIC compression_level_t choose_compression_level(ssize_t n_bytes);
 STATIC const smartlist_t *find_dl_schedule(const download_status_t *dls,
                                            const or_options_t *options);
-STATIC void find_dl_min_and_max_delay(download_status_t *dls,
-                                      const or_options_t *options,
-                                      int *min, int *max);
+STATIC int find_dl_min_delay(download_status_t *dls,
+                             const or_options_t *options);
 
 STATIC int next_random_exponential_delay(int delay,
-                                         int base_delay,
-                                         int max_delay);
+                                         int base_delay);
 
 STATIC void next_random_exponential_delay_range(int *low_bound_out,
                                                 int *high_bound_out,
diff --git a/src/test/test_dir.c b/src/test/test_dir.c
index dc8d2a3bc..929ccd6c5 100644
--- a/src/test/test_dir.c
+++ b/src/test/test_dir.c
@@ -3965,7 +3965,7 @@ test_dir_packages(void *arg)
 }
 
 static void
-download_status_random_backoff_helper(int min_delay, int max_delay)
+download_status_random_backoff_helper(int min_delay)
 {
   download_status_t dls_random =
     { 0, 0, 0, DL_SCHED_GENERIC, DL_WANT_AUTHORITY,
@@ -3978,22 +3978,21 @@ download_status_random_backoff_helper(int min_delay, int max_delay)
   int n_attempts = 0;
   do {
     increment = download_status_schedule_get_delay(&dls_random,
-                                                   min_delay, max_delay,
+                                                   min_delay,
                                                    current_time);
 
-    log_debug(LD_DIR, "Min: %d, Max: %d, Inc: %d, Old Inc: %d",
-              min_delay, max_delay, increment, old_increment);
+    log_debug(LD_DIR, "Min: %d, Inc: %d, Old Inc: %d",
+              min_delay, increment, old_increment);
 
     /* Regression test for 20534 and friends
      * increment must always increase after the first */
-    if (dls_random.last_backoff_position > 0 && max_delay > 0) {
+    if (dls_random.last_backoff_position > 0) {
       /* Always increment the exponential backoff */
       tt_int_op(increment, OP_GE, 1);
     }
 
     /* Test */
     tt_int_op(increment, OP_GE, min_delay);
-    tt_int_op(increment, OP_LE, max_delay);
 
     /* Advance */
     if (dls_random.n_download_attempts < IMPOSSIBLE_TO_DOWNLOAD - 1) {
@@ -4003,7 +4002,7 @@ download_status_random_backoff_helper(int min_delay, int max_delay)
 
     /* Try another maybe */
     old_increment = increment;
-  } while (increment < max_delay && ++n_attempts < 1000);
+  } while (++n_attempts < 1000);
 
  done:
   return;
@@ -4015,19 +4014,13 @@ test_dir_download_status_random_backoff(void *arg)
   (void)arg;
 
   /* Do a standard test */
-  download_status_random_backoff_helper(0, 1000000);
-  /* Regression test for 20534 and friends:
-   * try tighter bounds */
-  download_status_random_backoff_helper(0, 100);
+  download_status_random_backoff_helper(0);
   /* regression tests for 17750: initial delay */
-  download_status_random_backoff_helper(10, 1000);
-  download_status_random_backoff_helper(20, 30);
+  download_status_random_backoff_helper(10);
+  download_status_random_backoff_helper(20);
 
   /* Pathological cases */
-  download_status_random_backoff_helper(0, 0);
-  download_status_random_backoff_helper(1, 1);
-  download_status_random_backoff_helper(0, INT_MAX);
-  download_status_random_backoff_helper(INT_MAX/2, INT_MAX);
+  download_status_random_backoff_helper(INT_MAX/2);
 }
 
 static void





More information about the tor-commits mailing list