Stream fairness: Stop packaging inbufs onto circuits greedily
Analysis copied from a comment in bug #1298 (moved), with
crossouts and changes
Grepping: We call connection_edge_package_raw_inbuf() in 4 places. One is the case in relay.c that we fixed in #1937 (moved). The other three cases have no constraint on how much we process from the stream. They are:
- (A) connection_edge_process_relay_cell_not_open(), upon receiving a RELAY_COMMAND_CONNECTED
- (B) connection_edge_process_relay_cell(), upon receiving a RELAY_COMMAND_SENDME.
- (C) The case in relay.c that we fixed with #1937 (moved)
- (D) connection_edge_process_inbuf(), upon receiving data on an edge connection.
Case (C) is fixed.
Case (D) is fair Case (D) gives preference to loud streams that show up earlier, though after that it is fair since we'll just package data as it arrives, and we consider connections that we can read from in pseuodrandom or first-come-first-served order.
Case (A) and case (B) can lead to hogging the circuit temporarily, though I think it happens in a more-or-less fair way. It would be neat to make those cases less hoggish, but we have a bit of an architectural problem in the way. Right now (as you can see above) there are two circumstances when we package edge data into relay cells:
- (1) Data arrives on an edge conn, so we package as much as we can immediately.
- (2) An edge-conn becomes non-blocked (either it was blocked on its own, or its whole circuit was blocked), so we package as much data as we can (from the connection if just one was blocked, or from all the connections on a circuit if a whole circuit was blocked).
Thus, if we leave data on an edge conn inbuf temporarily without blocking the edge conn, there's currently no mechanism to make sure that it will get pulled off unless more data arrives, or unless the edge conn becomes blocked then unblocked. We could add such a mechanism, I guess, but I'm not currently clear on how it would look.