[tor-commits] [tor/master] Remove the old ("deterministic") download schedule.
nickm at torproject.org
nickm at torproject.org
Sat Mar 3 16:54:23 UTC 2018
commit e0049ef022b8bf9b808a9074820bb2a33f92ac1b
Author: Nick Mathewson <nickm at torproject.org>
Date: Thu Jan 25 15:51:13 2018 -0500
Remove the old ("deterministic") download schedule.
We haven't meant to use it since we introduced the random
exponential schedule.
Closes ticket 23814.
---
changes/refactor23814 | 4 +
src/or/bridges.c | 1 -
src/or/control.c | 39 +---
src/or/directory.c | 35 +---
src/or/directory.h | 11 +-
src/or/networkstatus.c | 8 +-
src/or/or.h | 14 --
src/or/routerlist.c | 1 -
src/or/routerparse.c | 1 -
src/test/test_controller.c | 26 +--
src/test/test_dir.c | 440 +--------------------------------------------
11 files changed, 46 insertions(+), 534 deletions(-)
diff --git a/changes/refactor23814 b/changes/refactor23814
new file mode 100644
index 000000000..75d84bfd1
--- /dev/null
+++ b/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 24814
diff --git a/src/or/bridges.c b/src/or/bridges.c
index 383c33fa6..3d9af21fa 100644
--- a/src/or/bridges.c
+++ b/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. */
diff --git a/src/or/control.c b/src/or/control.c
index cce5c7953..1cb6360c8 100644
--- a/src/or/control.c
+++ b/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) {
- 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;
- }
+ backoff_str = "DL_SCHED_RANDOM_EXPONENTIAL";
/* 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);
-
- if (dl->backoff == DL_SCHED_RANDOM_EXPONENTIAL) {
- /* 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;
- }
+ backoff_str,
+ dl->last_backoff_position,
+ dl->last_delay_used);
}
return rv;
diff --git a/src/or/directory.c b/src/or/directory.c
index c55f81bc6..a2e514e79 100644
--- a/src/or/directory.c
+++ b/src/or/directory.c
@@ -5383,10 +5383,7 @@ find_dl_min_and_max_delay(download_status_t *dls, const or_options_t *options,
*min = *((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;
+ *max = INT_MAX;
}
/** As next_random_exponential_delay() below, but does not compute a random
@@ -5453,26 +5450,20 @@ next_random_exponential_delay(int delay,
return MIN(rand_delay, max_delay);
}
-/** Find the current delay for dls based on schedule or 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.
+/** Find the current delay for dls based on min_delay/
+ * max_delay. Requires that 0 <= min_delay <= max_delay <= INT_MAX.
+ *
* 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 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 ||
- (min_delay >= 0 && max_delay >= min_delay));
+ tor_assert(min_delay >= 0 && max_delay >= min_delay);
int delay = INT_MAX;
uint8_t dls_schedule_position = (dls->increment_on
@@ -5480,14 +5471,8 @@ download_status_schedule_get_delay(download_status_t *dls,
? dls->n_download_attempts
: dls->n_download_failures);
- if (dls->backoff == DL_SCHED_DETERMINISTIC) {
- if (dls_schedule_position < smartlist_len(schedule))
- delay = *(int *)smartlist_get(schedule, dls_schedule_position);
- else if (dls_schedule_position == IMPOSSIBLE_TO_DOWNLOAD)
- delay = INT_MAX;
- else
- delay = *(int *)smartlist_get(schedule, smartlist_len(schedule) - 1);
- } else if (dls->backoff == DL_SCHED_RANDOM_EXPONENTIAL) {
+ { // XXXX 23814 unindent.
+
/* Check if we missed a reset somehow */
IF_BUG_ONCE(dls->last_backoff_position > dls_schedule_position) {
dls->last_backoff_position = 0;
@@ -5603,9 +5588,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());
find_dl_min_and_max_delay(dls, get_options(), &min_delay, &max_delay);
- increment = download_status_schedule_get_delay(dls, schedule,
+ increment = download_status_schedule_get_delay(dls,
min_delay, max_delay, now);
}
@@ -5657,9 +5641,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());
find_dl_min_and_max_delay(dls, get_options(), &min_delay, &max_delay);
- delay = download_status_schedule_get_delay(dls, schedule,
+ delay = download_status_schedule_get_delay(dls,
min_delay, max_delay, now);
download_status_log_helper(item, dls->increment_on, "attempted",
diff --git a/src/or/directory.h b/src/or/directory.h
index edf75ffe1..6008d3a88 100644
--- a/src/or/directory.h
+++ b/src/or/directory.h
@@ -142,19 +142,13 @@ static inline int
download_status_is_ready(download_status_t *dls, time_t now,
int max_failures)
{
+ (void) max_failures; // 23814 REMOVE
+
/* 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,7 +254,6 @@ 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 max_delay,
time_t now);
diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c
index aa0a8d15c..e051e41f5 100644
--- a/src/or/networkstatus.c
+++ b/src/or/networkstatus.c
@@ -106,9 +106,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
@@ -125,10 +125,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
diff --git a/src/or/or.h b/src/or/or.h
index c81e29c95..f93050dfe 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -2069,15 +2069,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
@@ -2124,11 +2115,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
diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index 2815c6096..fec224fad 100644
--- a/src/or/routerlist.c
+++ b/src/or/routerlist.c
@@ -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;
diff --git a/src/or/routerparse.c b/src/or/routerparse.c
index f1895ce31..e25860a67 100644
--- a/src/or/routerparse.c
+++ b/src/or/routerparse.c
@@ -3781,7 +3781,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);
}
}
diff --git a/src/test/test_controller.c b/src/test/test_controller.c
index af19f63f6..1c285bb3a 100644
--- a/src/test/test_controller.c
+++ b/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"
diff --git a/src/test/test_dir.c b/src/test/test_dir.c
index f2223ee17..dc8d2a3bc 100644
--- a/src/test/test_dir.c
+++ b/src/test/test_dir.c
@@ -3965,164 +3965,11 @@ test_dir_packages(void *arg)
}
static void
-test_dir_download_status_schedule(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_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,7 +3978,6 @@ 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, max_delay,
current_time);
@@ -4220,18 +4066,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 +4077,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 +4109,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 +4136,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 +5883,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),
More information about the tor-commits
mailing list