[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: Tor client performance (was Re: URGENT: patch needed ASAP for authority bug)



On Wed, Apr 21, 2010 at 06:59:18AM -0400, Roger Dingledine wrote:
> Once I've slept and have thought about it more, I'm going to write a
> test patch that will preemptively close these TLS connections earlier
> than the client would otherwise close them.

Attached is that patch. It only applies against git head, so if you
prefer tarballs, check out
http://freehaven.net/~arma/tor-0.2.2.12-alpha-dev.tar.gz

I expect this will be a miraculous solution for everybody involved. Please
let me know either way. ;)

I've left the once-per-second notice logs in place so it's easy to
compare to the previous behavior. There's also a whole lot more detail
at info-level if you like that sort of thing.

Thanks!
--Roger

diff --git a/src/or/circuituse.c b/src/or/circuituse.c
index 9eda9e2..1588052 100644
--- a/src/or/circuituse.c
+++ b/src/or/circuituse.c
@@ -17,7 +17,7 @@ extern circuit_t *global_circuitlist; /* from circuitlist.c */
 
 /********* END VARIABLES ************/
 
-static void circuit_expire_old_circuits(time_t now);
+static void circuit_expire_old_circuits_clientside(time_t now);
 static void circuit_increment_failure_count(void);
 
 long int lround(double x);
@@ -567,7 +567,7 @@ circuit_build_needed_circs(time_t now)
     time_to_new_circuit = now + options->NewCircuitPeriod;
     if (proxy_mode(get_options()))
       addressmap_clean(now);
-    circuit_expire_old_circuits(now);
+    circuit_expire_old_circuits_clientside(now);
 
 #if 0 /* disable for now, until predict-and-launch-new can cull leftovers */
     circ = circuit_get_youngest_clean_open(CIRCUIT_PURPOSE_C_GENERAL);
@@ -656,7 +656,7 @@ circuit_detach_stream(circuit_t *circ, edge_connection_t *conn)
  * for too long and has no streams on it: mark it for close.
  */
 static void
-circuit_expire_old_circuits(time_t now)
+circuit_expire_old_circuits_clientside(time_t now)
 {
   circuit_t *circ;
   time_t cutoff;
@@ -696,6 +696,38 @@ circuit_expire_old_circuits(time_t now)
   }
 }
 
+/** Find each non-origin circuit that has been unused for too long,
+ * has no streams on it, used a create_fast, and ends here: mark it
+ * for close.
+ */
+void
+circuit_expire_old_circuits_serverside(time_t now)
+{
+  circuit_t *circ;
+  time_t cutoff = now - 60;
+
+  for (circ = global_circuitlist; circ; circ = circ->next) {
+    or_circuit_t *or_circ;
+    if (circ->marked_for_close || CIRCUIT_IS_ORIGIN(circ))
+      continue;
+    or_circ = TO_OR_CIRCUIT(circ);
+    /* If the circuit has been idle for too long, and there are no streams
+     * on it, and it ends here, and it used a create_fast, mark it for close.
+     */
+    if (or_circ->is_first_hop && !circ->n_conn &&
+        !or_circ->n_streams && !or_circ->resolving_streams &&
+        or_circ->p_conn &&
+        or_circ->p_conn->timestamp_last_added_nonpadding <= cutoff) {
+      int age = now - or_circ->p_conn->timestamp_last_added_nonpadding;
+      connection_t *p_conn = TO_CONN(or_circ->p_conn);
+      log_info(LD_CIRC, "Closing circ_id %d (empty %d secs ago) (%s)",
+               or_circ->p_circ_id, age,
+               p_conn->have_handled_begindir ? "yes" : "no");
+      circuit_mark_for_close(circ, END_CIRC_REASON_FINISHED);
+    }
+  }
+}
+
 /** Number of testing circuits we want open before testing our bandwidth. */
 #define NUM_PARALLEL_TESTING_CIRCS 4
 
