httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Yann Ylavic <ylavic....@gmail.com>
Subject Re: mod_proxy, oooled backend connections and the keep-alive race condition
Date Thu, 17 Oct 2013 14:52:45 GMT
On Thu, Oct 17, 2013 at 11:36 AM, Thomas Eckert <thomas.r.w.eckert@gmail.com
> wrote:

> > Hence,
> 
> why cannot mod_proxy_http prefetch the client's body *before* trying
> anything with the backend connection, and only if it's all good, connect
> (or reuse a connection to) the
> > backend and then start forwarding the data immediatly (flushing the
> prefetched ones) ?
>
> Now I'm confused. Are we talking about flushing everything immediately
> from client to backend (that's what I understood from your first patch
> proposal in this thread) or are we talking about pre fetching the whole
> request body from the client and then passing it on to the backend as a
> whole ?
>
>
Sorry, I really should not have proposed 2 patches in the same thread
without being clear about their different purposes.

I first thought flushing everything received from the client to the backend
immediatly, and disabling *retries* in the 16K-prefetch, would address the
issue (my first patch, "httpd-trunk-mod_proxy_http_flushall.patch"). But as
you said in a previous message, that did not address the case where the
client and/or an input filter retain the very first body bytes, and
mod_proxy_http blocks, and the backend timeouts...

Hence the second patch ("httpd-trunk-mod_proxy_http_prefetch.patch"), which
does prefetch *before* establishing/checking the backend's connection, so
to minimize (to the minimum) the future delay between that and the first
bytes sent to the backend (hence the chances for it to timeout). The
prefetch is still up to 16K (same as existing), and not the "whole" body of
course.

Then, when it's time to forward (what is available from) the client's
request, the patched mod_proxy_http will establish or check+reuse a backend
connection and always flush immediatly what it has  already (headers +
prefetch, see stream_reqbody_* functions).

For the remaining request's body chunks (if any), I intentionally did not
change the current behaviour of letting the output filters choose when to
flush, this is not the keep-alive timeout anymore, so it becomes out of the
issue's scope. But I'm definitively +1 to flush-all here too (optionaly),
since mod_proxy may retain small chunks and can then face another backend
timeout later! That's where patch1 meets patch2, and comes patch3
("httpd-trunk-mod_proxy_http_prefetch+flushall.patch", attached here).

There is still the potential (minimal) race condition, the backend could
have closed in the (short) meantime, that's indeed *not* addressed by my
patch(es).


>
> The problem I mentioned in the first message is about treating the backend
> connection in an error prone way. By not ensuring the connection is still
> valid - e.g. as seen by the peer - we risk running into this problem no
> matter what we do elsewhere. So I really think the proper way to handle
> this is to reopen the connection if necessary - be this with a filter or
> integrated into the connection handling itself - and only fail after we
> tried at least once.
>

IMHO this is hardly feasible for non-idempotent (and no body) requests
(without disabling connection reuse, or using "proxy-initial-not-pooled" to
let the client choose), even if mod_proxy takes care of not sending the
full request in one time to be able to detect connection problems before
it's too late (to retry), there is still a possibility that it won't know
before sending the last chunk, and the issue remain. mod_proxy_http
addresses retries for requests *with body* by the "ping" parameter, maybe
it could also retry idempotent requests (like mod_proxy_ajp does), but this
is still not an overall solution, is there one?



>
> Re-thinking the proposed flush mechanism, I don't think using the flushing
> will really solve the problem, albeit it probably narrowing down the window
> of opportunity for the problem significantly in most scenarios. I mention
> this because I'm getting the feeling two different discussions are being
> mixed up: Solving the
> 
> "keep-alive time out" problem and introducing the
> 
> "flush-all-immediately" option. While the latter might improve the
> situation with the first, it is no complete solution and there are a lot of
> scenarios where it does not apply due to other factors like filters (as
> discussed previously).
>

See above, the flushall does not resolve the issue (filters...), but the
early prefetch + flushall reduces it considerably, IMHO.


>
> The flush option itself sounds like a good idea, so I see no reason why
> not to put it in. I just don't want to loose focus on the actual problem ;-)
>

I don't see either...

Please consider the new (yet) patch attached, can you still encounter some
"proxy: error reading status line from remote server" with it?
Even without "proxy-flushall" set, you shouldn't, whatever the time taken
to read the request first 16K bytes...

Regards,
Yann.

Mime
View raw message