httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Thomas Eckert <thomas.r.w.eck...@gmail.com>
Subject Re: mod_proxy, oooled backend connections and the keep-alive race condition
Date Thu, 05 Dec 2013 16:04:25 GMT
> It also seems that it adds more cycles to Apache on the front to
> reduce a race condition that can't really be removed.

While it's true that the race condition itself cannot be avoided we can
definitely work around the resulting problem situation, e.g. by trying to
open the connection again once and then either send the data or fail back
to the client as we do right now. The approach itself is not theoretical: I
know that's exactly how certain forward proxies solve the problem.


> Plus, if we do this here, shouldn't we do it for
> all proxy protocol submodules (fcgi, etc...)?

I go with my original suggestion: Have the above mentioned retry-once
mechanism apply on connection level so that request modules don't even
notice. Otherwise I would agree in having it added to the appropriate
modules. I would like to come up with the appropriate patch but I doubt
I'll have the time for it anytime soon :-(


As it stands I think Yann's approach in "flushing through the reverse
proxy" is better then doing nothing. Though I expect that to be useless for
a lot of deployments where stuff like AV scanning or mod_security will need
to buffer the request bodies - then you will have the original problem
again.


On Thu, Dec 5, 2013 at 4:05 PM, Jim Jagielski <jim@jagunet.com> wrote:

> There hardly seemed any consensus on the patch... It also
> seems that it adds more cycles to Apache on the front to
> reduce a race condition that can't really be removed.
> IMO, a reverse proxy should get out of the way as
> quickly as possible.
>
> Plus, if we do this here, shouldn't we do it for
> all proxy protocol submodules (fcgi, etc...)?
>
> Plus, as mentioned, this single patch includes 2 "fixes"
> which can be/should be independent.
>
> On Dec 5, 2013, at 8:06 AM, Yann Ylavic <ylavic.dev@gmail.com> wrote:
>
> > 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