Closing upstream keepalive connections in an invalid state
x at chrisbranch.co.uk
Tue Dec 1 12:19:27 UTC 2015
Thanks for your feedback!
> On 30 Nov 2015, at 18:09, Maxim Dounin <mdounin at mdounin.ru> wrote:
> The patch looks incomplete for me. It doesn't seem to handle the
> "next upstream" case. And the condition used looks wrong, too, as
> it doesn't take into account what nginx actually tried to send.
Next upstream was indeed forgotten, and is relevant for the case of buffered request + large request body (not tested).
However I disagree that the condition looks wrong. So long as a request is sent in full, the connection remains usable - that’s defined solely by the data remaining in our buffers and the data we are waiting for from the client. There are potential improvements to the keepalive handling for which ‘what nginx actually tried to send’ is irrelevant:
- If using chunked encoding, send a trailer sequence to place the upstream connection in a valid state
- Discard the request body to keep the downstream connection alive
Having a special flag for this simple case seems unnecessary. It is surely more robust to make a decision based on our current state rather than indirectly deciding that with a flag. You must agree or you wouldn’t dislike the quick and dirty (but otherwise completely functional) patch :)
More information about the nginx-devel