r14813@Kushana: nickm | 2007-10-09 11:10:48 -0400

Use download_status_t for v2 networkstatuses and certs as well as for routers. Make functions to manipulate it. Next steps: use it for consensus networkstatuses, and get consensus download rescheduling working. svn:r11800
2024-11-27 22:03:31 +01:00 · 2007-10-09 15:27:45 +00:00 · 2007-10-09 15:27:45 +00:00 · 5346a01796
commit 5346a01796
parent bb9cc4fb29
6 changed files with 132 additions and 62 deletions
--- a/1
+++ b/1
@ -55,6 +55,7 @@ Changes in version 0.2.0.8-alpha - 2007-??-??
      into routerstatus_t.  But once v3 directories are in use, clients and
      caches will no longer need to hold authority opinions; thus, the
      rationale for keeping the types separate is now gone.
+    - Make the code used to reschedule and reattempt downloads more uniform.


 Changes in version 0.2.0.7-alpha - 2007-09-21
--- a/doc/TODO
+++ b/doc/TODO
@ -56,7 +56,9 @@ Things we'd like to do in 0.2.0.x:
        o Code to manage key certificates
          o Download as needed.
            o Code to download
-            D Code to retry download.
+            . Code to retry download.
+              o Schedule download times on failure
+              - Reattempt downloads periodically
        o Code to generate consensus from a list of votes
          o Detect whether votes are really all for the same period.
        o Push/pull documents as appropriate.
--- a/src/or/directory.c
+++ b/src/or/directory.c
@ -45,6 +45,8 @@ static void http_set_address_origin(const char *headers, connection_t *conn);
 static void connection_dir_download_networkstatus_failed(
                               dir_connection_t *conn, int status_code);
 static void connection_dir_download_routerdesc_failed(dir_connection_t *conn);
+static void connection_dir_download_cert_failed(
+                               dir_connection_t *conn, int status_code);
 static void dir_networkstatus_download_failed(smartlist_t *failed,
                                              int status_code);
 static void dir_routerdesc_download_failed(smartlist_t *failed,
@ -499,7 +501,9 @@ connection_dir_request_failed(dir_connection_t *conn)
  } else if (conn->_base.purpose == DIR_PURPOSE_FETCH_CONSENSUS) {
    /* XXXX020 NMNM */
  } else if (conn->_base.purpose == DIR_PURPOSE_FETCH_CERTIFICATE) {
-    /* XXXX020 NMNM */
+    log_info(LD_DIR, "Giving up on directory server at '%s'; retrying",
+             conn->_base.address);
+    connection_dir_download_cert_failed(conn, 0);
  } else {
    /* XXXX020 handle failing: votes. signatures. */
  }
@ -525,7 +529,7 @@ connection_dir_download_networkstatus_failed(dir_connection_t *conn,
     * if all the authorities are shutting us out. */
    smartlist_t *trusted_dirs = router_get_trusted_dir_servers();
    SMARTLIST_FOREACH(trusted_dirs, trusted_dir_server_t *, ds,
-                      ++ds->n_networkstatus_failures);
+                      download_status_failed(&ds->v2_ns_dl_status, 0));
    directory_get_from_dirserver(conn->_base.purpose, conn->router_purpose,
                                 "all.z", 0 /* don't retry_if_no_servers */);
  } else if (!strcmpstart(conn->requested_resource, "fp/")) {
@ -559,6 +563,28 @@ connection_dir_download_routerdesc_failed(dir_connection_t *conn)
  (void) conn;
 }

+/** Called when an attempt to fetch a certificate fails. */
+static void
+connection_dir_download_cert_failed(dir_connection_t *conn, int status)
+{
+  smartlist_t *failed;
+  tor_assert(conn->_base.purpose == DIR_PURPOSE_FETCH_CERTIFICATE);
+
+  if (!conn->requested_resource)
+    return;
+  failed = smartlist_create();
+  dir_split_resource_into_fingerprints(conn->requested_resource,
+                                       failed, NULL, 1, 0);
+  SMARTLIST_FOREACH(failed, char *, cp,
+  {
+    trusted_dir_server_t *dir = trusteddirserver_get_by_v3_auth_digest(cp);
+    if (dir)
+      download_status_failed(&dir->cert_dl_status, status);
+    tor_free(cp);
+  });
+  smartlist_free(failed);
+}
+
 /** Helper for directory_initiate_command_(router|trusted_dir): send the
 * command to a server whose address is <b>address</b>, whose IP is
 * <b>addr</b>, whose directory port is <b>dir_port</b>, whose tor version
@ -1406,8 +1432,8 @@ connection_dir_client_reached_eof(dir_connection_t *conn)
          "'%s:%d' while fetching \"/tor/keys/%s\".",
           status_code, escaped(reason), conn->_base.address,
           conn->_base.port, conn->requested_resource);
+      connection_dir_download_cert_failed(conn, status_code);
      tor_free(body); tor_free(headers); tor_free(reason);
-      /* XXXX020NMNM retry. */
      return -1;
    }
    log_info(LD_DIR,"Received authority certificatess (size %d) from server "
@ -2660,16 +2686,69 @@ dir_networkstatus_download_failed(smartlist_t *failed, int status_code)
    dir = router_get_trusteddirserver_by_digest(digest);

    if (dir)
-      ++dir->n_networkstatus_failures;
+      download_status_failed(&dir->v2_ns_dl_status, status_code);
  });
 }

