commit 9af4cd6f31dd9cbfaee526f2042aa7fa501338ef Author: Nick Mathewson <nickm@torproject.org> Date: Tue Apr 17 12:05:39 2018 -0400 Refactor responsibility for checking global write bucket emptiness We used to do this 10x per second in connection_buckets_refill(); instead, we now do it when the bucket becomes empty. This change is part of the work of making connection_buckets_refill() obsolete. Closes ticket 25828; bugfix on 0.2.3.5-alpha. --- changes/bug25828 | 7 +++++++ src/or/connection.c | 28 ++++++++++++++++------------ 2 files changed, 23 insertions(+), 12 deletions(-) diff --git a/changes/bug25828 b/changes/bug25828 new file mode 100644 index 000000000..45cd1f4ae --- /dev/null +++ b/changes/bug25828 @@ -0,0 +1,7 @@ + o Minor bugfixes (bandwidth management): + - Consider ourselves "low on write bandwidth" if we have exhausted our + write bandwidth some time in the last second. This was the + documented behavior before, but the actual behavior was to change + this value every TokenBucketRefillInterval. Fixes bug 25828; bugfix on + 0.2.3.5-alpha. + diff --git a/src/or/connection.c b/src/or/connection.c index 631d665af..062c85c7f 100644 --- a/src/or/connection.c +++ b/src/or/connection.c @@ -2814,10 +2814,10 @@ connection_is_rate_limited(connection_t *conn) return 1; } -/** Did either global write bucket run dry last second? If so, - * we are likely to run dry again this second, so be stingy with the - * tokens we just put in. */ -static int write_buckets_empty_last_second = 0; +/** When was either global write bucket last empty? If this was recent, then + * we're probably low on bandwidth, and we should be stingy with our bandwidth + * usage. */ +static time_t write_buckets_last_empty_at = -100; /** How many seconds of no active local circuits will make the * connection revert to the "relayed" bandwidth class? */ @@ -2969,8 +2969,11 @@ global_write_bucket_low(connection_t *conn, size_t attempt, int priority) if (smaller_bucket < attempt) return 1; /* not enough space no matter the priority */ - if (write_buckets_empty_last_second) - return 1; /* we're already hitting our limits, no more please */ + { + const time_t diff = approx_time() - write_buckets_last_empty_at; + if (diff <= 1) + return 1; /* we're already hitting our limits, no more please */ + } if (priority == 1) { /* old-style v1 query */ /* Could we handle *two* of these requests within the next two seconds? */ @@ -3042,10 +3045,15 @@ connection_buckets_decrement(connection_t *conn, time_t now, if (!connection_is_rate_limited(conn)) return; /* local IPs are free */ + unsigned flags = 0; if (connection_counts_as_relayed_traffic(conn, now)) { - token_bucket_rw_dec(&global_relayed_bucket, num_read, num_written); + flags = token_bucket_rw_dec(&global_relayed_bucket, num_read, num_written); + } + flags |= token_bucket_rw_dec(&global_bucket, num_read, num_written); + + if (flags & TB_WRITE) { + write_buckets_last_empty_at = now; } - token_bucket_rw_dec(&global_bucket, num_read, num_written); if (connection_speaks_cells(conn) && conn->state == OR_CONN_STATE_OPEN) { or_connection_t *or_conn = TO_OR_CONN(conn); token_bucket_rw_dec(&or_conn->bucket, num_read, num_written); @@ -3185,10 +3193,6 @@ connection_bucket_refill_all(time_t now, uint32_t now_ts) { smartlist_t *conns = get_connection_array(); - write_buckets_empty_last_second = - token_bucket_rw_get_write(&global_bucket) <= 0 || - token_bucket_rw_get_write(&global_relayed_bucket) <= 0; - /* refill the global buckets */ token_bucket_rw_refill(&global_bucket, now_ts); token_bucket_rw_refill(&global_relayed_bucket, now_ts);