diff --git a/src/or/connection.c b/src/or/connection.c
index 7b1493b..0ded386 100644
--- a/src/or/connection.c
+++ b/src/or/connection.c
@@ -694,6 +694,7 @@ void
 connection_expire_held_open(void)
 {
   time_t now;
+  int num_or=0, num_begindir=0, num_empty=0, num_one=0;
   smartlist_t *conns = get_connection_array();
 
   now = time(NULL);
@@ -703,6 +704,44 @@ connection_expire_held_open(void)
     /* If we've been holding the connection open, but we haven't written
      * for 15 seconds...
      */
+    if (conn->type == CONN_TYPE_OR) {
+      num_or++;
+      if (conn->have_handled_begindir) {
+        num_begindir++;
+        if (TO_OR_CONN(conn)->n_circuits == 0)
+          num_empty++;
+        if (TO_OR_CONN(conn)->n_circuits == 1)
+          num_one++;
+        if (now % 60 == 0) { /* much more detailed stats */
+    int i = conn_sl_idx;
+    log(LOG_INFO, LD_GENERAL,
+        "Conn %d (socket %d) type %d (%s%s), "
+        "state %d (%s), created %d secs ago",
+        i, conn->s, conn->type, conn_type_to_string(conn->type),
+        conn->have_handled_begindir ? " YES" : "",
+        conn->state, conn_state_to_string(conn->type, conn->state),
+        (int)(now - conn->timestamp_created));
+    log(LOG_INFO, LD_GENERAL,
+        "Conn %d is to %s:%d.", i,
+        safe_str_client(conn->address),
+        conn->port);
+    log(LOG_INFO, LD_GENERAL,
+        "Conn %d: %d bytes waiting on inbuf (len %d, last read %d secs ago)",
+        i,
+        (int)buf_datalen(conn->inbuf),
+        (int)buf_allocation(conn->inbuf),
+        (int)(now - conn->timestamp_lastread));
+    log(LOG_INFO, LD_GENERAL,
+        "Conn %d: %d bytes waiting on outbuf "
+        "(len %d, last written %d secs ago)",i,
+        (int)buf_datalen(conn->outbuf),
+        (int)buf_allocation(conn->outbuf),
+        (int)(now - conn->timestamp_lastwritten));
+    log(LOG_INFO, LD_GENERAL,
+        "Conn %d: %d circuits open", i, TO_OR_CONN(conn)->n_circuits);
+        }
+      }
+    }
     if (conn->hold_open_until_flushed) {
       tor_assert(conn->marked_for_close);
       if (now - conn->timestamp_lastwritten >= 15) {
@@ -722,6 +761,8 @@ connection_expire_held_open(void)
       }
     }
   });
+  log_notice(LD_DIR, "%d (%d,%d) of %d/%d used for begindir",
+             num_begindir, num_empty, num_one, num_or, smartlist_len(conns));
 }
 
 /** Create an AF_INET listenaddr struct.
diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c
index a173dc1..4867627 100644
--- a/src/or/connection_edge.c
+++ b/src/or/connection_edge.c
@@ -2784,9 +2784,15 @@ connection_exit_connect_dir(edge_connection_t *exitconn)
 {
   dir_connection_t *dirconn = NULL;
   or_circuit_t *circ = TO_OR_CIRCUIT(exitconn->on_circuit);
+  connection_t *or_conn = TO_CONN(circ->p_conn);
 
   log_info(LD_EXIT, "Opening local connection for anonymized directory exit");
 
+  if (or_conn->have_handled_begindir == 0 && circ->is_first_hop) {
+    log_info(LD_DIR, "conn %d first used for begindir", or_conn->s);
+    or_conn->have_handled_begindir = 1;
+  }
+
   exitconn->_base.state = EXIT_CONN_STATE_OPEN;
 
   dirconn = dir_connection_new(AF_INET);
diff --git a/src/or/main.c b/src/or/main.c
index ccc25c5..790d022 100644
--- a/src/or/main.c
+++ b/src/or/main.c
@@ -696,6 +696,9 @@ run_connection_housekeeping(int i, time_t now)
   connection_t *conn = smartlist_get(connection_array, i);
   or_options_t *options = get_options();
   or_connection_t *or_conn;
+  int past_keepalive =
+    now >= conn->timestamp_lastwritten + options->KeepalivePeriod;
+  int maxCircuitlessPeriod = 60;
 
   if (conn->outbuf && !buf_datalen(conn->outbuf) && conn->type == CONN_TYPE_OR)
     TO_OR_CONN(conn)->timestamp_lastempty = now;
@@ -730,6 +733,9 @@ run_connection_housekeeping(int i, time_t now)
   if (!connection_speaks_cells(conn))
     return; /* we're all done here, the rest is just for OR conns */
 
