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

6 files changed, 132 insertions, 62 deletions

diff --git a/ChangeLog b/ChangeLog
index ae4e3ca8a..26568e008 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -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
diff --git a/doc/TODO b/doc/TODO
index dd5f108d0..83152fded 100644
--- 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.
diff --git a/src/or/directory.c b/src/or/directory.c
index 274118992..91f538a1c 100644
--- 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
diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c
index b83112941..b747d2797 100644
--- 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;
     });
 
diff --git a/src/or/or.h b/src/or/or.h
index e04b75495..931900091 100644
--- 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
diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index 450e41e33..48ce2f186 100644
--- 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);
   });
 }