You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Greg Ames <gr...@remulak.net> on 2004/10/28 20:54:23 UTC
[PATCH] pipelining bug in Event MPM
I was able to easily create a browser hang by configuring Mozilla to enable HTTP
pipelining, then pointing my DocumentRoot at an old copy of the
xml.apache.org web site which had tons of imbedded graphics. Thanks, Justin,
for pointing out the bug.
The attached patch fixes it when there are no connection filters such as mod_ssl
involved. When I switch to the version of the patch that does speculative
reads, it behaves oddly. With pipelining, the last couple of .gifs aren't
displayed until something times out. Without pipelining nearly all the .gifs
are delayed. In gdb, it looks like it's not properly detecting when the input
filters become empty. I'm going to investigate further but need to take care of
some day job stuff first.
Paul, my apologies, this is against my version of the patch. I plan to switch
over to yours after figuring out what's happening with the speculative reads.
Greg
Re: [PATCH] pipelining bug in Event MPM
Posted by Greg Ames <gr...@remulak.net>.
Justin Erenkrantz wrote:
> On Mon, Nov 01, 2004 at 08:39:47PM -0500, Greg Ames wrote:
>
>>This makes it behave properly on my laptop with speculative reads. I have
>>no idea if it works with mod_ssl or what speculative buys us.
>
>
> mod_ssl will most likely work correctly without changes. -- justin
Let's stick with EATCRLF then since it looks faster. Thanks for checking into
this, Justin.
btw, I don't know if the CRLF-eating side effect of that mode helps or hurts
performance. It sure doesn't help people understand why that mode exists. If
trailing CRLFs are really rare, the current implementation could hurt
performance since we are testing for them.
Greg
Re: [PATCH] pipelining bug in Event MPM
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On Mon, Nov 01, 2004 at 08:39:47PM -0500, Greg Ames wrote:
> This makes it behave properly on my laptop with speculative reads. I have
> no idea if it works with mod_ssl or what speculative buys us.
mod_ssl will most likely work correctly without changes. -- justin
Re: [PATCH] pipelining bug in Event MPM
Posted by Greg Ames <gr...@remulak.net>.
Greg Ames wrote:
> Justin Erenkrantz wrote:
>> Okay. I'd be curious to figure out what's going on with the
>> speculative non-blocking reads.
> The odd behavior with speculative reads is due to API differences that I
> didn't take into account.
This makes it behave properly on my laptop with speculative reads. I have no
idea if it works with mod_ssl or what speculative buys us.
Greg
--- modules/http/http_request.c.old 2004-11-01 20:05:49.000000000 -0500
+++ modules/http/http_request.c 2004-11-01 20:11:17.000000000 -0500
@@ -206,12 +206,10 @@
/* ### shouldn't this read from the connection input filters? */
/* ### is zero correct? that means "read one line" */
if (r->connection->keepalive != AP_CONN_CLOSE) {
- /* if (ap_get_brigade(r->input_filters, bb, AP_MODE_SPECULATIVE,
- * APR_NONBLOCK_READ, 1) != APR_SUCCESS) {
- */
- if (ap_get_brigade(r->input_filters, bb, AP_MODE_EATCRLF,
- APR_NONBLOCK_READ, 0) != APR_SUCCESS) {
- c->data_in_input_filters = 0; /* we got APR_EOF or an error */
+ if (ap_get_brigade(r->input_filters, bb, AP_MODE_SPECULATIVE,
+ APR_NONBLOCK_READ, 1) != APR_SUCCESS ||
+ APR_BRIGADE_EMPTY(bb)) {
+ c->data_in_input_filters = 0; /* no input or an error */
}
else {
c->data_in_input_filters = 1;
Re: [PATCH] pipelining bug in Event MPM
Posted by Paul Querna <ch...@force-elite.com>.
Justin Erenkrantz wrote:
> --On Monday, November 1, 2004 6:06 PM -0500 Greg Ames
> <gr...@remulak.net> wrote:
>
>>> Given the timeframe and the fact that my schedule just sucks for the
>>> next two weeks, is there enough interest (and presence) to discuss this
>>> at the Hackathon next weekend in LV?
>>
>>
>> wish I could be there physically, but I can't :( Maybe IRC?
>
>
> Absolutely. We should come up with a schedule and post it to dev@httpd
> and have those who can't be there in person chime in electronically. --
> justin
>
I would love to have a time to discuss this at the Hackathon.
-Paul
Re: [PATCH] pipelining bug in Event MPM
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
--On Monday, November 1, 2004 6:06 PM -0500 Greg Ames
<gr...@remulak.net> wrote:
>> Given the timeframe and the fact that my schedule just sucks for the
>> next two weeks, is there enough interest (and presence) to discuss this
>> at the Hackathon next weekend in LV?
>
> wish I could be there physically, but I can't :( Maybe IRC?
Absolutely. We should come up with a schedule and post it to dev@httpd and
have those who can't be there in person chime in electronically. -- justin
Re: [PATCH] pipelining bug in Event MPM
Posted by Greg Ames <gr...@remulak.net>.
Justin Erenkrantz wrote:
>> I was able to easily create a browser hang by configuring Mozilla to
>> enable HTTP pipelining, then pointing my DocumentRoot at an old copy
>> of the xml.apache.org web site which had tons of imbedded graphics.
>> Thanks, Justin, for pointing out the bug.
>
> I'm sure you *really* want to thank me for pointing this out. ;-)
Actually I am, because I'm pretty pleased with what Bill S, Paul & I have done
already. Fixing this bug makes it more stable.
> Okay. I'd be curious to figure out what's going on with the speculative
> non-blocking reads. I think that's the conceptually right solution (or
> as close as we can do today), but I'm almost positive there is going to
> be something that barf on it - so we'd have to fix some stuff. So, I'm
> not shocked that it doesn't quite work...
The odd behavior with speculative reads is due to API differences that I didn't
take into account. EATCRLF returns APR_EOF when there is no input; SPECULATIVE
returns APR_SUCCESS and (presumably) an empty brigade. So the pipeline didn't
get flushed when it should and the connection tied up a worker thread until
read_request_line timed out. It shouldn't be difficult to patch when I get a
little time (the hard part).
Glancing at core_input_filter, it wasn't clear how SPECULATIVE was going to be
an improvement. I saw socket bucket reads in my test environment (no mod_ssl)
with both modes.
> I think Paul's already posted a patch that folds it in. But, it still
> uses EATCRLF.
>
> Given the timeframe and the fact that my schedule just sucks for the
> next two weeks, is there enough interest (and presence) to discuss this
> at the Hackathon next weekend in LV?
wish I could be there physically, but I can't :( Maybe IRC?
Greg
Re: [PATCH] pipelining bug in Event MPM
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
--On Thursday, October 28, 2004 2:54 PM -0400 Greg Ames
<gr...@remulak.net> wrote:
> I was able to easily create a browser hang by configuring Mozilla to
> enable HTTP pipelining, then pointing my DocumentRoot at an old copy
> of the xml.apache.org web site which had tons of imbedded graphics.
> Thanks, Justin, for pointing out the bug.
I'm sure you *really* want to thank me for pointing this out. ;-)
> The attached patch fixes it when there are no connection filters such as
> mod_ssl involved. When I switch to the version of the patch that does
> speculative reads, it behaves oddly. With pipelining, the last couple of
> .gifs aren't displayed until something times out. Without pipelining
> nearly all the .gifs are delayed. In gdb, it looks like it's not
> properly detecting when the input filters become empty. I'm going to
> investigate further but need to take care of some day job stuff first.
Okay. I'd be curious to figure out what's going on with the speculative
non-blocking reads. I think that's the conceptually right solution (or as
close as we can do today), but I'm almost positive there is going to be
something that barf on it - so we'd have to fix some stuff. So, I'm not
shocked that it doesn't quite work...
> Paul, my apologies, this is against my version of the patch. I plan to
> switch over to yours after figuring out what's happening with the
> speculative reads.
I think Paul's already posted a patch that folds it in. But, it still uses
EATCRLF.
Given the timeframe and the fact that my schedule just sucks for the next
two weeks, is there enough interest (and presence) to discuss this at the
Hackathon next weekend in LV? I bet if you throw enough of us together in
a room, we could hammer something out that works... -- justin