+/** DOCDOC */
+time_t
+download_status_increment_failure(download_status_t *dls, int status_code,
+                                  const char *item, int server, time_t now)
+{
+  tor_assert(dls);
+  if (status_code != 503 || server)
+    ++dls->n_download_failures;
+  if (server) {
+    switch (dls->n_download_failures) {
+      case 0: dls->next_attempt_at = 0; break;
+      case 1: dls->next_attempt_at = 0; break;
+      case 2: dls->next_attempt_at = 0; break;
+      case 3: dls->next_attempt_at = now+60; break;
+      case 4: dls->next_attempt_at = now+60; break;
+      case 5: dls->next_attempt_at = now+60*2; break;
+      case 6: dls->next_attempt_at = now+60*5; break;
+      case 7: dls->next_attempt_at = now+60*15; break;
+      default: dls->next_attempt_at = TIME_MAX; break;
+    }
+  } else {
+    switch (dls->n_download_failures) {
+      case 0: dls->next_attempt_at = 0; break;
+      case 1: dls->next_attempt_at = 0; break;
+      case 2: dls->next_attempt_at = now+60; break;
+      case 3: dls->next_attempt_at = now+60*5; break;
+      case 4: dls->next_attempt_at = now+60*10; break;
+      default: dls->next_attempt_at = TIME_MAX; break;
+    }
+  }
+  if (item) {
+    if (dls->next_attempt_at == 0)
+      log_debug(LD_DIR, "%s failed %d time(s); I'll try again immediately.",
+                item, (int)dls->n_download_failures);
+    else if (dls->next_attempt_at < TIME_MAX)
+      log_debug(LD_DIR, "%s failed %d time(s); I'll try again in %d seconds.",
+                item, (int)dls->n_download_failures,
+                (int)(dls->next_attempt_at-now));
+    else
+      log_debug(LD_DIR, "%s failed %d time(s); Giving up for a while.",
+                item, (int)dls->n_download_failures);
+  }
+  return dls->next_attempt_at;
+}
+
+/** DOCDOC */
+void
+download_status_reset(download_status_t *dls)
+{
+  dls->n_download_failures = 0;
+  dls->next_attempt_at = 0;
+}
+
 /** Called when one or more routerdesc (or extrainfo, if <b>was_extrainfo</b>)
 * fetches have failed (with uppercase fingerprints listed in <b>failed</b>,
 * either as descriptor digests or as identity digests based on
 * <b>was_descriptor_digests</b>).
 */
