[tor-commits] [tor/master] Refactor responsibility for checking global write bucket emptiness

[nickm at torproject.org]

commit 9af4cd6f31dd9cbfaee526f2042aa7fa501338ef
Author: Nick Mathewson <nickm at 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);