Enable keep-alive, --keep-alive-timeout flag added (default: 5000ms)

[minrk]

extends #481 to enable keepAlive not just on the server, but on proxied requests as well via an http agent, and exposing timeout on the command-line.

alternative to #491 using the standard library

closes #481
closes #491

[consideRatio]

I don't understand the details well enough to review this.

Are there two connections to consider (or more, or something else)? The connection initiated to the proxy (A->proxy), and the one the proxy initiated to some destination (proxy->B)?

Is "agent" associated with the connection towards the proxy, and "server" associated with the connection from the proxy onwards to some destination?

[minrk]

Are there two connections to consider (or more, or something else)? The connection initiated to the proxy (A->proxy), and the one the proxy initiated to some destination (proxy->B)?

Yes, both of these. The agent is the pool used to allow keeping connections alive for the proxy->backend requests. The keepAlive argument is for requests to the proxy. We need both of these because the keep-alive propagates all the way through the proxy, and if either server (the proxy or the backend) sets Connection: close the connections will be closed all the way up. This is because the proxy server relays the Connection header to the lowest value.

If any of the 4 entities involved (the client, the proxy's http server, the proxy's http client, or the upstream server) don't ask for the socket to be kept alive, then the connection will be closed.

• the client's response from the proxy will have Connection: close if any of the following are true:
  • the client doesn't request keep-alive (keep-alive is the default in http 1.1)
  • the proxy server doesn't specify keepAlive support (changed in this PR)
  • the upstream response sets Connection: close
• and that upstream response has again the same logic, where the client must request keepalive (changed in this PR), and the upstream server must support it, which tornado does.

[consideRatio]

Thanks a lot for helping me understand this better, I think I understand it well enough to dare agree this makes sense and merge given that this LGTM!