-static void
+void
 dir_routerdesc_download_failed(smartlist_t *failed, int status_code,
                               int was_extrainfo, int was_descriptor_digests)
 {
@ -2695,40 +2774,7 @@ dir_routerdesc_download_failed(smartlist_t *failed, int status_code,
    }
    if (!dls || dls->n_download_failures >= MAX_ROUTERDESC_DOWNLOAD_FAILURES)
      continue;
-    if (status_code != 503 || server)
-      ++dls->n_download_failures;
-    if (server) {
-      switch (dls->n_download_failures) {
-        case 0: dls->next_attempt_at = 0; break;
-        case 1: dls->next_attempt_at = 0; break;
-        case 2: dls->next_attempt_at = 0; break;
-        case 3: dls->next_attempt_at = now+60; break;
-        case 4: dls->next_attempt_at = now+60; break;
-        case 5: dls->next_attempt_at = now+60*2; break;
-        case 6: dls->next_attempt_at = now+60*5; break;
-        case 7: dls->next_attempt_at = now+60*15; break;
-        default: dls->next_attempt_at = TIME_MAX; break;
-      }
-    } else {
-      switch (dls->n_download_failures) {
-        case 0: dls->next_attempt_at = 0; break;
-        case 1: dls->next_attempt_at = 0; break;
-        case 2: dls->next_attempt_at = now+60; break;
-        case 3: dls->next_attempt_at = now+60*5; break;
-        case 4: dls->next_attempt_at = now+60*10; break;
-        default: dls->next_attempt_at = TIME_MAX; break;
-      }
-    }
-    if (dls->next_attempt_at == 0)
-      log_debug(LD_DIR, "%s failed %d time(s); I'll try again immediately.",
-                cp, (int)dls->n_download_failures);
-    else if (dls->next_attempt_at < TIME_MAX)
-      log_debug(LD_DIR, "%s failed %d time(s); I'll try again in %d seconds.",
-                cp, (int)dls->n_download_failures,
-                (int)(dls->next_attempt_at-now));
-    else
-      log_debug(LD_DIR, "%s failed %d time(s); Giving up for a while.",
-                cp, (int)dls->n_download_failures);
+    download_status_increment_failure(dls, status_code, cp, server, now);
  });

  /* No need to relaunch descriptor downloads here: we already do it
--- a/src/or/networkstatus.c
+++ b/src/or/networkstatus.c
@ -382,7 +382,7 @@ router_set_networkstatus(const char *s, time_t arrived_at,
          }
          old_ns->received_on = arrived_at;
        }
-        ++trusted_dir->n_networkstatus_failures;
+        download_status_failed(&trusted_dir->v2_ns_dl_status, 0);
        return 0;
      } else if (old_ns->published_on >= ns->published_on) {
        char old_published[ISO_TIME_LEN+1];
@ -394,7 +394,7 @@ router_set_networkstatus(const char *s, time_t arrived_at,
                 trusted_dir->description, published,
                 old_published);
        networkstatus_free(ns);
-        ++trusted_dir->n_networkstatus_failures;
+        download_status_failed(&trusted_dir->v2_ns_dl_status, 0);
        return 0;
      } else {
        networkstatus_free(old_ns);
@ -405,8 +405,9 @@ router_set_networkstatus(const char *s, time_t arrived_at,
    }
  }

-  if (source != NS_FROM_CACHE && trusted_dir)
-    trusted_dir->n_networkstatus_failures = 0;
+  if (source != NS_FROM_CACHE && trusted_dir) {
+    download_status_reset(&trusted_dir->v2_ns_dl_status);
+  }

  if (!found)
    smartlist_add(networkstatus_list, ns);
@ -737,7 +738,8 @@ update_networkstatus_client_downloads(time_t now)
       if (!(ds->type & V2_AUTHORITY))
         continue;
       ++n_dirservers;
-       if (ds->n_networkstatus_failures > NETWORKSTATUS_N_ALLOWABLE_FAILURES)
+       if (!download_status_is_ready(&ds->v2_ns_dl_status, now,
+                                     NETWORKSTATUS_N_ALLOWABLE_FAILURES))
         continue;
       ++n_running_dirservers;
       if (ns && ns->published_on > now-NETWORKSTATUS_MAX_AGE)
@ -791,7 +793,8 @@ update_networkstatus_client_downloads(time_t now)
        smartlist_free(missing);
        return;
      }
-      if (ds->n_networkstatus_failures > NETWORKSTATUS_N_ALLOWABLE_FAILURES) {
+      if (ds->v2_ns_dl_status.n_download_failures >
+          NETWORKSTATUS_N_ALLOWABLE_FAILURES) {
        ++n_failed;
        continue;
      }
@ -1584,11 +1587,10 @@ routers_update_status_from_networkstatus(smartlist_t *routers,
      router->is_bad_exit = rs->is_bad_exit;
    }
    if (router->is_running && ds) {
-      ds->n_networkstatus_failures = 0;
+      download_status_reset(&ds->v2_ns_dl_status);
    }
    if (reset_failures) {
-      rs->dl_status.n_download_failures = 0;
-      rs->dl_status.next_attempt_at = 0;
+      download_status_reset(&rs->dl_status);
    }
  });
  router_dir_info_changed();
@ -1608,7 +1610,7 @@ have_tried_downloading_all_statuses(int n_failures)
      /* If we don't have the status, and we haven't failed to get the status,
       * we haven't tried to get the status. */
      if (!networkstatus_get_by_digest(ds->digest) &&
-          ds->n_networkstatus_failures <= n_failures)
+          ds->v2_ns_dl_status.n_download_failures <= n_failures)
        return 0;
    });

