Merge branch 'maint-0.3.3' into release-0.3.3

changes/refactor23814

@@ -0,0 +1,4 @@
+  o Code simplification and refactoring:
+    - Remove the old (deterministic) directory retry logic entirely:
+      We've used exponential backoff exclusively for some time.
+      Closes ticket 23814.

doc/tor.1.txt

@@ -1757,14 +1757,6 @@ The following options are useful only for clients (that is, if
     which are advanced by connection failures. (Default: 0, 3, 7, 3600,
     10800, 25200, 54000, 111600, 262800)
 
-[[ClientBootstrapConsensusMaxDownloadTries]] **ClientBootstrapConsensusMaxDownloadTries** __NUM__::
-    Try this many times to download a consensus while bootstrapping using
-    fallback directory mirrors before giving up. (Default: 7)
-
-[[ClientBootstrapConsensusAuthorityOnlyMaxDownloadTries]] **ClientBootstrapConsensusAuthorityOnlyMaxDownloadTries** __NUM__::
-    Try this many times to download a consensus while bootstrapping using
-    authorities before giving up. (Default: 4)
-
 [[ClientBootstrapConsensusMaxInProgressTries]] **ClientBootstrapConsensusMaxInProgressTries** __NUM__::
     Try this many simultaneous connections to download a consensus before
     waiting for one to complete, timeout, or error out. (Default: 3)
@@ -2871,8 +2863,6 @@ The following options are used for running a testing Tor network.
           4 (for 40 seconds), 8, 16, 32, 60
        ClientBootstrapConsensusAuthorityOnlyDownloadSchedule 0, 1,
           4 (for 40 seconds), 8, 16, 32, 60
-       ClientBootstrapConsensusMaxDownloadTries 80
-       ClientBootstrapConsensusAuthorityOnlyMaxDownloadTries 80
        ClientDNSRejectInternalAddresses 0
        ClientRejectInternalAddresses 0
        CountPrivateBandwidth 1
@@ -2895,10 +2885,6 @@ The following options are used for running a testing Tor network.
        TestingBridgeBootstrapDownloadSchedule 0, 0, 5, 10, 15, 20, 30, 60
        TestingClientMaxIntervalWithoutRequest 5 seconds
        TestingDirConnectionMaxStall 30 seconds
-       TestingConsensusMaxDownloadTries 80
-       TestingDescriptorMaxDownloadTries 80
-       TestingMicrodescMaxDownloadTries 80
-       TestingCertMaxDownloadTries 80
        TestingEnableConnBwEvent 1
        TestingEnableCellStatsEvent 1
        TestingEnableTbEmptyEvent 1
@@ -2979,22 +2965,6 @@ The following options are used for running a testing Tor network.
     Changing this requires that **TestingTorNetwork** is set. (Default:
     5 minutes)
 
-[[TestingConsensusMaxDownloadTries]] **TestingConsensusMaxDownloadTries** __NUM__::
-    Try this many times to download a consensus before giving up. Changing
-    this requires that **TestingTorNetwork** is set. (Default: 8)
-
-[[TestingDescriptorMaxDownloadTries]] **TestingDescriptorMaxDownloadTries** __NUM__::
-    Try this often to download a server descriptor before giving up.
-    Changing this requires that **TestingTorNetwork** is set. (Default: 8)
-
-[[TestingMicrodescMaxDownloadTries]] **TestingMicrodescMaxDownloadTries** __NUM__::
-    Try this often to download a microdesc descriptor before giving up.
-    Changing this requires that **TestingTorNetwork** is set. (Default: 8)
-
-[[TestingCertMaxDownloadTries]] **TestingCertMaxDownloadTries** __NUM__::
-    Try this often to download a v3 authority certificate before giving up.
-    Changing this requires that **TestingTorNetwork** is set. (Default: 8)
-
 [[TestingDirAuthVoteExit]] **TestingDirAuthVoteExit** __node__,__node__,__...__::
     A list of identity fingerprints, country codes, and
     address patterns of nodes to vote Exit for regardless of their

src/or/bridges.c

@@ -458,7 +458,6 @@ bridge_add_from_config(bridge_line_t *bridge_line)
   if (bridge_line->transport_name)
     b->transport_name = bridge_line->transport_name;
   b->fetch_status.schedule = DL_SCHED_BRIDGE;
-  b->fetch_status.backoff = DL_SCHED_RANDOM_EXPONENTIAL;
   b->fetch_status.increment_on = DL_SCHED_INCREMENT_ATTEMPT;
   /* We can't reset the bridge's download status here, because UseBridges
    * might be 0 now, and it might be changed to 1 much later. */
@@ -637,8 +636,7 @@ fetch_bridge_descriptors(const or_options_t *options, time_t now)
   SMARTLIST_FOREACH_BEGIN(bridge_list, bridge_info_t *, bridge)
     {
       /* This resets the download status on first use */
-      if (!download_status_is_ready(&bridge->fetch_status, now,
+      if (!download_status_is_ready(&bridge->fetch_status, now))
-                                    IMPOSSIBLE_TO_DOWNLOAD))
         continue; /* don't bother, no need to retry yet */
       if (routerset_contains_bridge(options->ExcludeNodes, bridge)) {
         download_status_mark_impossible(&bridge->fetch_status);

src/or/config.c

@@ -645,15 +645,12 @@ static config_var_t option_vars_[] = {
     "0, 30, 90, 600, 3600, 10800, 25200, 54000, 111600, 262800"),
   V(TestingClientMaxIntervalWithoutRequest, INTERVAL, "10 minutes"),
   V(TestingDirConnectionMaxStall, INTERVAL, "5 minutes"),
-  V(TestingConsensusMaxDownloadTries, UINT, "8"),
+  OBSOLETE("TestingConsensusMaxDownloadTries"),
-  /* Since we try connections rapidly and simultaneously, we can afford
+  OBSOLETE("ClientBootstrapConsensusMaxDownloadTries"),
-   * to give up earlier. (This protects against overloading directories.) */
+  OBSOLETE("ClientBootstrapConsensusAuthorityOnlyMaxDownloadTries"),
-  V(ClientBootstrapConsensusMaxDownloadTries, UINT, "7"),
+  OBSOLETE("TestingDescriptorMaxDownloadTries"),
-  /* We want to give up much earlier if we're only using authorities. */
+  OBSOLETE("TestingMicrodescMaxDownloadTries"),
-  V(ClientBootstrapConsensusAuthorityOnlyMaxDownloadTries, UINT, "4"),
+  OBSOLETE("TestingCertMaxDownloadTries"),
-  V(TestingDescriptorMaxDownloadTries, UINT, "8"),
-  V(TestingMicrodescMaxDownloadTries, UINT, "8"),
-  V(TestingCertMaxDownloadTries, UINT, "8"),
   V(TestingDirAuthVoteExit, ROUTERSET, NULL),
   V(TestingDirAuthVoteExitIsStrict,  BOOL,     "0"),
   V(TestingDirAuthVoteGuard, ROUTERSET, NULL),
@@ -678,8 +675,6 @@ static const config_var_t testing_tor_network_defaults[] = {
     "0, 1, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 8, 16, 32, 60"),
   V(ClientBootstrapConsensusAuthorityOnlyDownloadSchedule, CSV_INTERVAL,
     "0, 1, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 8, 16, 32, 60"),
-  V(ClientBootstrapConsensusMaxDownloadTries, UINT, "80"),
-  V(ClientBootstrapConsensusAuthorityOnlyMaxDownloadTries, UINT, "80"),
   V(ClientDNSRejectInternalAddresses, BOOL,"0"),
   V(ClientRejectInternalAddresses, BOOL,   "0"),
   V(CountPrivateBandwidth,       BOOL,     "1"),
@@ -707,10 +702,6 @@ static const config_var_t testing_tor_network_defaults[] = {
                                  "15, 20, 30, 60"),
   V(TestingClientMaxIntervalWithoutRequest, INTERVAL, "5 seconds"),
   V(TestingDirConnectionMaxStall, INTERVAL, "30 seconds"),
-  V(TestingConsensusMaxDownloadTries, UINT, "80"),
-  V(TestingDescriptorMaxDownloadTries, UINT, "80"),
-  V(TestingMicrodescMaxDownloadTries, UINT, "80"),
-  V(TestingCertMaxDownloadTries, UINT, "80"),
   V(TestingEnableConnBwEvent,    BOOL,     "1"),
   V(TestingEnableCellStatsEvent, BOOL,     "1"),
   V(TestingEnableTbEmptyEvent,   BOOL,     "1"),
@@ -4418,10 +4409,6 @@ options_validate(or_options_t *old_options, or_options_t *options,
   CHECK_DEFAULT(TestingBridgeBootstrapDownloadSchedule);
   CHECK_DEFAULT(TestingClientMaxIntervalWithoutRequest);
   CHECK_DEFAULT(TestingDirConnectionMaxStall);
-  CHECK_DEFAULT(TestingConsensusMaxDownloadTries);
-  CHECK_DEFAULT(TestingDescriptorMaxDownloadTries);
-  CHECK_DEFAULT(TestingMicrodescMaxDownloadTries);
-  CHECK_DEFAULT(TestingCertMaxDownloadTries);
   CHECK_DEFAULT(TestingAuthKeyLifetime);
   CHECK_DEFAULT(TestingLinkCertLifetime);
   CHECK_DEFAULT(TestingSigningKeySlop);
@@ -4496,33 +4483,6 @@ options_validate(or_options_t *old_options, or_options_t *options,
     COMPLAIN("TestingDirConnectionMaxStall is insanely high.");
   }
 
-  if (options->TestingConsensusMaxDownloadTries < 2) {
-    REJECT("TestingConsensusMaxDownloadTries must be greater than 2.");
-  } else if (options->TestingConsensusMaxDownloadTries > 800) {
-    COMPLAIN("TestingConsensusMaxDownloadTries is insanely high.");
-  }
-
-  if (options->ClientBootstrapConsensusMaxDownloadTries < 2) {
-    REJECT("ClientBootstrapConsensusMaxDownloadTries must be greater "
-           "than 2."
-           );
-  } else if (options->ClientBootstrapConsensusMaxDownloadTries > 800) {
-    COMPLAIN("ClientBootstrapConsensusMaxDownloadTries is insanely "
-             "high.");
-  }
-
-  if (options->ClientBootstrapConsensusAuthorityOnlyMaxDownloadTries
-      < 2) {
-    REJECT("ClientBootstrapConsensusAuthorityOnlyMaxDownloadTries must "
-           "be greater than 2."
-           );
-  } else if (
-        options->ClientBootstrapConsensusAuthorityOnlyMaxDownloadTries
-        > 800) {
-    COMPLAIN("ClientBootstrapConsensusAuthorityOnlyMaxDownloadTries is "
-             "insanely high.");
-  }
-
   if (options->ClientBootstrapConsensusMaxInProgressTries < 1) {
     REJECT("ClientBootstrapConsensusMaxInProgressTries must be greater "
            "than 0.");
@@ -4532,24 +4492,6 @@ options_validate(or_options_t *old_options, or_options_t *options,
              "high.");
   }
 
-  if (options->TestingDescriptorMaxDownloadTries < 2) {
-    REJECT("TestingDescriptorMaxDownloadTries must be greater than 1.");
-  } else if (options->TestingDescriptorMaxDownloadTries > 800) {
-    COMPLAIN("TestingDescriptorMaxDownloadTries is insanely high.");
-  }
-
-  if (options->TestingMicrodescMaxDownloadTries < 2) {
-    REJECT("TestingMicrodescMaxDownloadTries must be greater than 1.");
-  } else if (options->TestingMicrodescMaxDownloadTries > 800) {
-    COMPLAIN("TestingMicrodescMaxDownloadTries is insanely high.");
-  }
-
-  if (options->TestingCertMaxDownloadTries < 2) {
-    REJECT("TestingCertMaxDownloadTries must be greater than 1.");
-  } else if (options->TestingCertMaxDownloadTries > 800) {
-    COMPLAIN("TestingCertMaxDownloadTries is insanely high.");
-  }
-
   if (options->TestingEnableConnBwEvent &&
       !options->TestingTorNetwork && !options->UsingTestNetworkDefaults_) {
     REJECT("TestingEnableConnBwEvent may only be changed in testing "

src/or/control.c

@@ -2252,7 +2252,7 @@ digest_list_to_string(const smartlist_t *sl)
 static char *
 download_status_to_string(const download_status_t *dl)
 {
-  char *rv = NULL, *tmp;
+  char *rv = NULL;
   char tbuf[ISO_TIME_LEN+1];
   const char *schedule_str, *want_authority_str;
   const char *increment_on_str, *backoff_str;
@@ -2300,49 +2300,28 @@ download_status_to_string(const download_status_t *dl)
         break;
     }
 
-    switch (dl->backoff) {
+    backoff_str = "DL_SCHED_RANDOM_EXPONENTIAL";
-      case DL_SCHED_DETERMINISTIC:
-        backoff_str = "DL_SCHED_DETERMINISTIC";
-        break;
-      case DL_SCHED_RANDOM_EXPONENTIAL:
-        backoff_str = "DL_SCHED_RANDOM_EXPONENTIAL";
-        break;
-      default:
-        backoff_str = "unknown";
-        break;
-    }
 
     /* Now assemble them */
-    tor_asprintf(&tmp,
+    tor_asprintf(&rv,
                  "next-attempt-at %s\n"
                  "n-download-failures %u\n"
                  "n-download-attempts %u\n"
                  "schedule %s\n"
                  "want-authority %s\n"
                  "increment-on %s\n"
-                 "backoff %s\n",
+                 "backoff %s\n"
+                 "last-backoff-position %u\n"
+                 "last-delay-used %d\n",
                  tbuf,
                  dl->n_download_failures,
                  dl->n_download_attempts,
                  schedule_str,
                  want_authority_str,
                  increment_on_str,
-                 backoff_str);
+                 backoff_str,
-
+                 dl->last_backoff_position,
-    if (dl->backoff == DL_SCHED_RANDOM_EXPONENTIAL) {
+                 dl->last_delay_used);
-      /* Additional fields become relevant in random-exponential mode */
-      tor_asprintf(&rv,
-                   "%s"
-                   "last-backoff-position %u\n"
-                   "last-delay-used %d\n",
-                   tmp,
-                   dl->last_backoff_position,
-                   dl->last_delay_used);
-      tor_free(tmp);
-    } else {
-      /* That was it */
-      rv = tmp;
-    }
   }
 
   return rv;

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
+STATIC int
-find_dl_min_and_max_delay(download_status_t *dls, const or_options_t *options,
+find_dl_min_delay(download_status_t *dls, const or_options_t *options)
-                          int *min, int *max)
 {
   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,13 +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)));
+  return *(int *)(smartlist_get(schedule, 0));
-  /* Increment on failure schedules always use exponential backoff, but they
-   * have a smaller limit when they're deterministic */
-  if (dls->backoff == DL_SCHED_DETERMINISTIC)
-    *max = *((int *)((smartlist_get(schedule, smartlist_len(schedule) - 1))));
-  else
-    *max = INT_MAX;
 }
 
 /** As next_random_exponential_delay() below, but does not compute a random
@@ -5424,55 +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 base_delay)
-                              int max_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 crypto_rand_int_range(low_bound, high_bound);
-
-  return MIN(rand_delay, max_delay);
 }
 
-/** Find the current delay for dls based on schedule or min_delay/
+/** Find the current delay for dls based on min_delay.
- * max_delay if we're using exponential backoff.  If dls->backoff is
+ *
- * DL_SCHED_RANDOM_EXPONENTIAL, we must have 0 <= min_delay <= max_delay <=
- * INT_MAX, but schedule may be set to NULL; otherwise schedule is required.
  * 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,
-                                   const smartlist_t *schedule,
+                                   int min_delay,
-                                   int min_delay, int max_delay,
                                    time_t now)
 {
   tor_assert(dls);
-  /* We don't need a schedule if we're using random exponential backoff */
-  tor_assert(dls->backoff == DL_SCHED_RANDOM_EXPONENTIAL ||
-             schedule != NULL);
   /* If we're using random exponential backoff, we do need min/max delay */
-  tor_assert(dls->backoff != DL_SCHED_RANDOM_EXPONENTIAL ||
+  tor_assert(min_delay >= 0);
-             (min_delay >= 0 && max_delay >= min_delay));
 
   int delay = INT_MAX;
   uint8_t dls_schedule_position = (dls->increment_on
@@ -5480,43 +5455,33 @@ download_status_schedule_get_delay(download_status_t *dls,
                                    ? dls->n_download_attempts
                                    : dls->n_download_failures);
 
-  if (dls->backoff == DL_SCHED_DETERMINISTIC) {
+  /* Check if we missed a reset somehow */
-    if (dls_schedule_position < smartlist_len(schedule))
+  IF_BUG_ONCE(dls->last_backoff_position > dls_schedule_position) {
-      delay = *(int *)smartlist_get(schedule, dls_schedule_position);
+    dls->last_backoff_position = 0;
-    else if (dls_schedule_position == IMPOSSIBLE_TO_DOWNLOAD)
+    dls->last_delay_used = 0;
-      delay = INT_MAX;
-    else
-      delay = *(int *)smartlist_get(schedule, smartlist_len(schedule) - 1);
-  } else if (dls->backoff == DL_SCHED_RANDOM_EXPONENTIAL) {
-    /* Check if we missed a reset somehow */
-    IF_BUG_ONCE(dls->last_backoff_position > dls_schedule_position) {
-      dls->last_backoff_position = 0;
-      dls->last_delay_used = 0;
-    }
-
-    if (dls_schedule_position > 0) {
-      delay = dls->last_delay_used;
-
-      while (dls->last_backoff_position < dls_schedule_position) {
-        /* Do one increment step */
-        delay = next_random_exponential_delay(delay, min_delay, max_delay);
-        /* Update our position */
-        ++(dls->last_backoff_position);
-      }
-    } else {
-      /* If we're just starting out, use the minimum delay */
-      delay = min_delay;
-    }
-
-    /* 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;
-    dls->last_delay_used = delay;
   }
 
+  if (dls_schedule_position > 0) {
+    delay = dls->last_delay_used;
+
+    while (dls->last_backoff_position < dls_schedule_position) {
+      /* Do one increment step */
+      delay = next_random_exponential_delay(delay, min_delay);
+      /* Update our position */
+      ++(dls->last_backoff_position);
+    }
+  } else {
+    /* If we're just starting out, use the minimum delay */
+    delay = min_delay;
+  }
+
+  /* Clamp it within min/max if we have them */
+  if (min_delay >= 0 && delay < min_delay) delay = min_delay;
+
+  /* Store it for next time */
+  dls->last_backoff_position = dls_schedule_position;
+  dls->last_delay_used = delay;
+
   /* A negative delay makes no sense. Knowing that delay is
    * non-negative allows us to safely do the wrapping check below. */
   tor_assert(delay >= 0);
@@ -5578,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);
 
@@ -5603,10 +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
      */
-    const smartlist_t *schedule = find_dl_schedule(dls, get_options());
+    min_delay = find_dl_min_delay(dls, get_options());
-    find_dl_min_and_max_delay(dls, get_options(), &min_delay, &max_delay);
+    increment = download_status_schedule_get_delay(dls, min_delay, now);
-    increment = download_status_schedule_get_delay(dls, schedule,
-                                                   min_delay, max_delay, now);
   }
 
   download_status_log_helper(item, !dls->increment_on, "failed",
@@ -5637,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);
 
@@ -5657,10 +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;
 
-  const smartlist_t *schedule = find_dl_schedule(dls, get_options());
+  min_delay = find_dl_min_delay(dls, get_options());
-  find_dl_min_and_max_delay(dls, get_options(), &min_delay, &max_delay);
+  delay = download_status_schedule_get_delay(dls, min_delay, now);
-  delay = download_status_schedule_get_delay(dls, schedule,
-                                             min_delay, max_delay, now);
 
   download_status_log_helper(item, dls->increment_on, "attempted",
                              "on failure", dls->n_download_attempts,
@@ -5769,8 +5730,7 @@ dir_routerdesc_download_failed(smartlist_t *failed, int status_code,
     } else {
       dls = router_get_dl_status_by_descriptor_digest(digest);
     }
-    if (!dls || dls->n_download_failures >=
+    if (!dls)
-                get_options()->TestingDescriptorMaxDownloadTries)
       continue;
     download_status_increment_failure(dls, status_code, cp, server, now);
   } SMARTLIST_FOREACH_END(cp);
@@ -5807,10 +5767,6 @@ dir_microdesc_download_failed(smartlist_t *failed,
     if (!rs)
       continue;
     dls = &rs->dl_status;
-    if (dls->n_download_failures >=
-        get_options()->TestingMicrodescMaxDownloadTries) {
-      continue;
-    }
 
     { /* Increment the failure count for this md fetch */
       char buf[BASE64_DIGEST256_LEN+1];

src/or/directory.h

@@ -132,29 +132,19 @@ time_t download_status_increment_attempt(download_status_t *dls,
                                     time(NULL))
 
 void download_status_reset(download_status_t *dls);
-static int download_status_is_ready(download_status_t *dls, time_t now,
+static int download_status_is_ready(download_status_t *dls, time_t now);
-                                    int max_failures);
 time_t download_status_get_next_attempt_at(const download_status_t *dls);
 
 /** Return true iff, as of <b>now</b>, the resource tracked by <b>dls</b> is
  * ready to get its download reattempted. */
 static inline int
-download_status_is_ready(download_status_t *dls, time_t now,
+download_status_is_ready(download_status_t *dls, time_t now)
-                         int max_failures)
 {
   /* dls wasn't reset before it was used */
   if (dls->next_attempt_at == 0) {
     download_status_reset(dls);
   }
 
-  if (dls->backoff == DL_SCHED_DETERMINISTIC) {
-    /* Deterministic schedules can hit an endpoint; exponential backoff
-     * schedules just wait longer and longer. */
-    int under_failure_limit = (dls->n_download_failures <= max_failures
-                               && dls->n_download_attempts <= max_failures);
-    if (!under_failure_limit)
-      return 0;
-  }
   return download_status_get_next_attempt_at(dls) <= now;
 }
 
@@ -260,8 +250,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,
-                                              const smartlist_t *schedule,
+                                              int min_delay,
-                                              int min_delay, int max_delay,
                                               time_t now);
 
 STATIC int handle_post_hs_descriptor(const char *url, const char *body);
@@ -272,13 +261,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,
+STATIC int find_dl_min_delay(download_status_t *dls,
-                                      const or_options_t *options,
+                             const or_options_t *options);
-                                      int *min, int *max);
 
 STATIC int next_random_exponential_delay(int delay,
-                                         int base_delay,
+                                         int base_delay);
-                                         int max_delay);
 
 STATIC void next_random_exponential_delay_range(int *low_bound_out,
                                                 int *high_bound_out,

src/or/microdesc.c

@@ -920,8 +920,7 @@ microdesc_list_missing_digest256(networkstatus_t *ns, microdesc_cache_t *cache,
     if (microdesc_cache_lookup_by_digest256(cache, rs->descriptor_digest))
       continue;
     if (downloadable_only &&
-        !download_status_is_ready(&rs->dl_status, now,
+        !download_status_is_ready(&rs->dl_status, now))
-                  get_options()->TestingMicrodescMaxDownloadTries))
       continue;
     if (skip && digest256map_get(skip, (const uint8_t*)rs->descriptor_digest))
       continue;

src/or/networkstatus.c

@@ -107,9 +107,9 @@ static time_t time_to_download_next_consensus[N_CONSENSUS_FLAVORS];
 static download_status_t consensus_dl_status[N_CONSENSUS_FLAVORS] =
   {
     { 0, 0, 0, DL_SCHED_CONSENSUS, DL_WANT_ANY_DIRSERVER,
-      DL_SCHED_INCREMENT_FAILURE, DL_SCHED_RANDOM_EXPONENTIAL, 0, 0 },
+      DL_SCHED_INCREMENT_FAILURE, 0, 0 },
     { 0, 0, 0, DL_SCHED_CONSENSUS, DL_WANT_ANY_DIRSERVER,
-      DL_SCHED_INCREMENT_FAILURE, DL_SCHED_RANDOM_EXPONENTIAL, 0, 0 },
+      DL_SCHED_INCREMENT_FAILURE, 0, 0 },
   };
 
 #define N_CONSENSUS_BOOTSTRAP_SCHEDULES 2
