Clean up proposal 166 and its implementation.

7 files changed, 24 insertions, 26 deletions

diff --git a/doc/spec/proposals/166-statistics-extra-info-docs.txt b/doc/spec/proposals/166-statistics-extra-info-docs.txt
index 3716c049cc..6bc71cafd8 100644
--- a/doc/spec/proposals/166-statistics-extra-info-docs.txt
+++ b/doc/spec/proposals/166-statistics-extra-info-docs.txt
@@ -298,7 +298,7 @@ Exit statistics:
 
   The last type of statistics affects exit nodes counting the number of
   bytes written and read and the number of streams opened per port and
-  per 24 hours. Exit port statistics can be measured from looking of
+  per 24 hours. Exit port statistics can be measured from looking at
   headers of BEGIN and DATA cells. A BEGIN cell contains the exit port
   that is required for the exit node to open a new exit stream.
   Subsequent DATA cells coming from the client or being sent back to the
@@ -361,7 +361,7 @@ Implementation notes:
      basically means renaming keywords.
 
   2. The timing of writing the four *-stats files should be unified, so
-     that they are written exactly after 24 hours after starting the
+     that they are written exactly 24 hours after starting the
      relay. Right now, the measurement intervals for dirreq, entry, and
      exit stats starts with the first observed request, and files are
      written when observing the first request that occurs more than 24
@@ -373,14 +373,14 @@ Implementation notes:
      directory until they are included in extra-info documents. The
      reason is that the 24-hour measurement interval can be very
      different from the 18-hour publication interval of extra-info
-     documents. When a relay crashed after finishing a measurement
+     documents. When a relay crashes after finishing a measurement
      interval, but before publishing the next extra-info document,
      statistics would get lost. Therefore, statistics are written to
      disk when finishing a measurement interval and read from disk when
-     generating an extra-info document. As a result, the *-stats files
-     need to be overwritten after 24 hours, rather than appending new
-     statistics to them. Further, the contents of the *-stats files need
-     to be checked in the process of generating extra-info documents.
+     generating an extra-info document. Only the statistics that were
+     appended to the *-stats files within the past 24 hours are included
+     in extra-info documents. Further, the contents of the *-stats files
+     need to be checked in the process of generating extra-info documents.
 
   4. With the statistics patches being tested, the ./configure options
      should be removed and the statistics code be compiled by default.
diff --git a/src/or/connection.c b/src/or/connection.c
index af4d1349ba..48f8278573 100644
--- a/src/or/connection.c
+++ b/src/or/connection.c
@@ -2302,11 +2302,13 @@ connection_handle_write(connection_t *conn, int force)
     /* else open, or closing */
     result = flush_buf_tls(or_conn->tls, conn->outbuf,
                            max_to_write, &conn->outbuf_flushlen);
+
     /* If we just flushed the last bytes, check if this tunneled dir
      * request is done. */
     if (buf_datalen(conn->outbuf) == 0 && conn->dirreq_id)
       geoip_change_dirreq_state(conn->dirreq_id, DIRREQ_TUNNELED,
                                 DIRREQ_OR_CONN_BUFFER_FLUSHED);