--- a/src/or/or.h
+++ b/src/or/or.h
@ -2801,6 +2801,25 @@ int dir_split_resource_into_fingerprints(const char *resource,
 char *directory_dump_request_log(void);
 int router_supports_extrainfo(const char *identity_digest, int is_authority);

+time_t download_status_increment_failure(download_status_t *dls,
+                                         int status_code, const char *item,
+                                         int server, time_t now);
+#define download_status_failed(dls, sc)                                 \
+  download_status_increment_failure((dls), (sc), NULL,                  \
+                                    get_options()->DirPort, time(NULL))
+
+void download_status_reset(download_status_t *dls);
+/** DOCDOC */
+static int download_status_is_ready(download_status_t *dls, time_t now,
+                                    int max_failures);
+static INLINE int
+download_status_is_ready(download_status_t *dls, time_t now,
+                         int max_failures)
+{
+  return (dls->n_download_failures <= max_failures
+          && dls->next_attempt_at <= now);
+}
+
 /********************************* dirserv.c ***************************/

 #define UNNAMED_ROUTER_NICKNAME "Unnamed"
@ -3458,9 +3477,11 @@ typedef struct trusted_dir_server_t {
  authority_type_t type;

  smartlist_t *v3_certs; /**< V3 key certificates for this authority */
+  download_status_t cert_dl_status; /**< Status of downloading this server's
+                               * latest certificate. */
+  download_status_t v2_ns_dl_status; /**< Status of downloading this server's
+                               * v2 network status. */

-  int n_networkstatus_failures; /**< How many times have we asked for this
-                                 * server's network-status unsuccessfully? */
  routerstatus_t fake_status; /**< Used when we need to pass this trusted
                               * dir_server_t to directory_initiate_command_*
                               * as a routerstatus_t.  Not updated by the
--- a/src/or/routerlist.c
+++ b/src/or/routerlist.c
@ -967,8 +967,7 @@ mark_all_trusteddirservers_up(void)
    {
      routerstatus_t *rs;
      dir->is_running = 1;
-      dir->n_networkstatus_failures = 0;
-      dir->fake_status.last_dir_503_at = 0;
+      download_status_reset(&dir->v2_ns_dl_status);
      rs = router_get_combined_status_by_digest(dir->digest);
      if (rs && !rs->is_running) {
        rs->is_running = 1;
@ -3486,7 +3485,8 @@ router_list_client_downloadable(void)
      /* Oddly, we have a descriptor more recent than the 'best' one, but it
         was once best. So that's okay. */
      ++n_uptodate;
-    } else if (rs->dl_status.next_attempt_at > now) {
+    } else if (!download_status_is_ready(&rs->dl_status, now,
+                                         MAX_ROUTERDESC_DOWNLOAD_FAILURES)) {
      /* We failed too recently to try again. */
      ++n_not_ready;
    } else {
@ -3837,7 +3837,8 @@ should_download_extrainfo(signed_descriptor_t *sd,
  const char *d = sd->extra_info_digest;
  return (!sd->is_extrainfo &&
          !tor_digest_is_zero(d) &&
-          sd->ei_dl_status.next_attempt_at <= now &&
+          download_status_is_ready(&sd->ei_dl_status, now,
+                                   MAX_ROUTERDESC_DOWNLOAD_FAILURES) &&
          !eimap_get(rl->extra_info_map, d) &&
          !digestmap_get(pending, d));
 }
@ -3997,8 +3998,7 @@ router_reset_descriptor_download_failures(void)
  const smartlist_t *routerstatus_list = networkstatus_get_all_statuses();
  SMARTLIST_FOREACH(routerstatus_list, routerstatus_t *, rs,
  {
-    rs->dl_status.n_download_failures = 0;
-    rs->dl_status.next_attempt_at = 0;
+    download_status_reset(&rs->dl_status);
  });
  SMARTLIST_FOREACH(networkstatus_list, networkstatus_t *, ns,
     SMARTLIST_FOREACH(ns->entries, routerstatus_t *, rs,
@ -4011,13 +4011,11 @@ router_reset_descriptor_download_failures(void)
    return;
  SMARTLIST_FOREACH(routerlist->routers, routerinfo_t *, ri,
  {
-    ri->cache_info.ei_dl_status.n_download_failures = 0;
-    ri->cache_info.ei_dl_status.next_attempt_at = 0;
+    download_status_reset(&ri->cache_info.ei_dl_status);
  });
  SMARTLIST_FOREACH(routerlist->old_routers, signed_descriptor_t *, sd,
  {
-    sd->ei_dl_status.n_download_failures = 0;
-    sd->ei_dl_status.next_attempt_at = 0;
+    download_status_reset(&sd->ei_dl_status);
  });
 }