@@ -126,10 +126,10 @@ static download_status_t
               consensus_bootstrap_dl_status[N_CONSENSUS_BOOTSTRAP_SCHEDULES] =
   {
     { 0, 0, 0, DL_SCHED_CONSENSUS, DL_WANT_AUTHORITY,
-      DL_SCHED_INCREMENT_ATTEMPT, DL_SCHED_RANDOM_EXPONENTIAL, 0, 0 },
+      DL_SCHED_INCREMENT_ATTEMPT, 0, 0 },
     /* During bootstrap, DL_WANT_ANY_DIRSERVER means "use fallbacks". */
     { 0, 0, 0, DL_SCHED_CONSENSUS, DL_WANT_ANY_DIRSERVER,
-      DL_SCHED_INCREMENT_ATTEMPT, DL_SCHED_RANDOM_EXPONENTIAL, 0, 0 },
+      DL_SCHED_INCREMENT_ATTEMPT, 0, 0 },
   };
 
 /** True iff we have logged a warning about this OR's version being older than
@@ -943,14 +943,11 @@ update_consensus_networkstatus_downloads(time_t now)
       update_consensus_bootstrap_multiple_downloads(now, options);
     } else {
       /* Check if we failed downloading a consensus too recently */
-      int max_dl_tries = options->TestingConsensusMaxDownloadTries;
 
       /* Let's make sure we remembered to update consensus_dl_status */
       tor_assert(consensus_dl_status[i].schedule == DL_SCHED_CONSENSUS);
 
