za419 / python-server

A genericized version of cadence-server, using a flexible interface to implement API endpoints
0 stars 0 forks source link

Implement keepalive connections #1

Open za419 opened 6 years ago

za419 commented 6 years ago

The best thing we could do performance wise is to keep TCP connections alive as long as possible. This would avoid repeated setup costs for long transactions, plus the benefit of skipping accept calls for repeated GETs. This is valuable for cadence, which involves about a dozen separate requests.

Right now, the server closes a connection as soon as the response has been sent. This means that each of those twelve requests is serviced by a separate TCP connection, while this could be handled by one - This means an extra 33 one-way trips (SYN, ACK, SYNACK times eleven unneeded connections), which could be 1.3 seconds of latency, before a client using one connection at a time would be able to display Cadence.

Now, this isn't quite as bad in reality - Modern browsers tend to use multiple simultaneous downloading connections, which both raises the lower limit on the number of connection setups and spreads out the latency cost - Saturating four connections would reduce the unneeded setups to eight, and the delay to about a quarter of a second.

Still, that's frankly unacceptable delay for an otherwise avoidable issue, even though kenellorando/cadence#120 helped increase this asynchronicity.

HTTP 1.1 features the Connection header for just this reason - The huge performance increase available when both the server and the client want to keep the connection alive.

Therefore, we should check for anytime a request asks us to keep the connection alive, and obey it. The force-close behavior has been good for development because it's nice and simple, but it's time to say goodbye, now that we're moving into production and expecting larger traffic volumes.

za419 commented 5 years ago

I've also begun to notice some issues in testing where extremely short select_timeouts (I was testing with 0) result in sockets being asked to return two different files at the same time, and the second overrides the first (resulting from a race condition between the queueResponse and the select, where the select will always win if it's allowed to run again in very short order).

My investigation has suggested that this is because the browser expects to use a persistent connection, which we don't support.

The problem is, a zero-second timeout is optimal for throughput and latency if the machine can handle it, so I'd really like it if this actually, y'know, worked.

This is more involved than just implementing keepalives, by about.... Maybe five lines of code in queueResponse. So I'll just bundle it in here.