+  /* If we haven't written to an OR connection for a while, then either nuke
+     the connection or send a keepalive, depending. */
+
   or_conn = TO_OR_CONN(conn);
   tor_assert(conn->outbuf);
 
@@ -745,52 +751,46 @@ run_connection_housekeeping(int i, time_t now)
                                    "Tor gave up on the connection");
     connection_mark_for_close(conn);
     conn->hold_open_until_flushed = 1;
-    return;
-  }
-
-  /* If we haven't written to an OR connection for a while, then either nuke
-     the connection or send a keepalive, depending. */
-  if (now >= conn->timestamp_lastwritten + options->KeepalivePeriod) {
-    int maxCircuitlessPeriod = options->MaxCircuitDirtiness*3/2;
-    if (!connection_state_is_open(conn)) {
-      /* We never managed to actually get this connection open and happy. */
-      log_info(LD_OR,"Expiring non-open OR connection to fd %d (%s:%d).",
-               conn->s,conn->address, conn->port);
-      connection_mark_for_close(conn);
-      conn->hold_open_until_flushed = 1;
-    } else if (we_are_hibernating() && !or_conn->n_circuits &&
-               !buf_datalen(conn->outbuf)) {
-      /* We're hibernating, there's no circuits, and nothing to flush.*/
-      log_info(LD_OR,"Expiring non-used OR connection to fd %d (%s:%d) "
-               "[Hibernating or exiting].",
-               conn->s,conn->address, conn->port);
-      connection_mark_for_close(conn);
-      conn->hold_open_until_flushed = 1;
-    } else if (!or_conn->n_circuits &&
-               now >= or_conn->timestamp_last_added_nonpadding +
-                                           maxCircuitlessPeriod) {
-      log_info(LD_OR,"Expiring non-used OR connection to fd %d (%s:%d) "
-               "[idle].", conn->s,conn->address, conn->port);
-      connection_mark_for_close(conn);
-      conn->hold_open_until_flushed = 1;
-    } else if (
-         now >= or_conn->timestamp_lastempty + options->KeepalivePeriod*10 &&
-         now >= conn->timestamp_lastwritten + options->KeepalivePeriod*10) {
-      log_fn(LOG_PROTOCOL_WARN,LD_PROTOCOL,
-             "Expiring stuck OR connection to fd %d (%s:%d). (%d bytes to "
-             "flush; %d seconds since last write)",
-             conn->s, conn->address, conn->port,
-             (int)buf_datalen(conn->outbuf),
-             (int)(now-conn->timestamp_lastwritten));
-      connection_mark_for_close(conn);
-    } else if (!buf_datalen(conn->outbuf)) {
-      /* either in clique mode, or we've got a circuit. send a padding cell. */
-      log_fn(LOG_DEBUG,LD_OR,"Sending keepalive to (%s:%d)",
-             conn->address, conn->port);
-      memset(&cell,0,sizeof(cell_t));
-      cell.command = CELL_PADDING;
-      connection_or_write_cell_to_buf(&cell, or_conn);
-    }
+  } else if (past_keepalive && !connection_state_is_open(conn)) {
+    /* We never managed to actually get this connection open and happy. */
+    log_info(LD_OR,"Expiring non-open OR connection to fd %d (%s:%d).",
+             conn->s,conn->address, conn->port);
+    connection_mark_for_close(conn);
+    conn->hold_open_until_flushed = 1;
+  } else if (we_are_hibernating() && !or_conn->n_circuits &&
+             !buf_datalen(conn->outbuf)) {
+    /* We're hibernating, there's no circuits, and nothing to flush.*/
+    log_info(LD_OR,"Expiring non-used OR connection to fd %d (%s:%d) "
+             "[Hibernating or exiting].",
+             conn->s,conn->address, conn->port);
+    connection_mark_for_close(conn);
+    conn->hold_open_until_flushed = 1;
+  } else if (!or_conn->n_circuits &&
+             now >= or_conn->timestamp_last_added_nonpadding +
+                                         maxCircuitlessPeriod) {
+    log_info(LD_OR,"Expiring non-used OR connection to fd %d (%s:%d) "
+             "[idle %d].", conn->s,conn->address, conn->port,
+             (int)(now - or_conn->timestamp_last_added_nonpadding));
+    connection_mark_for_close(conn);
+    conn->hold_open_until_flushed = 1;
+  } else if (
+      now >= or_conn->timestamp_lastempty + options->KeepalivePeriod*10 &&
+      now >= conn->timestamp_lastwritten + options->KeepalivePeriod*10) {
+    log_fn(LOG_PROTOCOL_WARN,LD_PROTOCOL,
+           "Expiring stuck OR connection to fd %d (%s:%d). (%d bytes to "
+           "flush; %d seconds since last write)",
+           conn->s, conn->address, conn->port,
+           (int)buf_datalen(conn->outbuf),
+           (int)(now-conn->timestamp_lastwritten));
+    connection_mark_for_close(conn);
+  } else if (past_keepalive && !buf_datalen(conn->outbuf) &&
+             !conn->have_handled_begindir) {
+    /* send a padding cell */
+    log_fn(LOG_DEBUG,LD_OR,"Sending keepalive to (%s:%d)",
+           conn->address, conn->port);
+    memset(&cell,0,sizeof(cell_t));
+    cell.command = CELL_PADDING;
+    connection_or_write_cell_to_buf(&cell, or_conn);
   }
 }
 