-      if (!download_status_is_ready(&consensus_dl_status[i],
+      if (!download_status_is_ready(&consensus_dl_status[i], now)) {
-                                    now,
-                                    max_dl_tries)) {
         continue;
       }
 
@@ -977,17 +974,9 @@ update_consensus_networkstatus_downloads(time_t now)
 static void
 update_consensus_bootstrap_attempt_downloads(
                                       time_t now,
-                                      const or_options_t *options,
                                       download_status_t *dls,
                                       download_want_authority_t want_authority)
 {
-  int use_fallbacks = networkstatus_consensus_can_use_extra_fallbacks(options);
-  int max_dl_tries = options->ClientBootstrapConsensusMaxDownloadTries;
-  if (!use_fallbacks) {
-    max_dl_tries =
-              options->ClientBootstrapConsensusAuthorityOnlyMaxDownloadTries;
-  }
-
   const char *resource = networkstatus_get_flavor_name(
                                                   usable_consensus_flavor());
 
@@ -996,7 +985,7 @@ update_consensus_bootstrap_attempt_downloads(
 
   /* Allow for multiple connections in the same second, if the schedule value
    * is 0. */
-  while (download_status_is_ready(dls, now, max_dl_tries)) {
+  while (download_status_is_ready(dls, now)) {
     log_info(LD_DIR, "Launching %s bootstrap %s networkstatus consensus "
              "download.", resource, (want_authority == DL_WANT_AUTHORITY
                                      ? "authority"
@@ -1047,7 +1036,7 @@ update_consensus_bootstrap_multiple_downloads(time_t now,
 
     if (!check_consensus_waiting_for_certs(usable_flavor, now, dls_f)) {
       /* During bootstrap, DL_WANT_ANY_DIRSERVER means "use fallbacks". */
-      update_consensus_bootstrap_attempt_downloads(now, options, dls_f,
+      update_consensus_bootstrap_attempt_downloads(now, dls_f,
                                                    DL_WANT_ANY_DIRSERVER);
     }
   }
@@ -1057,7 +1046,7 @@ update_consensus_bootstrap_multiple_downloads(time_t now,
     &consensus_bootstrap_dl_status[CONSENSUS_BOOTSTRAP_SOURCE_AUTHORITY];
 
   if (!check_consensus_waiting_for_certs(usable_flavor, now, dls_a)) {
-    update_consensus_bootstrap_attempt_downloads(now, options, dls_a,
+    update_consensus_bootstrap_attempt_downloads(now, dls_a,
                                                  DL_WANT_AUTHORITY);
   }
 }

src/or/or.h

@@ -2070,15 +2070,6 @@ typedef enum {
 #define download_schedule_increment_bitfield_t \
                                         ENUM_BF(download_schedule_increment_t)
 
-/** Enumeration: do we want to use the random exponential backoff
- * mechanism? */
-typedef enum {
-  DL_SCHED_DETERMINISTIC = 0,
-  DL_SCHED_RANDOM_EXPONENTIAL = 1,
-} download_schedule_backoff_t;
-#define download_schedule_backoff_bitfield_t \
-                                        ENUM_BF(download_schedule_backoff_t)
-
 /** Information about our plans for retrying downloads for a downloadable
  * directory object.
  * Each type of downloadable directory object has a corresponding retry
@@ -2125,11 +2116,6 @@ typedef struct download_status_t {
   download_schedule_increment_bitfield_t increment_on : 1; /**< does this
                                         * schedule increment on each attempt,
                                         * or after each failure? */
-  download_schedule_backoff_bitfield_t backoff : 1; /**< do we use the
-                                        * deterministic schedule, or random
-                                        * exponential backoffs?
-                                        * Increment on failure schedules
-                                        * always use exponential backoff. */
   uint8_t last_backoff_position; /**< number of attempts/failures, depending
                                   * on increment_on, when we last recalculated
                                   * the delay.  Only updated if backoff
@@ -4425,37 +4411,11 @@ typedef struct {
    * it?  Only altered on testing networks. */
   int TestingDirConnectionMaxStall;
 
-  /** How many times will we try to fetch a consensus before we give
-   * up?  Only altered on testing networks. */
-  int TestingConsensusMaxDownloadTries;
-
-  /** How many times will a client try to fetch a consensus while
-   * bootstrapping using a list of fallback directories, before it gives up?
-   * Only altered on testing networks. */
-  int ClientBootstrapConsensusMaxDownloadTries;
-
-  /** How many times will a client try to fetch a consensus while
-   * bootstrapping using only a list of authorities, before it gives up?
-   * Only altered on testing networks. */
-  int ClientBootstrapConsensusAuthorityOnlyMaxDownloadTries;
-
   /** How many simultaneous in-progress connections will we make when trying
    * to fetch a consensus before we wait for one to complete, timeout, or
    * error out?  Only altered on testing networks. */
   int ClientBootstrapConsensusMaxInProgressTries;
 
-  /** How many times will we try to download a router's descriptor before
-   * giving up?  Only altered on testing networks. */
-  int TestingDescriptorMaxDownloadTries;
-
-  /** How many times will we try to download a microdescriptor before
-   * giving up?  Only altered on testing networks. */
-  int TestingMicrodescMaxDownloadTries;
-
-  /** How many times will we try to fetch a certificate before giving
-   * up?  Only altered on testing networks. */
-  int TestingCertMaxDownloadTries;
-
   /** If true, we take part in a testing network. Change the defaults of a
    * couple of other configuration options and allow to change the values
    * of certain configuration options. */

src/or/routerlist.c

@@ -177,7 +177,7 @@ static void download_status_reset_by_sk_in_cl(cert_list_t *cl,
                                               const char *digest);
 static int download_status_is_ready_by_sk_in_cl(cert_list_t *cl,
                                                 const char *digest,
-                                                time_t now, int max_failures);
+                                                time_t now);
 
 /****************************************************************************/
 
@@ -246,7 +246,6 @@ download_status_cert_init(download_status_t *dlstatus)
   dlstatus->schedule = DL_SCHED_CONSENSUS;
   dlstatus->want_authority = DL_WANT_ANY_DIRSERVER;
   dlstatus->increment_on = DL_SCHED_INCREMENT_FAILURE;
-  dlstatus->backoff = DL_SCHED_RANDOM_EXPONENTIAL;
   dlstatus->last_backoff_position = 0;
   dlstatus->last_delay_used = 0;
 
@@ -288,7 +287,7 @@ download_status_reset_by_sk_in_cl(cert_list_t *cl, const char *digest)
 static int
 download_status_is_ready_by_sk_in_cl(cert_list_t *cl,
                                      const char *digest,
-                                     time_t now, int max_failures)
+                                     time_t now)
 {
   int rv = 0;
   download_status_t *dlstatus = NULL;
@@ -305,7 +304,7 @@ download_status_is_ready_by_sk_in_cl(cert_list_t *cl,
   /* Got one? */
   if (dlstatus) {
     /* Use download_status_is_ready() */
-    rv = download_status_is_ready(dlstatus, now, max_failures);
+    rv = download_status_is_ready(dlstatus, now);
   } else {
     /*
      * If we don't know anything about it, return 1, since we haven't
@@ -1068,8 +1067,7 @@ authority_certs_fetch_missing(networkstatus_t *status, time_t now,
       }
     } SMARTLIST_FOREACH_END(cert);
     if (!found &&
-        download_status_is_ready(&(cl->dl_status_by_id), now,
+        download_status_is_ready(&(cl->dl_status_by_id), now) &&
-                                 options->TestingCertMaxDownloadTries) &&
         !digestmap_get(pending_id, ds->v3_identity_digest)) {
       log_info(LD_DIR,
                "No current certificate known for authority %s "
@@ -1132,8 +1130,7 @@ authority_certs_fetch_missing(networkstatus_t *status, time_t now,
           continue;
         }
         if (download_status_is_ready_by_sk_in_cl(
-              cl, sig->signing_key_digest,
+              cl, sig->signing_key_digest, now) &&
-              now, options->TestingCertMaxDownloadTries) &&
             !fp_pair_map_get_by_digests(pending_cert,
                                         voter->identity_digest,
                                         sig->signing_key_digest)) {
@@ -5167,8 +5164,7 @@ update_consensus_router_descriptor_downloads(time_t now, int is_vote,
         ++n_inprogress;
         continue; /* We have an in-progress download. */
       }
-      if (!download_status_is_ready(&rs->dl_status, now,
+      if (!download_status_is_ready(&rs->dl_status, now)) {
-                          options->TestingDescriptorMaxDownloadTries)) {
         ++n_delayed; /* Not ready for retry. */
         continue;
       }
@@ -5344,8 +5340,7 @@ update_extrainfo_downloads(time_t now)
         ++n_have;
         continue;
       }
-      if (!download_status_is_ready(&sd->ei_dl_status, now,
+      if (!download_status_is_ready(&sd->ei_dl_status, now)) {
-                          options->TestingDescriptorMaxDownloadTries)) {
         ++n_delay;
         continue;
       }

src/or/routerparse.c

@@ -3809,7 +3809,6 @@ networkstatus_parse_vote_from_string(const char *s, const char **eos_out,
                                                      ns->consensus_method,
                                                      flav))) {
         /* Use exponential-backoff scheduling when downloading microdescs */
-        rs->dl_status.backoff = DL_SCHED_RANDOM_EXPONENTIAL;
         smartlist_add(ns->routerstatus_list, rs);
       }
     }

src/test/test_controller.c

@@ -396,7 +396,7 @@ test_add_onion_helper_clientauth(void *arg)
 
 static const download_status_t dl_status_default =
   { 0, 0, 0, DL_SCHED_CONSENSUS, DL_WANT_ANY_DIRSERVER,
-    DL_SCHED_INCREMENT_FAILURE, DL_SCHED_RANDOM_EXPONENTIAL, 0, 0 };
+    DL_SCHED_INCREMENT_FAILURE, 0, 0 };
 static download_status_t ns_dl_status[N_CONSENSUS_FLAVORS];
 static download_status_t ns_dl_status_bootstrap[N_CONSENSUS_FLAVORS];
 static download_status_t ns_dl_status_running[N_CONSENSUS_FLAVORS];
@@ -407,7 +407,7 @@ static download_status_t ns_dl_status_running[N_CONSENSUS_FLAVORS];
  */
 static const download_status_t dls_sample_1 =
   { 1467163900, 0, 0, DL_SCHED_GENERIC, DL_WANT_ANY_DIRSERVER,
-    DL_SCHED_INCREMENT_FAILURE, DL_SCHED_DETERMINISTIC, 0, 0 };
+    DL_SCHED_INCREMENT_FAILURE, 0, 0 };
 static const char * dls_sample_1_str =
     "next-attempt-at 2016-06-29 01:31:40\n"
     "n-download-failures 0\n"
@@ -415,10 +415,12 @@ static const char * dls_sample_1_str =
     "schedule DL_SCHED_GENERIC\n"
     "want-authority DL_WANT_ANY_DIRSERVER\n"
     "increment-on DL_SCHED_INCREMENT_FAILURE\n"
-    "backoff DL_SCHED_DETERMINISTIC\n";
+    "backoff DL_SCHED_RANDOM_EXPONENTIAL\n"
+    "last-backoff-position 0\n"
+    "last-delay-used 0\n";
 static const download_status_t dls_sample_2 =
   { 1467164400, 1, 2, DL_SCHED_CONSENSUS, DL_WANT_AUTHORITY,
-    DL_SCHED_INCREMENT_FAILURE, DL_SCHED_DETERMINISTIC, 0, 0 };
+    DL_SCHED_INCREMENT_FAILURE, 0, 0 };
 static const char * dls_sample_2_str =
     "next-attempt-at 2016-06-29 01:40:00\n"
     "n-download-failures 1\n"
@@ -426,10 +428,12 @@ static const char * dls_sample_2_str =
     "schedule DL_SCHED_CONSENSUS\n"
     "want-authority DL_WANT_AUTHORITY\n"
     "increment-on DL_SCHED_INCREMENT_FAILURE\n"
-    "backoff DL_SCHED_DETERMINISTIC\n";
+    "backoff DL_SCHED_RANDOM_EXPONENTIAL\n"
+    "last-backoff-position 0\n"
+    "last-delay-used 0\n";
 static const download_status_t dls_sample_3 =
   { 1467154400, 12, 25, DL_SCHED_BRIDGE, DL_WANT_ANY_DIRSERVER,
-    DL_SCHED_INCREMENT_ATTEMPT, DL_SCHED_DETERMINISTIC, 0, 0 };
+    DL_SCHED_INCREMENT_ATTEMPT, 0, 0 };
 static const char * dls_sample_3_str =
     "next-attempt-at 2016-06-28 22:53:20\n"
     "n-download-failures 12\n"
@@ -437,10 +441,12 @@ static const char * dls_sample_3_str =
     "schedule DL_SCHED_BRIDGE\n"
     "want-authority DL_WANT_ANY_DIRSERVER\n"
     "increment-on DL_SCHED_INCREMENT_ATTEMPT\n"
-    "backoff DL_SCHED_DETERMINISTIC\n";
+    "backoff DL_SCHED_RANDOM_EXPONENTIAL\n"
+    "last-backoff-position 0\n"
+    "last-delay-used 0\n";
 static const download_status_t dls_sample_4 =
   { 1467166600, 3, 0, DL_SCHED_GENERIC, DL_WANT_ANY_DIRSERVER,
-    DL_SCHED_INCREMENT_FAILURE, DL_SCHED_RANDOM_EXPONENTIAL, 0, 0 };
+    DL_SCHED_INCREMENT_FAILURE, 0, 0 };
 static const char * dls_sample_4_str =
     "next-attempt-at 2016-06-29 02:16:40\n"
     "n-download-failures 3\n"
@@ -453,7 +459,7 @@ static const char * dls_sample_4_str =
     "last-delay-used 0\n";
 static const download_status_t dls_sample_5 =
   { 1467164600, 3, 7, DL_SCHED_CONSENSUS, DL_WANT_ANY_DIRSERVER,
-    DL_SCHED_INCREMENT_FAILURE, DL_SCHED_RANDOM_EXPONENTIAL, 1, 2112, };
+    DL_SCHED_INCREMENT_FAILURE, 1, 2112, };
 static const char * dls_sample_5_str =
     "next-attempt-at 2016-06-29 01:43:20\n"
     "n-download-failures 3\n"
@@ -466,7 +472,7 @@ static const char * dls_sample_5_str =
     "last-delay-used 2112\n";
 static const download_status_t dls_sample_6 =
   { 1467164200, 4, 9, DL_SCHED_CONSENSUS, DL_WANT_AUTHORITY,
-    DL_SCHED_INCREMENT_ATTEMPT, DL_SCHED_RANDOM_EXPONENTIAL, 3, 432 };
+    DL_SCHED_INCREMENT_ATTEMPT, 3, 432 };
 static const char * dls_sample_6_str =
     "next-attempt-at 2016-06-29 01:36:40\n"
     "n-download-failures 4\n"

src/test/test_dir.c

@@ -3965,164 +3965,11 @@ test_dir_packages(void *arg)
 }
 
 static void
