r11677@Kushana: nickm | 2006-12-23 21:17:54 -0500

 Track when we get 503s from directory servers.  Do not use directory servers that have given us a 503 until either an hour has passed, or we are are out of non-busy servers.


svn:r9172

5 files changed, 63 insertions, 16 deletions

diff --git a/ChangeLog b/ChangeLog
index 41ef9c4adf..32c5cd05d0 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -39,6 +39,10 @@ Changes in version 0.1.2.5-xxxx - 200?-??-??
     - DirServer configuration lines now have an orport option so clients can
       open encrypted tunnels to the authorities without having downloaded
       their descriptors yet.
+    - Clients track responses with status 503 from dirservers.  After a
+      dirserver has given us a 503, we try not to use it until an hour
+      has gone by, or until we have no dirservers that haven't given us
+      a 503.
 
   o Security bugfixes:
     - Stop sending the HttpProxyAuthenticator string to directory
diff --git a/doc/TODO b/doc/TODO
index 57cf6e565b..d87a7a683e 100644
--- a/doc/TODO
+++ b/doc/TODO
@@ -134,12 +134,16 @@ R o Take out the '5 second' timeout from the socks detach schedule.
 N     - they don't count toward the 3-strikes rule
         - should there be some threshold of 503's after which we give up?
         - Delay when we get a lot of 503s?
-N     - split "router is down" from "dirport shouldn't be tried for a while"?
+      o split "router is down" from "dirport shouldn't be tried for a while"?
         We want a field to hold "when did we last get a 503 from this
         directory server."  Probably, it should go in local_routerstatus_t,
         not in routerinfo_t, since we can try to use servers as directories
         before we have their descriptors.  Possibly, it should also go in
         trusted_dir_server_t.
+        o Add a last_dir_503_at field.
+        o Have it get updated correctly.
+        o Prefer to use directories that haven't given us a 503 for the last
+          60 minutes.
       - authorities should *never* 503 a cache, and should never 503
         network status requests. They can 503 client descriptor requests
         when they feel like it.
