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
<[email protected] <mailto:[email protected]>> 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.