-test_dir_download_status_schedule(void *arg)
+download_status_random_backoff_helper(int min_delay)
-{
-  (void)arg;
-  download_status_t dls_failure = { 0, 0, 0, DL_SCHED_GENERIC,
-                                             DL_WANT_AUTHORITY,
-                                             DL_SCHED_INCREMENT_FAILURE,
-                                             DL_SCHED_DETERMINISTIC, 0, 0 };
-  download_status_t dls_attempt = { 0, 0, 0, DL_SCHED_CONSENSUS,
-                                             DL_WANT_ANY_DIRSERVER,
-                                             DL_SCHED_INCREMENT_ATTEMPT,
-                                             DL_SCHED_DETERMINISTIC, 0, 0 };
-  download_status_t dls_bridge  = { 0, 0, 0, DL_SCHED_BRIDGE,
-                                             DL_WANT_AUTHORITY,
-                                             DL_SCHED_INCREMENT_FAILURE,
-                                             DL_SCHED_DETERMINISTIC, 0, 0 };
-  int increment = -1;
-  int expected_increment = -1;
-  time_t current_time = time(NULL);
-  int delay1 = -1;
-  int delay2 = -1;
-  smartlist_t *schedule = smartlist_new();
-
-  /* Make a dummy schedule */
-  smartlist_add(schedule, (void *)&delay1);
-  smartlist_add(schedule, (void *)&delay2);
-
-  /* check a range of values */
-  delay1 = 1000;
-  increment = download_status_schedule_get_delay(&dls_failure,
-                                                 schedule,
-                                                 0, INT_MAX,
-                                                 TIME_MIN);
-  expected_increment = delay1;
-  tt_assert(increment == expected_increment);
-  tt_assert(dls_failure.next_attempt_at == TIME_MIN + expected_increment);
-
-  delay1 = INT_MAX;
-  increment =  download_status_schedule_get_delay(&dls_failure,
-                                                  schedule,
-                                                  0, INT_MAX,
-                                                  -1);
-  expected_increment = delay1;
-  tt_assert(increment == expected_increment);
-  tt_assert(dls_failure.next_attempt_at == TIME_MAX);
-
-  delay1 = 0;
-  increment = download_status_schedule_get_delay(&dls_attempt,
-                                                 schedule,
-                                                 0, INT_MAX,
-                                                 0);
-  expected_increment = delay1;
-  tt_assert(increment == expected_increment);
-  tt_assert(dls_attempt.next_attempt_at == 0 + expected_increment);
-
-  delay1 = 1000;
-  increment = download_status_schedule_get_delay(&dls_attempt,
-                                                 schedule,
-                                                 0, INT_MAX,
-                                                 1);
-  expected_increment = delay1;
-  tt_assert(increment == expected_increment);
-  tt_assert(dls_attempt.next_attempt_at == 1 + expected_increment);
-
-  delay1 = INT_MAX;
-  increment = download_status_schedule_get_delay(&dls_bridge,
-                                                 schedule,
-                                                 0, INT_MAX,
-                                                 current_time);
-  expected_increment = delay1;
-  tt_assert(increment == expected_increment);
-  tt_assert(dls_bridge.next_attempt_at == TIME_MAX);
-
-  delay1 = 1;
-  increment = download_status_schedule_get_delay(&dls_bridge,
-                                                 schedule,
-                                                 0, INT_MAX,
-                                                 TIME_MAX);
-  expected_increment = delay1;
-  tt_assert(increment == expected_increment);
-  tt_assert(dls_bridge.next_attempt_at == TIME_MAX);
-
-  /* see what happens when we reach the end */
-  dls_attempt.n_download_attempts++;
-  dls_bridge.n_download_failures++;
-
-  delay2 = 100;
-  increment = download_status_schedule_get_delay(&dls_attempt,
-                                                 schedule,
-                                                 0, INT_MAX,
-                                                 current_time);
-  expected_increment = delay2;
-  tt_assert(increment == expected_increment);
-  tt_assert(dls_attempt.next_attempt_at == current_time + delay2);
-
-  delay2 = 1;
-  increment = download_status_schedule_get_delay(&dls_bridge,
-                                                 schedule,
-                                                 0, INT_MAX,
-                                                 current_time);
-  expected_increment = delay2;
-  tt_assert(increment == expected_increment);
-  tt_assert(dls_bridge.next_attempt_at == current_time + delay2);
-
-  /* see what happens when we try to go off the end */
-  dls_attempt.n_download_attempts++;
-  dls_bridge.n_download_failures++;
-
-  delay2 = 5;
-  increment = download_status_schedule_get_delay(&dls_attempt,
-                                                 schedule,
-                                                 0, INT_MAX,
-                                                 current_time);
-  expected_increment = delay2;
-  tt_assert(increment == expected_increment);
-  tt_assert(dls_attempt.next_attempt_at == current_time + delay2);
-
-  delay2 = 17;
-  increment = download_status_schedule_get_delay(&dls_bridge,
-                                                 schedule,
-                                                 0, INT_MAX,
-                                                 current_time);
-  expected_increment = delay2;
-  tt_assert(increment == expected_increment);
-  tt_assert(dls_bridge.next_attempt_at == current_time + delay2);
-
-  /* see what happens when we reach IMPOSSIBLE_TO_DOWNLOAD */
-  dls_attempt.n_download_attempts = IMPOSSIBLE_TO_DOWNLOAD;
-  dls_bridge.n_download_failures = IMPOSSIBLE_TO_DOWNLOAD;
-
-  delay2 = 35;
-  increment = download_status_schedule_get_delay(&dls_attempt,
-                                                 schedule,
-                                                 0, INT_MAX,
-                                                 current_time);
-  expected_increment = INT_MAX;
-  tt_assert(increment == expected_increment);
-  tt_assert(dls_attempt.next_attempt_at == TIME_MAX);
-
-  delay2 = 99;
-  increment = download_status_schedule_get_delay(&dls_bridge,
-                                                 schedule,
-                                                 0, INT_MAX,
-                                                 current_time);
-  expected_increment = INT_MAX;
-  tt_assert(increment == expected_increment);
-  tt_assert(dls_bridge.next_attempt_at == TIME_MAX);
-
- done:
-  /* the pointers in schedule are allocated on the stack */
-  smartlist_free(schedule);
-}
-
-static void
-download_status_random_backoff_helper(int min_delay, int max_delay)
 {
   download_status_t dls_random =
     { 0, 0, 0, DL_SCHED_GENERIC, DL_WANT_AUTHORITY,
-               DL_SCHED_INCREMENT_FAILURE, DL_SCHED_RANDOM_EXPONENTIAL, 0, 0 };
+               DL_SCHED_INCREMENT_FAILURE, 0, 0 };
   int increment = -1;
   int old_increment = -1;
   time_t current_time = time(NULL);
