httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joe Orton <jor...@redhat.com>
Subject Re: trunk/2.4 core output filter is broken
Date Mon, 23 Jan 2012 13:32:40 GMT
On Sun, Jan 22, 2012 at 12:12:09PM +0100, Stefan Fritsch wrote:
> On Friday 20 January 2012, Joe Orton wrote:
> > If we assume that morphing buckets cannot be buffered, the code
> > could be adjusted to always place them in the "to flush" segment,
> > and then there is no need to read the buckets until they need to
> > be sent, as in the patch below.  This seems to fix the memory
> > consumption behaviour without obviously breaking anything else
> > (cough cough).
> 
> I think that your patch is correct. However, as an optimization, one 
> could try reading the morphing bucket until there are 
> THRESHOLD_MAX_BUFFER bytes in memory. If all morphing buckets in the 
> brigade disappear before reaching that limit, one could still do async 
> write completion.

The blocking behaviour does matter here.  If the brigade contains 60K of 
HEAP buckets and then a morphing bucket, the core must have written out 
the HEAP buckets before it attempts a *blocking* read on the morphing 
bucket.  The usual style is per:

http://httpd.apache.org/docs/trunk/developer/output-filters.html#nonblock

The current behaviour is a regression back to early 2.0.x, PR 8482.

> I don't think so. AFAIK, there is no way for an async mpm to poll on 
> more bytes from the producer bucket. If the network to the client is 
> faster than the producing cgi, a non blocking read would only put a 
> few (or even zero) bytes in the output buffer, the output socket would 
> immediately become writable again, and the output filter would be 
> called again. This would cause a busy loop, wasting CPU cycles. If my 
> assessment is correct, this should go as a comment into 
> send_brigade_nonblocking().

I think I was not clear enough here: yes, the non-blocking read must be 
followed by blocking reads.

> > Good catch on ctx->bytes_in. I'd add: why is 
> > core_output_filter_ctx_t in a public header?
> 
> There is no good reason other than that other core filter structs like 
> core_filter_ctx and core_net_rec are exposed, too. And those are 
> actually used by the winnt MPM. I would prefer if all these structs 
> were private and core_filters.c would provide an API to allocate them 
> from the winnt MPM and add additional buckets to the input brigade.

Yes!  I totally agree.

> Is this something we should still do for 2.4.x (iff 2.4.0 is not 
> released)?

Don't know. :)

Regards, Joe

Mime
View raw message