r11876@catbus: nickm | 2007-02-22 02:23:13 -0500

 Fix two XXXX012 issues in routerlist.c: a possible performance issue hasnt shown up on any profiles, so unflag it.  Stop warning when we get a router descriptor that we asked for but no longer want: just drop it (if we are not a cache) or cache if (if we are).


svn:r9616

3 files changed, 26 insertions, 19 deletions

diff --git a/ChangeLog b/ChangeLog
index 1b0e70cd6c..095cda696b 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -44,6 +44,10 @@ Changes in version 0.1.2.8-alpha - 2007-??-??
       machines using pthreads. (Patch from coderman.)
     - Fix switched arguments on memset in the implementation of tor_munmap()
       for systems with no mmap() call.
+    - When Tor receives a router descriptor that it asked for, but no longer
+      wants (because it has received fresh networkstatuses in the meantime),
+      do not warn the user.  Cache the descriptor if we're a cache; drop it
+      if we aren't.
 
   o Minor features (controller):
     - Warn the user when an application uses the obsolete binary v0
diff --git a/src/or/directory.c b/src/or/directory.c
index 5a709000ac..b7a5535ca3 100644
--- a/src/or/directory.c
+++ b/src/or/directory.c
@@ -352,9 +352,9 @@ connection_dir_download_routerdesc_failed(dir_connection_t *conn)
   /* No need to increment the failure count for routerdescs, since
    * it's not their fault. */
 
-  /* There's no need to call this here: we already call it every 10 seconds *
-   *  (DESCRIPTOR_RETRY_INTERVAL) in main.c -NM */
-  /* update_router_descriptor_downloads(time(NULL)); */
+  /* There's no relaunch descriptor downloads here: we already do it every 10
+   * seconds (DESCRIPTOR_RETRY_INTERVAL) in main.c */
+
   (void) conn;
 }
 
@@ -2057,9 +2057,8 @@ dir_routerdesc_download_failed(smartlist_t *failed, int status_code)
                 cp, (int)rs->n_download_failures);
   });
 
-  /* There's no need to call this here: we already call it every 10 seconds *
-   *  (DESCRIPTOR_RETRY_INTERVAL) in main.c -NM */
-  /* update_router_descriptor_downloads(time(NULL)); */
+  /* There's no relaunch descriptor downloads here: we already do it every 10
+   * seconds (DESCRIPTOR_RETRY_INTERVAL) in main.c */
 }
 
 /** Given a directory <b>resource</b> request, containing zero
diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index d8b76546b8..f11f1670ea 100644
--- a/src/or/routerlist.c
+++ b/src/or/routerlist.c
@@ -729,8 +729,7 @@ routerlist_add_family(smartlist_t *sl, routerinfo_t *router)
   config_line_t *cl;
   or_options_t *options = get_options();
 
-  /* First, add any routers with similar network addresses.
-   * XXXX012 It's possible this will be really expensive; we'll see. */
+  /* First, add any routers with similar network addresses. */
   if (options->EnforceDistinctSubnets)
     routerlist_add_network_family(sl, router);
 
@@ -1909,19 +1908,19 @@ router_add_to_routerlist(routerinfo_t *router, const char **msg,
   } else if (from_fetch) {
     /* Only check the descriptor digest against the network statuses when
      * we are receiving in response to a fetch. */
-    /* XXXX012 This warning seems to happen fairly regularly when we download
-     * router information based on an old networkstatus, then discard the
-     * networkstatus between requesting the routers and getting the reply.
-     * That's no good at all.  I think we should switch to a behavior where we
-     * don't download a descriptor unless it's in a _recent_ networkstatus;
-     * also, we should drop this warning in (hopefully less likely) case where
-     * we decide we don't want a descriptor after we start downloading
-     * it. -NM */
+
     if (!signed_desc_digest_is_recognized(&router->cache_info)) {
-      log_warn(LD_DIR, "Dropping unrecognized descriptor for router '%s'",
+      /* We asked for it, so some networkstatus must have listed it when we
+       * did.  save it in case we're a cache and somebody else asks for it. */
+      log_info(LD_DIR,
+               "Received a no-longer-recognized descriptor for router '%s'",
                router->nickname);
       *msg = "Router descriptor is not referenced by any network-status.";
-      routerinfo_free(router);
+
+      /* Only journal this desc if we'll be serving it. */
+      if (!from_cache && get_options()->DirPort)
+        router_append_to_journal(&router->cache_info);
+      routerlist_insert_old(routerlist, router);
       return -1;
     }
   }
@@ -4029,7 +4028,12 @@ update_router_descriptor_cache_downloads(time_t now)
   n_download = 0;
   SMARTLIST_FOREACH(networkstatus_list, networkstatus_t *, ns,
     {
-      smartlist_t *dl = smartlist_create();
+      smartlist_t *dl;
+      if (ns->published_on + MAX_NETWORKSTATUS_AGE-10*60 > now) {
+        /* Don't download if the networkstatus is almost ancient. */
+        continue;
+      }
+      dl = smartlist_create();
       downloadable[ns_sl_idx] = dl;
       download_from[ns_sl_idx] = smartlist_create();
       SMARTLIST_FOREACH(ns->entries, routerstatus_t * , rs,