@@ -4131,23 +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,
-                                                   NULL,
+                                                   min_delay,
-                                                   min_delay, max_delay,
                                                    current_time);
 
-    log_debug(LD_DIR, "Min: %d, Max: %d, Inc: %d, Old Inc: %d",
+    log_debug(LD_DIR, "Min: %d, Inc: %d, Old Inc: %d",
-              min_delay, max_delay, increment, old_increment);
+              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) {
@@ -4157,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;
@@ -4169,19 +4014,13 @@ test_dir_download_status_random_backoff(void *arg)
   (void)arg;
 
   /* Do a standard test */
-  download_status_random_backoff_helper(0, 1000000);
+  download_status_random_backoff_helper(0);
-  /* Regression test for 20534 and friends:
-   * try tighter bounds */
-  download_status_random_backoff_helper(0, 100);
   /* regression tests for 17750: initial delay */
-  download_status_random_backoff_helper(10, 1000);
+  download_status_random_backoff_helper(10);
-  download_status_random_backoff_helper(20, 30);
+  download_status_random_backoff_helper(20);
 
   /* Pathological cases */
-  download_status_random_backoff_helper(0, 0);
+  download_status_random_backoff_helper(INT_MAX/2);
-  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);
 }
 
 static void
@@ -4220,18 +4059,10 @@ static void
 test_dir_download_status_increment(void *arg)
 {
   (void)arg;
-  download_status_t dls_failure = { 0, 0, 0, DL_SCHED_GENERIC,
-    DL_WANT_AUTHORITY,
-    DL_SCHED_INCREMENT_FAILURE,
-    DL_SCHED_DETERMINISTIC, 0, 0 };
-  download_status_t dls_attempt = { 0, 0, 0, DL_SCHED_BRIDGE,
-    DL_WANT_ANY_DIRSERVER,
-    DL_SCHED_INCREMENT_ATTEMPT,
-    DL_SCHED_DETERMINISTIC, 0, 0 };
   download_status_t dls_exp = { 0, 0, 0, DL_SCHED_GENERIC,
     DL_WANT_ANY_DIRSERVER,
     DL_SCHED_INCREMENT_ATTEMPT,
-    DL_SCHED_RANDOM_EXPONENTIAL, 0, 0 };
+    0, 0 };
   int no_delay = 0;
   int delay0 = -1;
   int delay1 = -1;
