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, 05 Dec 2013 13:06:05 GMT
Hi Jan,

I don't think it is fixed in trunk, but I may have missed the commits.
Which ones are you talking about?

Regards,
Yann.


On Thu, Dec 5, 2013 at 1:51 PM, Jan Kalu┼ża <jkaluza@redhat.com> wrote:

> Hi,
>
> should this be fixed in trunk already? I see some commits in proxy code
> based on your ideas Yann, but I'm not sure if they address this particular
> problem too.
>
> Jan Kaluza
>
>
> On 10/17/2013 04:52 PM, Yann Ylavic wrote:
>
>>
>> On Thu, Oct 17, 2013 at 11:36 AM, Thomas Eckert
>> <thomas.r.w.eckert@gmail.com <mailto: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