@@ -1134,6 +1134,9 @@ run_scheduled_events(time_t now)
   if (have_dir_info && !we_are_hibernating())
     circuit_build_needed_circs(now);
 
+  if (now % 10 == 5)
+    circuit_expire_old_circuits_serverside(now);
+
   /** 5. We do housekeeping for each connection... */
   connection_or_set_bad_connections();
   for (i=0;i<smartlist_len(connection_array);i++) {
@@ -1672,8 +1675,10 @@ dumpstats(int severity)
   {
     int i = conn_sl_idx;
     log(severity, LD_GENERAL,
-        "Conn %d (socket %d) type %d (%s), state %d (%s), created %d secs ago",
+        "Conn %d (socket %d) type %d (%s%s), "
+        "state %d (%s), created %d secs ago",
         i, conn->s, conn->type, conn_type_to_string(conn->type),
+        conn->have_handled_begindir ? " YES" : "",
         conn->state, conn_state_to_string(conn->type, conn->state),
         (int)(now - conn->timestamp_created));
     if (!connection_is_listener(conn)) {
diff --git a/src/or/or.h b/src/or/or.h
index 4688548..38a4cca 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -956,6 +956,9 @@ typedef struct connection_t {
   /** CONNECT/SOCKS proxy client handshake state (for outgoing connections). */
   unsigned int proxy_state:4;
 
+  /** True iff this is an OR conn that has received a begindir request */
+  unsigned int have_handled_begindir:1;
+
   /** Our socket; -1 if this connection is closed, or has no socket. */
   evutil_socket_t s;
   int conn_array_index; /**< Index into the global connection array. */
@@ -3230,6 +3233,7 @@ int circuit_conforms_to_options(const origin_circuit_t *circ,
 void circuit_build_needed_circs(time_t now);
 void circuit_detach_stream(circuit_t *circ, edge_connection_t *conn);
 
+void circuit_expire_old_circuits_serverside(time_t now);
 void reset_bandwidth_test(void);
 int circuit_enough_testing_circs(void);