+
     switch (result) {
       CASE_TOR_TLS_ERROR_ANY:
       case TOR_TLS_CLOSE:
diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c
index 3b399d9bd1..1df576d636 100644
--- a/src/or/connection_edge.c
+++ b/src/or/connection_edge.c
@@ -2544,9 +2544,11 @@ connection_exit_begin_conn(cell_t *cell, circuit_t *circ)
 
   log_debug(LD_EXIT,"Creating new exit connection.");
   n_stream = edge_connection_new(CONN_TYPE_EXIT, AF_INET);
+
   /* Remember the tunneled request ID in the new edge connection, so that
    * we can measure download times. */
   TO_CONN(n_stream)->dirreq_id = circ->dirreq_id;
+
   n_stream->_base.purpose = EXIT_PURPOSE_CONNECT;
 
   n_stream->stream_id = rh.stream_id;
@@ -2786,6 +2788,7 @@ connection_exit_connect_dir(edge_connection_t *exitconn)
   /* Note that the new dir conn belongs to the same tunneled request as
    * the edge conn, so that we can measure download times. */
   TO_CONN(dirconn)->dirreq_id = TO_CONN(exitconn)->dirreq_id;
+
   connection_link_connections(TO_CONN(dirconn), TO_CONN(exitconn));
 
   if (connection_add(TO_CONN(exitconn))<0) {
diff --git a/src/or/or.h b/src/or/or.h
index c9eb4a3f91..9a0f51f461 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -840,7 +840,7 @@ typedef struct packed_cell_t {
 /** Number of cells added to a circuit queue including their insertion
  * time on 10 millisecond detail; used for buffer statistics. */
 typedef struct insertion_time_elem_t {
-  struct insertion_time_elem_t *next;
+  struct insertion_time_elem_t *next; /**< Next element in queue. */
   uint32_t insertion_time; /**< When were cells inserted (in 10 ms steps
                              * starting at 0:00 of the current day)? */
   unsigned counter; /**< How many cells were inserted? */
@@ -848,8 +848,8 @@ typedef struct insertion_time_elem_t {
 
 /** Queue of insertion times. */
 typedef struct insertion_time_queue_t {
-  struct insertion_time_elem_t *first;
-  struct insertion_time_elem_t *last;
+  struct insertion_time_elem_t *first; /**< First element in queue. */
+  struct insertion_time_elem_t *last; /**< Last element in queue. */
 } insertion_time_queue_t;
 
 /** A queue of cells on a circuit, waiting to be added to the
@@ -858,7 +858,7 @@ typedef struct cell_queue_t {
   packed_cell_t *head; /**< The first cell, or NULL if the queue is empty. */
   packed_cell_t *tail; /**< The last cell, or NULL if the queue is empty. */
   int n; /**< The number of cells in the queue. */
-  insertion_time_queue_t *insertion_times;
+  insertion_time_queue_t *insertion_times; /**< Insertion times of cells. */
 } cell_queue_t;
 
 /** Beginning of a RELAY cell payload. */
diff --git a/src/or/relay.c b/src/or/relay.c
index b45856d51e..a79a4c161a 100644
--- a/src/or/relay.c
+++ b/src/or/relay.c
@@ -1626,12 +1626,11 @@ void
 cell_queue_append_packed_copy(cell_queue_t *queue, const cell_t *cell)
 {
   packed_cell_t *copy = packed_cell_copy(cell);
-  /* Remember the time in millis when this cell was put in the queue. */
+  /* Remember the time when this cell was put in the queue. */
   if (get_options()->CellStatistics) {
     struct timeval now;
     uint32_t added;
     insertion_time_queue_t *it_queue = queue->insertion_times;
-    int add_new_elem = 0;
     if (!it_pool)
       it_pool = mp_pool_new(sizeof(insertion_time_elem_t), 1024);
     tor_gettimeofday(&now);
@@ -1641,15 +1640,9 @@ cell_queue_append_packed_copy(cell_queue_t *queue, const cell_t *cell)
       it_queue = tor_malloc_zero(sizeof(insertion_time_queue_t));
       queue->insertion_times = it_queue;
     }
-    if (!it_queue->first) {
-      add_new_elem = 1;
+    if (it_queue->last && it_queue->last->insertion_time == added) {
+      it_queue->last->counter++;
     } else {
-      if (it_queue->last->insertion_time == added)
-        it_queue->last->counter++;
-      else
-        add_new_elem = 1;
-    }
-    if (add_new_elem) {
       insertion_time_elem_t *elem = mp_pool_get(it_pool);
       elem->next = NULL;
       elem->insertion_time = added;
diff --git a/src/or/rephist.c b/src/or/rephist.c
index a9bf95da1b..3e4ba672d0 100644
--- a/src/or/rephist.c
+++ b/src/or/rephist.c
@@ -1336,7 +1336,7 @@ rep_hist_note_bytes_read(size_t num_bytes, time_t when)
 /* The following data structures are arrays and no fancy smartlists or maps,
  * so that all write operations can be done in constant time. This comes at
  * the price of some memory (1.25 MB) and linear complexity when writing
- * stats. */
+ * stats for measuring relays. */
 /** Number of bytes read in current period by exit port */
 static uint64_t *exit_bytes_read = NULL;
 /** Number of bytes written in current period by exit port */
diff --git a/src/or/router.c b/src/or/router.c
index e6d6cc37b2..3249f49ce0 100644
--- a/src/or/router.c
+++ b/src/or/router.c
@@ -1269,6 +1269,7 @@ router_rebuild_descriptor(int force)
   uint32_t addr;
   char platform[256];
   int hibernating = we_are_hibernating();
+  size_t ei_size;
   or_options_t *options = get_options();
 
   if (desc_clean_since && !force)
@@ -1382,11 +1383,10 @@ router_rebuild_descriptor(int force)
   ei->cache_info.published_on = ri->cache_info.published_on;
   memcpy(ei->cache_info.identity_digest, ri->cache_info.identity_digest,
          DIGEST_LEN);
-  ei->cache_info.signed_descriptor_body =
-      tor_malloc(MAX_EXTRAINFO_UPLOAD_SIZE);
+  ei_size = options->ExtraInfoStatistics ? MAX_EXTRAINFO_UPLOAD_SIZE : 8192;
+  ei->cache_info.signed_descriptor_body = tor_malloc(ei_size);
   if (extrainfo_dump_to_string(ei->cache_info.signed_descriptor_body,
-                               MAX_EXTRAINFO_UPLOAD_SIZE,
-                               ei, get_identity_key()) < 0) {
+                               ei_size, ei, get_identity_key()) < 0) {
     log_warn(LD_BUG, "Couldn't generate extra-info descriptor.");
     extrainfo_free(ei);
     return -1;