diff --git a/src/or/directory.c b/src/or/directory.c
index a38aada299..78a4f23a62 100644
--- a/src/or/directory.c
+++ b/src/or/directory.c
@@ -109,7 +109,7 @@ directory_post_to_dirservers(uint8_t purpose, const char *payload,
    */
   SMARTLIST_FOREACH(dirservers, trusted_dir_server_t *, ds,
     {
-      routerstatus_t *rs = &(ds->fake_status);
+      routerstatus_t *rs = &(ds->fake_status.status);
       if (post_to_hidserv_only && !ds->is_hidserv_authority)
         continue;
       if (!post_to_hidserv_only &&
@@ -921,10 +921,18 @@ connection_dir_client_reached_eof(dir_connection_t *conn)
   (void) skewed; /* skewed isn't used yet. */
 
   if (status_code == 503) {
+    local_routerstatus_t *rs;
+    trusted_dir_server_t *ds;
+    time_t now = time(NULL);
     log_info(LD_DIR,"Received http status code %d (%s) from server "
              "'%s:%d'. I'll try again soon.",
              status_code, escaped(reason), conn->_base.address,
              conn->_base.port);
+    if ((rs = router_get_combined_status_by_digest(conn->identity_digest)))
+      rs->last_dir_503_at = now;
+    if ((ds = router_get_trusteddirserver_by_digest(conn->identity_digest)))
+      ds->fake_status.last_dir_503_at = now;
+
     tor_free(body); tor_free(headers); tor_free(reason);
     return -1;
   }
diff --git a/src/or/or.h b/src/or/or.h
index a1847ed05b..1691098183 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -1028,7 +1028,10 @@ typedef struct local_routerstatus_t {
    * descriptor_digest represents the descriptor we would most like to use for
    * this router. */
   routerstatus_t status;
-  time_t next_attempt_at; /**< When should we try this descriptor again? */
+  time_t next_attempt_at; /**< When should we try downloading this descriptor
+                           * again? */
+  time_t last_dir_503_at; /**< When did this router last tell us that it
+                           * was too busy to serve directory info? */
   uint8_t n_download_failures; /**< Number of failures trying to download the
                                 * most recent descriptor. */
   unsigned int name_lookup_warned:1; /**< Have we warned the user for referring
@@ -2675,7 +2678,7 @@ typedef struct trusted_dir_server_t {
 
   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
+  local_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
                                * router-status management code!
diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index 9ea916744f..c7b6dd0f62 100644
--- a/src/or/routerlist.c
+++ b/src/or/routerlist.c
@@ -499,6 +499,9 @@ router_pick_trusteddirserver(authority_type_t type,
                                            requireother, fascistfirewall);
 }
 
+/** How long do we avoid using a directory server after it's given us a 503? */
+#define DIR_503_TIMEOUT (60*60)
+
 /** Pick a random running valid directory server/mirror from our
  * routerlist.  Don't pick an authority if any non-authorities are viable.
  * If <b>fascistfirewall</b>,
@@ -513,13 +516,16 @@ router_pick_directory_server_impl(int requireother, int fascistfirewall,
 {
   routerstatus_t *result;
   smartlist_t *sl;
+  smartlist_t *overloaded;
   smartlist_t *trusted;
+  time_t now = time(NULL);
 
   if (!routerstatus_list)
     return NULL;
 
   /* Find all the running dirservers we know about. */
   sl = smartlist_create();
+  overloaded = smartlist_create();
   trusted = smartlist_create();
   SMARTLIST_FOREACH(routerstatus_list, local_routerstatus_t *, _local_status,
   {
@@ -536,14 +542,23 @@ router_pick_directory_server_impl(int requireother, int fascistfirewall,
     is_trusted = router_digest_is_trusted_dir(status->identity_digest);
     if (for_v2_directory && !(status->is_v2_dir || is_trusted))
       continue;
-    smartlist_add(is_trusted ? trusted : sl, status);
+    if (is_trusted) {
+      smartlist_add(trusted, status);
+    } else if (_local_status->last_dir_503_at + DIR_503_TIMEOUT > now) {
+      smartlist_add(overloaded, status);
+    } else {
+      smartlist_add(sl, status);
+    }
   });
 
   if (smartlist_len(sl))
     result = smartlist_choose(sl);
+  else if (smartlist_len(overloaded))
+    result = smartlist_choose(overloaded);
   else
     result = smartlist_choose(trusted);
   smartlist_free(sl);
+  smartlist_free(overloaded);
   smartlist_free(trusted);
   return result;
 }
@@ -559,9 +574,12 @@ router_pick_trusteddirserver_impl(authority_type_t type,
                                   int requireother, int fascistfirewall)
 {
   smartlist_t *sl;
+  smartlist_t *overloaded;
   routerinfo_t *me;
   routerstatus_t *rs;
+  time_t now = time(NULL);
   sl = smartlist_create();
+  overloaded = smartlist_create();
   me = router_get_my_routerinfo();
 
   if (!trusted_dir_servers)
@@ -582,11 +600,18 @@ router_pick_trusteddirserver_impl(authority_type_t type,
         if (!fascist_firewall_allows_address_dir(d->addr, d->dir_port))
           continue;
       }
-      smartlist_add(sl, &d->fake_status);
+      if (d->fake_status.last_dir_503_at + DIR_503_TIMEOUT > now)
+        smartlist_add(overloaded, &d->fake_status.status);
+      else
+        smartlist_add(sl, &d->fake_status.status);
     });
 
-  rs = smartlist_choose(sl);
+  if (smartlist_len(sl))
+    rs = smartlist_choose(sl);
+  else
+    rs = smartlist_choose(overloaded);
   smartlist_free(sl);
+  smartlist_free(overloaded);
   return rs;
 }
 
@@ -607,9 +632,11 @@ mark_all_trusteddirservers_up(void)
       local_routerstatus_t *rs;
       dir->is_running = 1;
       dir->n_networkstatus_failures = 0;
+      dir->fake_status.last_dir_503_at = 0;
       rs = router_get_combined_status_by_digest(dir->digest);
       if (rs && !rs->status.is_running) {
         rs->status.is_running = 1;
+        rs->last_dir_503_at = 0;
         control_event_networkstatus_changed_single(rs);
       }
     });
@@ -2637,7 +2664,7 @@ update_networkstatus_cache_downloads(time_t now)
          base16_encode(resource+3, sizeof(resource)-3, ds->digest, DIGEST_LEN);
          strlcat(resource, ".z", sizeof(resource));
          directory_initiate_command_routerstatus(
-               &ds->fake_status, DIR_PURPOSE_FETCH_NETWORKSTATUS,
+               &ds->fake_status.status, DIR_PURPOSE_FETCH_NETWORKSTATUS,
                0, /* Not private */
                resource,
                NULL, 0 /* No payload. */);
@@ -2869,15 +2896,15 @@ add_trusted_dir_server(const char *nickname, const char *address,
     tor_snprintf(ent->description, dlen, "directory server at %s:%d",
                  hostname, (int)dir_port);
 
-  ent->fake_status.addr = ent->addr;
-  memcpy(ent->fake_status.identity_digest, digest, DIGEST_LEN);
+  ent->fake_status.status.addr = ent->addr;
+  memcpy(ent->fake_status.status.identity_digest, digest, DIGEST_LEN);
   if (nickname)
-    strlcpy(ent->fake_status.nickname, nickname,
-            sizeof(ent->fake_status.nickname));
+    strlcpy(ent->fake_status.status.nickname, nickname,
+            sizeof(ent->fake_status.status.nickname));
   else
-    ent->fake_status.nickname[0] = '\0';
-  ent->fake_status.dir_port = ent->dir_port;
-  ent->fake_status.or_port = ent->or_port;
+    ent->fake_status.status.nickname[0] = '\0';
+  ent->fake_status.status.dir_port = ent->dir_port;
+  ent->fake_status.status.or_port = ent->or_port;
 
   smartlist_add(trusted_dir_servers, ent);
   router_dir_info_changed();
@@ -3427,6 +3454,7 @@ routerstatus_list_update_from_networkstatus(time_t now)
         rs_out->next_attempt_at = rs_old->next_attempt_at;
       }
       rs_out->name_lookup_warned = rs_old->name_lookup_warned;
+      rs_out->last_dir_503_at = rs_old->last_dir_503_at;
     }
     smartlist_add(result, rs_out);
     log_debug(LD_DIR, "Router '%s' is listed by %d/%d directories, "
@@ -3900,7 +3928,7 @@ update_router_descriptor_cache_downloads(time_t now)
     log_info(LD_DIR, "Requesting %d descriptors from authority \"%s\"",
              smartlist_len(dl), ds->nickname);
     for (j=0; j < smartlist_len(dl); j += MAX_DL_PER_REQUEST) {
-      initiate_descriptor_downloads(&(ds->fake_status), dl, j,
+      initiate_descriptor_downloads(&(ds->fake_status.status), dl, j,
                                     j+MAX_DL_PER_REQUEST);
     }
   }