diff --git a/doc/TODO b/doc/TODO index b6668ad853..1487f34990 100644 --- a/doc/TODO +++ b/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 diff --git a/doc/dir-spec.txt b/doc/dir-spec.txt index 5e730bafd8..bbce148ed3 100644 --- a/doc/dir-spec.txt +++ b/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 diff --git a/src/or/connection.c b/src/or/connection.c index 7f3f34e8da..8b6a209c0e 100644 --- a/src/or/connection.c +++ b/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 attempt 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. */ diff --git a/src/or/directory.c b/src/or/directory.c index 34fbf5b7ff..a38aada299 100644 --- a/src/or/directory.c +++ b/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); diff --git a/src/or/dirserv.c b/src/or/dirserv.c index cba1374e3e..3a21926a2b 100644 --- a/src/or/dirserv.c +++ b/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 is_v1_object), 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, diff --git a/src/or/or.h b/src/or/or.h index 3c3901d8ae..d8144af52b 100644 --- a/src/or/or.h +++ b/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);