@@ -4239,7 +4070,6 @@ test_dir_download_status_increment(void *arg)
   smartlist_t *schedule = smartlist_new();
   smartlist_t *schedule_no_initial_delay = smartlist_new();
   or_options_t test_options;
-  time_t next_at = TIME_MAX;
   time_t current_time = time(NULL);
 
   /* Provide some values for the schedules */
@@ -4272,26 +4102,6 @@ test_dir_download_status_increment(void *arg)
   mock_get_options_calls = 0;
   /* we really want to test that it's equal to time(NULL) + delay0, but that's
    * an unrealiable test, because time(NULL) might change. */
-  tt_assert(download_status_get_next_attempt_at(&dls_failure)
-            >= current_time + no_delay);
-  tt_assert(download_status_get_next_attempt_at(&dls_failure)
-            != TIME_MAX);
-  tt_int_op(download_status_get_n_failures(&dls_failure), OP_EQ, 0);
-  tt_int_op(download_status_get_n_attempts(&dls_failure), OP_EQ, 0);
-  tt_int_op(mock_get_options_calls, OP_GE, 1);
-
-  /* regression test for 17750: initial delay */
-  mock_options->TestingClientDownloadSchedule = schedule;
-  mock_get_options_calls = 0;
-  /* we really want to test that it's equal to time(NULL) + delay0, but that's
-   * an unrealiable test, because time(NULL) might change. */
-  tt_assert(download_status_get_next_attempt_at(&dls_failure)
-            >= current_time + delay0);
-  tt_assert(download_status_get_next_attempt_at(&dls_failure)
-            != TIME_MAX);
-  tt_int_op(download_status_get_n_failures(&dls_failure), OP_EQ, 0);
-  tt_int_op(download_status_get_n_attempts(&dls_failure), OP_EQ, 0);
-  tt_int_op(mock_get_options_calls, OP_GE, 1);
 
   /* regression test for 17750: exponential, no initial delay */
   mock_options->TestingClientDownloadSchedule = schedule_no_initial_delay;
