reinstate the most important dead chickens for limiting

impact of directory writes now that we're write limiting.

a few more dead chickens remain.


svn:r9170

doc/TODO

@@ -183,7 +183,8 @@ R   - look into "uncounting" bytes spent on local connections, so
 R   - "bandwidth classes", for incoming vs initiated-here conns,
       and to give dir conns lower priority.
     . Write limiting; separate token bucket for write
-      - preemptively give a 503 to some dir requests
+      o preemptively give a 503 to some v1 dir requests
+      - preemptively give a 503 to some v2 dir requests
       - per-conn write buckets
       - separate config options for read vs write limiting
 

doc/dir-spec.txt

@@ -215,10 +215,10 @@ $Id$
     "accept" exitpattern
     "reject" exitpattern
 
-       These lines, in order, describe the rules that an OR follows when
+       These lines describe the rules that an OR follows when
        deciding whether to allow a new stream to a given address.  The
        'exitpattern' syntax is described below.  The rules are considered in
-       order; if no rule matches, the address will be accept.  For clarity,
+       order; if no rule matches, the address will be accepted.  For clarity,
        the last such entry SHOULD be accept *:* or reject *:*.
 
     "router-signature" NL Signature NL

src/or/connection.c

@@ -1104,19 +1104,22 @@ retry_all_listeners(int force, smartlist_t *replaced_conns,
 extern int global_read_bucket, global_write_bucket;
 
 static int
-connection_bucket_round_robin(int base, int global_bucket, int conn_bucket)
+connection_bucket_round_robin(int base, int priority,
+                              int global_bucket, int conn_bucket)
 {
   int at_most;
+  int num_bytes_high = (priority ? 32 : 16) * base;
+  int num_bytes_low = (priority ? 4 : 2) * base;
 
   /* Do a rudimentary round-robin so one circuit can't hog a connection.
    * Pick at most 32 cells, at least 4 cells if possible, and if we're in
    * the middle pick 1/8 of the available bandwidth. */
   at_most = global_bucket / 8;
   at_most -= (at_most % base); /* round down */
-  if (at_most > 32*base) /* 16 KB */
-    at_most = 32*base;
-  else if (at_most < 4*base) /* 2 KB */
-    at_most = 4*base;
+  if (at_most > num_bytes_high) /* 16 KB, or 8 KB for low-priority */
+    at_most = num_bytes_high;
+  else if (at_most < num_bytes_low) /* 2 KB, or 1 KB for low-priority */
+    at_most = num_bytes_low;
 
   if (at_most > global_bucket)
     at_most = global_bucket;
@@ -1135,12 +1138,14 @@ connection_bucket_read_limit(connection_t *conn)
 {
   int base = connection_speaks_cells(conn) ?
                CELL_NETWORK_SIZE : RELAY_PAYLOAD_SIZE;
+  int priority = conn->type != CONN_TYPE_DIR;
   int conn_bucket = -1;
   if (connection_speaks_cells(conn) && conn->state == OR_CONN_STATE_OPEN) {
     or_connection_t *or_conn = TO_OR_CONN(conn);
     conn_bucket = or_conn->read_bucket;
   }
-  return connection_bucket_round_robin(base, global_read_bucket, conn_bucket);
+  return connection_bucket_round_robin(base, priority,
+                                       global_read_bucket, conn_bucket);
 }
 
 /** How many bytes at most can we write onto this connection? */
@@ -1149,17 +1154,44 @@ connection_bucket_write_limit(connection_t *conn)
 {
   int base = connection_speaks_cells(conn) ?
                CELL_NETWORK_SIZE : RELAY_PAYLOAD_SIZE;
+  int priority = conn->type != CONN_TYPE_DIR;
 
-  return connection_bucket_round_robin(base, global_write_bucket,
+  return connection_bucket_round_robin(base, priority, global_write_bucket,
                                        conn->outbuf_flushlen);
 }
 
-/** Return 1 if the global write bucket has no bytes in it,
- * or return 0 if it does. */
+/** Return 1 if the global write bucket is low enough that we shouldn't
+ * send <b>attempt</b> bytes of low-priority directory stuff out.
+ * Else return 0.
+
+ * Priority is 1 for v1 requests (directories and running-routers),
+ * and 2 for v2 requests (statuses and descriptors). But see FFFF in
+ * directory_handle_command_get() for why we don't use priority 2 yet.
+ *
+ * There are a lot of parameters we could use here:
+ * - global_write_bucket. Low is bad.
+ * - bandwidthrate. Low is bad.
+ * - bandwidthburst. Not a big factor?
+ * - attempt. High is bad.
+ * - total bytes queued on outbufs. High is bad. But I'm wary of
+ *   using this, since a few slow-flushing queues will pump up the
+ *   number without meaning what we meant to mean. What we really
+ *   mean is "total directory bytes added to outbufs recently", but
+ *   that's harder to quantify and harder to keep track of.
+ */
 int
-global_write_bucket_empty(void)
+global_write_bucket_low(size_t attempt, int priority)
 {
-  return global_write_bucket <= 0;
+  if (global_write_bucket < (int)attempt) /* not enough space no matter what */
+    return 1;
+
+  if (priority == 1) { /* old-style v1 query */
+    if (global_write_bucket-attempt < 2*get_options()->BandwidthRate)
+      return 1;
+  } else { /* v2 query */
+    /* no further constraints yet */
+  }
+  return 0;
 }
 
 /** We just read num_read onto conn. Decrement buckets appropriately. */

src/or/directory.c

@@ -1510,7 +1510,7 @@ directory_handle_command_get(dir_connection_t *conn, char *headers,
     }
     dlen = deflated ? d->dir_z_len : d->dir_len;
 
-    if (global_write_bucket_empty()) {
+    if (global_write_bucket_low(dlen, 1)) {
       log_info(LD_DIRSERV,
                "Client asked for the mirrored directory, but we've been "
                "writing too many bytes lately. Sending 503 Dir busy.");
@@ -1544,16 +1544,24 @@ directory_handle_command_get(dir_connection_t *conn, char *headers,
       !strcmp(url,"/tor/running-routers.z")) { /* running-routers fetch */
     int deflated = !strcmp(url,"/tor/running-routers.z");
     dlen = dirserv_get_runningrouters(&cp, deflated);
-    note_request(url, dlen);
-    tor_free(url);
     if (!dlen) { /* we failed to create/cache cp */
       write_http_status_line(conn, 503, "Directory unavailable");
       /* try to get a new one now */
       if (!already_fetching_directory(DIR_PURPOSE_FETCH_RUNNING_LIST))
         directory_get_from_dirserver(DIR_PURPOSE_FETCH_RUNNING_LIST, NULL, 1);
+      tor_free(url);
       return 0;
     }
-
+    if (global_write_bucket_low(dlen, 1)) {
+      log_info(LD_DIRSERV,
+               "Client asked for running-routers, but we've been "
+               "writing too many bytes lately. Sending 503 Dir busy.");
+      write_http_status_line(conn, 503, "Directory busy, try again later");
+      tor_free(url);
+      return 0;
+    }
+    note_request(url, dlen);
+    tor_free(url);
     write_http_response_header(conn, dlen,
                  deflated?"application/octet-stream":"text/plain",
                  deflated?"deflate":"identity",
@@ -1562,6 +1570,10 @@ directory_handle_command_get(dir_connection_t *conn, char *headers,
     return 0;
   }
 
+  /* FFFF for status/ and server/ requests, we don't have a good
+   * guess of the length we're going to be writing out, so it's hard
+   * to call global_write_bucket_low(). How to proceed? */
+
   if (!strcmpstart(url,"/tor/status/")) {
     /* v2 network status fetch. */
     size_t url_len = strlen(url);

src/or/dirserv.c

@@ -1101,7 +1101,7 @@ dirserv_set_cached_networkstatus_v2(const char *networkstatus,
 /** Helper: If we're an authority for the right directory version (the
  * directory version is determined by <b>is_v1_object</b>), try to regenerate
  * auth_src as appropriate and return it, falling back to cache_src on
- * failure.  If we're a cache, return cach_src.
+ * failure.  If we're a cache, return cache_src.
  */
 static cached_dir_t *
 dirserv_pick_cached_dir_obj(cached_dir_t *cache_src,

src/or/or.h

@@ -1976,7 +1976,7 @@ int retry_all_listeners(int force, smartlist_t *replaced_conns,
                         smartlist_t *new_conns);
 
 int connection_bucket_write_limit(connection_t *conn);
-int global_write_bucket_empty(void);
+int global_write_bucket_low(size_t attempt, int priority);
 void connection_bucket_init(void);
 void connection_bucket_refill(int seconds_elapsed);