@@ -4319,258 +4129,6 @@ test_dir_download_status_increment(void *arg)
   tt_int_op(download_status_get_n_attempts(&dls_exp), OP_EQ, 0);
   tt_int_op(mock_get_options_calls, OP_GE, 1);
 
-  /* Check that a failure reset works */
-  mock_get_options_calls = 0;
-  download_status_reset(&dls_failure);
-  /* we really want to test that it's equal to time(NULL) + delay0, but that's
-   * an unrealiable test, because time(NULL) might change. */
-  tt_assert(download_status_get_next_attempt_at(&dls_failure)
-            >= current_time + delay0);
-  tt_assert(download_status_get_next_attempt_at(&dls_failure)
-            != TIME_MAX);
-  tt_int_op(download_status_get_n_failures(&dls_failure), OP_EQ, 0);
-  tt_int_op(download_status_get_n_attempts(&dls_failure), OP_EQ, 0);
-  tt_int_op(mock_get_options_calls, OP_GE, 1);
-
-  /* avoid timing inconsistencies */
-  dls_failure.next_attempt_at = current_time + delay0;
-
-  /* check that a reset schedule becomes ready at the right time */
-  tt_int_op(download_status_is_ready(&dls_failure,
-                                     current_time + delay0 - 1, 1),
-            OP_EQ, 0);
-  tt_int_op(download_status_is_ready(&dls_failure,
-                                     current_time + delay0, 1),
-            OP_EQ, 1);
-  tt_int_op(download_status_is_ready(&dls_failure,
-                                     current_time + delay0 + 1, 1),
-            OP_EQ, 1);
-
-  /* Check that a failure increment works */
-  mock_get_options_calls = 0;
-  next_at = download_status_increment_failure(&dls_failure, 404, "test", 0,
-                                              current_time);
-  tt_assert(next_at == current_time + delay1);
-  tt_int_op(download_status_get_n_failures(&dls_failure), OP_EQ, 1);
-  tt_int_op(download_status_get_n_attempts(&dls_failure), OP_EQ, 1);
-  tt_int_op(mock_get_options_calls, OP_GE, 1);
-
-  /* check that an incremented schedule becomes ready at the right time */
-  tt_int_op(download_status_is_ready(&dls_failure,
-                                     current_time + delay1 - 1, 1),
-            OP_EQ, 0);
-  tt_int_op(download_status_is_ready(&dls_failure,
-                                     current_time + delay1, 1),
-            OP_EQ, 1);
-  tt_int_op(download_status_is_ready(&dls_failure,
-                                     current_time + delay1 + 1, 1),
-            OP_EQ, 1);
-
-  /* check that a schedule isn't ready if it's had too many failures */
-  tt_int_op(download_status_is_ready(&dls_failure,
-                                     current_time + delay1 + 10, 0),
-            OP_EQ, 0);
-
-  /* Check that failure increments do happen on 503 for clients, and
-   * attempt increments do too. */
-  mock_get_options_calls = 0;
-  next_at = download_status_increment_failure(&dls_failure, 503, "test", 0,
-                                              current_time);
-  tt_i64_op(next_at, OP_EQ, current_time + delay2);
-  tt_int_op(download_status_get_n_failures(&dls_failure), OP_EQ, 2);
-  tt_int_op(download_status_get_n_attempts(&dls_failure), OP_EQ, 2);
-  tt_int_op(mock_get_options_calls, OP_GE, 1);
-
-  /* Check that failure increments do happen on 503 for servers */
-  mock_get_options_calls = 0;
-  next_at = download_status_increment_failure(&dls_failure, 503, "test", 1,
-                                              current_time);
-  tt_assert(next_at == current_time + delay2);
-  tt_int_op(download_status_get_n_failures(&dls_failure), OP_EQ, 3);
-  tt_int_op(download_status_get_n_attempts(&dls_failure), OP_EQ, 3);
-  tt_int_op(mock_get_options_calls, OP_GE, 1);
-
-  /* Check what happens when we run off the end of the schedule */
-  mock_get_options_calls = 0;
-  next_at = download_status_increment_failure(&dls_failure, 404, "test", 0,
-                                              current_time);
-  tt_assert(next_at == current_time + delay2);
-  tt_int_op(download_status_get_n_failures(&dls_failure), OP_EQ, 4);
-  tt_int_op(download_status_get_n_attempts(&dls_failure), OP_EQ, 4);
-  tt_int_op(mock_get_options_calls, OP_GE, 1);
-
-  /* Check what happens when we hit the failure limit */
-  mock_get_options_calls = 0;
-  download_status_mark_impossible(&dls_failure);
-  next_at = download_status_increment_failure(&dls_failure, 404, "test", 0,
-                                              current_time);
-  tt_assert(next_at == TIME_MAX);
-  tt_int_op(download_status_get_n_failures(&dls_failure), OP_EQ,
-            IMPOSSIBLE_TO_DOWNLOAD);
-  tt_int_op(download_status_get_n_attempts(&dls_failure), OP_EQ,
-            IMPOSSIBLE_TO_DOWNLOAD);
-  tt_int_op(mock_get_options_calls, OP_GE, 1);
-
-  /* Check that a failure reset doesn't reset at the limit */
-  mock_get_options_calls = 0;
-  download_status_reset(&dls_failure);
-  tt_assert(download_status_get_next_attempt_at(&dls_failure)
-            == TIME_MAX);
-  tt_int_op(download_status_get_n_failures(&dls_failure), OP_EQ,
-            IMPOSSIBLE_TO_DOWNLOAD);
-  tt_int_op(download_status_get_n_attempts(&dls_failure), OP_EQ,
-            IMPOSSIBLE_TO_DOWNLOAD);
-  tt_int_op(mock_get_options_calls, OP_EQ, 0);
-
-  /* Check that a failure reset resets just before the limit */
-  mock_get_options_calls = 0;
-  dls_failure.n_download_failures = IMPOSSIBLE_TO_DOWNLOAD - 1;
-  dls_failure.n_download_attempts = IMPOSSIBLE_TO_DOWNLOAD - 1;
-  download_status_reset(&dls_failure);
-  /* we really want to test that it's equal to time(NULL) + delay0, but that's
-   * an unrealiable test, because time(NULL) might change. */
-  tt_assert(download_status_get_next_attempt_at(&dls_failure)
-            >= current_time + delay0);
-  tt_assert(download_status_get_next_attempt_at(&dls_failure)
-            != TIME_MAX);
-  tt_int_op(download_status_get_n_failures(&dls_failure), OP_EQ, 0);
-  tt_int_op(download_status_get_n_attempts(&dls_failure), OP_EQ, 0);
-  tt_int_op(mock_get_options_calls, OP_GE, 1);
-
-  /* Check that failure increments do happen on attempt-based schedules,
-   * but that the retry is set at the end of time */
-  mock_options->UseBridges = 1;
-  mock_get_options_calls = 0;
-  next_at = download_status_increment_failure(&dls_attempt, 404, "test", 0,
-                                              current_time);
-  tt_assert(next_at == TIME_MAX);
-  tt_int_op(download_status_get_n_failures(&dls_attempt), OP_EQ, 1);
-  tt_int_op(download_status_get_n_attempts(&dls_attempt), OP_EQ, 0);
-  tt_int_op(mock_get_options_calls, OP_GE, 1);
-
-  /* Check that an attempt reset works */
-  mock_get_options_calls = 0;
-  download_status_reset(&dls_attempt);
-  /* we really want to test that it's equal to time(NULL) + delay0, but that's
-   * an unrealiable test, because time(NULL) might change. */
-  tt_assert(download_status_get_next_attempt_at(&dls_attempt)
-            >= current_time + delay0);
-  tt_assert(download_status_get_next_attempt_at(&dls_attempt)
-            != TIME_MAX);
-  tt_int_op(download_status_get_n_failures(&dls_attempt), OP_EQ, 0);
-  tt_int_op(download_status_get_n_attempts(&dls_attempt), OP_EQ, 0);
-  tt_int_op(mock_get_options_calls, OP_GE, 1);
-
-  /* avoid timing inconsistencies */
-  dls_attempt.next_attempt_at = current_time + delay0;
-
-  /* check that a reset schedule becomes ready at the right time */
-  tt_int_op(download_status_is_ready(&dls_attempt,
-                                     current_time + delay0 - 1, 1),
-            OP_EQ, 0);
-  tt_int_op(download_status_is_ready(&dls_attempt,
-                                     current_time + delay0, 1),
-            OP_EQ, 1);
-  tt_int_op(download_status_is_ready(&dls_attempt,
-                                     current_time + delay0 + 1, 1),
-            OP_EQ, 1);
-
-  /* Check that an attempt increment works */
-  mock_get_options_calls = 0;
-  next_at = download_status_increment_attempt(&dls_attempt, "test",
-                                              current_time);
-  tt_assert(next_at == current_time + delay1);
-  tt_int_op(download_status_get_n_failures(&dls_attempt), OP_EQ, 0);
-  tt_int_op(download_status_get_n_attempts(&dls_attempt), OP_EQ, 1);
-  tt_int_op(mock_get_options_calls, OP_GE, 1);
-
-  /* check that an incremented schedule becomes ready at the right time */
-  tt_int_op(download_status_is_ready(&dls_attempt,
-                                     current_time + delay1 - 1, 1),
-            OP_EQ, 0);
-  tt_int_op(download_status_is_ready(&dls_attempt,
-                                     current_time + delay1, 1),
-            OP_EQ, 1);
-  tt_int_op(download_status_is_ready(&dls_attempt,
-                                     current_time + delay1 + 1, 1),
-            OP_EQ, 1);
-
-  /* check that a schedule isn't ready if it's had too many attempts */
-  tt_int_op(download_status_is_ready(&dls_attempt,
-                                     current_time + delay1 + 10, 0),
-            OP_EQ, 0);
-
-  /* Check what happens when we reach then run off the end of the schedule */
-  mock_get_options_calls = 0;
-  next_at = download_status_increment_attempt(&dls_attempt, "test",
-                                              current_time);
-  tt_assert(next_at == current_time + delay2);
-  tt_int_op(download_status_get_n_failures(&dls_attempt), OP_EQ, 0);
-  tt_int_op(download_status_get_n_attempts(&dls_attempt), OP_EQ, 2);
-  tt_int_op(mock_get_options_calls, OP_GE, 1);
-
-  mock_get_options_calls = 0;
-  next_at = download_status_increment_attempt(&dls_attempt, "test",
-                                              current_time);
-  tt_assert(next_at == current_time + delay2);
-  tt_int_op(download_status_get_n_failures(&dls_attempt), OP_EQ, 0);
-  tt_int_op(download_status_get_n_attempts(&dls_attempt), OP_EQ, 3);
-  tt_int_op(mock_get_options_calls, OP_GE, 1);
-
-  /* Check what happens when we hit the attempt limit */
-  mock_get_options_calls = 0;
-  download_status_mark_impossible(&dls_attempt);
-  next_at = download_status_increment_attempt(&dls_attempt, "test",
-                                              current_time);
-  tt_assert(next_at == TIME_MAX);
-  tt_int_op(download_status_get_n_failures(&dls_attempt), OP_EQ,
-            IMPOSSIBLE_TO_DOWNLOAD);
-  tt_int_op(download_status_get_n_attempts(&dls_attempt), OP_EQ,
-            IMPOSSIBLE_TO_DOWNLOAD);
-  tt_int_op(mock_get_options_calls, OP_GE, 1);
-
-  /* Check that an attempt reset doesn't reset at the limit */
-  mock_get_options_calls = 0;
-  download_status_reset(&dls_attempt);
-  tt_assert(download_status_get_next_attempt_at(&dls_attempt)
-            == TIME_MAX);
-  tt_int_op(download_status_get_n_failures(&dls_attempt), OP_EQ,
-            IMPOSSIBLE_TO_DOWNLOAD);
-  tt_int_op(download_status_get_n_attempts(&dls_attempt), OP_EQ,
-            IMPOSSIBLE_TO_DOWNLOAD);
-  tt_int_op(mock_get_options_calls, OP_EQ, 0);
-
-  /* Check that an attempt reset resets just before the limit */
-  mock_get_options_calls = 0;
-  dls_attempt.n_download_failures = IMPOSSIBLE_TO_DOWNLOAD - 1;
-  dls_attempt.n_download_attempts = IMPOSSIBLE_TO_DOWNLOAD - 1;
-  download_status_reset(&dls_attempt);
-  /* we really want to test that it's equal to time(NULL) + delay0, but that's
-   * an unrealiable test, because time(NULL) might change. */
-  tt_assert(download_status_get_next_attempt_at(&dls_attempt)
-            >= current_time + delay0);
-  tt_assert(download_status_get_next_attempt_at(&dls_attempt)
-            != TIME_MAX);
-  tt_int_op(download_status_get_n_failures(&dls_attempt), OP_EQ, 0);
-  tt_int_op(download_status_get_n_attempts(&dls_attempt), OP_EQ, 0);
-  tt_int_op(mock_get_options_calls, OP_GE, 1);
-  mock_options->UseBridges = 0;
-
-  /* Check that attempt increments don't happen on failure-based schedules,
-   * and that the attempt is set at the end of time */
-  mock_get_options_calls = 0;
-  setup_full_capture_of_logs(LOG_WARN);
-  next_at = download_status_increment_attempt(&dls_failure, "test",
-                                              current_time);
-  expect_single_log_msg_containing(
-    "Tried to launch an attempt-based connection on a failure-based "
-    "schedule.");
-  teardown_capture_of_logs();
-  tt_assert(next_at == TIME_MAX);
-  tt_int_op(download_status_get_n_failures(&dls_failure), OP_EQ, 0);
-  tt_int_op(download_status_get_n_attempts(&dls_failure), OP_EQ, 0);
-  tt_int_op(mock_get_options_calls, OP_EQ, 0);
-
  done:
   /* the pointers in schedule are allocated on the stack */
   smartlist_free(schedule);
@@ -6318,7 +5876,6 @@ struct testcase_t dir_tests[] = {
   DIR(post_parsing, 0),
   DIR(fetch_type, 0),
   DIR(packages, 0),
-  DIR(download_status_schedule, 0),
   DIR(download_status_random_backoff, 0),
   DIR(download_status_random_backoff_ranges, 0),
   DIR(download_status_increment, TT_FORK),

src/test/test_options.c

@@ -377,17 +377,11 @@ fixed_get_uname(void)
 }
 
 #define TEST_OPTIONS_OLD_VALUES   "TestingV3AuthInitialVotingInterval 1800\n" \
-  "ClientBootstrapConsensusMaxDownloadTries 7\n" \
-  "ClientBootstrapConsensusAuthorityOnlyMaxDownloadTries 4\n" \
   "ClientBootstrapConsensusMaxInProgressTries 3\n" \
   "TestingV3AuthInitialVoteDelay 300\n"   \
   "TestingV3AuthInitialDistDelay 300\n" \
   "TestingClientMaxIntervalWithoutRequest 600\n" \
   "TestingDirConnectionMaxStall 600\n" \
-  "TestingConsensusMaxDownloadTries 8\n" \
-  "TestingDescriptorMaxDownloadTries 8\n" \
-  "TestingMicrodescMaxDownloadTries 8\n" \
-  "TestingCertMaxDownloadTries 8\n"
 
 #define TEST_OPTIONS_DEFAULT_VALUES TEST_OPTIONS_OLD_VALUES \
   "MaxClientCircuitsPending 1\n"                                        \
@@ -2081,10 +2075,6 @@ test_options_validate__testing(void *ignored)
   ENSURE_DEFAULT(TestingBridgeBootstrapDownloadSchedule, 3000);
   ENSURE_DEFAULT(TestingClientMaxIntervalWithoutRequest, 3000);
   ENSURE_DEFAULT(TestingDirConnectionMaxStall, 3000);
-  ENSURE_DEFAULT(TestingConsensusMaxDownloadTries, 3000);
-  ENSURE_DEFAULT(TestingDescriptorMaxDownloadTries, 3000);
-  ENSURE_DEFAULT(TestingMicrodescMaxDownloadTries, 3000);
-  ENSURE_DEFAULT(TestingCertMaxDownloadTries, 3000);
   ENSURE_DEFAULT(TestingAuthKeyLifetime, 3000);
   ENSURE_DEFAULT(TestingLinkCertLifetime, 3000);
   ENSURE_DEFAULT(TestingSigningKeySlop, 3000);
@@ -4069,15 +4059,6 @@ test_options_validate__testing_options(void *ignored)
                       "is way too low.");
   TEST_TESTING_OPTION(TestingDirConnectionMaxStall, 1, 3601,
                       "is way too low.");
-  // TODO: I think this points to a bug/regression in options_validate
-  TEST_TESTING_OPTION(TestingConsensusMaxDownloadTries, 1, 801,
-                      "must be greater than 2.");
-  TEST_TESTING_OPTION(TestingDescriptorMaxDownloadTries, 1, 801,
-                      "must be greater than 1.");
-  TEST_TESTING_OPTION(TestingMicrodescMaxDownloadTries, 1, 801,
-                      "must be greater than 1.");
-  TEST_TESTING_OPTION(TestingCertMaxDownloadTries, 1, 801,
-                      "must be greater than 1.");
 
   free_options_test_data(tdata);
   tdata = get_options_test_data(TEST_OPTIONS_DEFAULT_VALUES