httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ruediger Pluem <rpl...@apache.org>
Subject Re: strange usage pattern for child processes
Date Sun, 26 Oct 2008 16:15:13 GMT


On 10/26/2008 04:58 PM, Jim Jagielski wrote:
> 
> On Oct 25, 2008, at 11:17 AM, Ruediger Pluem wrote:
> 
>>
>>
>> On 10/19/2008 03:06 PM, Ruediger Pluem wrote:
>>
>>> Another maybe funny sidenote: Because of the way the read method on
>>> socket buckets
>>> work and the way the core input filter works, the ap_get_brigade call
>>> when processing
>>> the http body of the backend response in mod_proxy_http never returns
>>> a brigade that
>>> contains more then 8k of data no matter what you set for
>>> ProxyIOBufferSize.
>>> And this is the case since 2.0.x days. So the optimization was always
>>> limited to
>>> sending at most 8k and in this case the TCP buffer (the send buffer)
>>> should have
>>> fixed this in many cases.
>>
>> The following patch should fix the above thing. Comments?
>>
>> Index: server/core_filters.c
>> ===================================================================
>> --- server/core_filters.c       (Revision 707744)
>> +++ server/core_filters.c       (Arbeitskopie)
>> @@ -267,6 +267,35 @@
>>             return APR_SUCCESS;
>>         }
>>
>> +        /* Have we read as much data as we wanted (be greedy)? */
>> +        if (len < readbytes) {
>> +            apr_size_t bucket_len;
>> +
>> +            rv = APR_SUCCESS;
>> +            /* We already registered the data in e in len */
>> +            e = APR_BUCKET_NEXT(e);
>> +            while ((len < readbytes) && (rv == APR_SUCCESS)
>> +                   && (e != APR_BRIGADE_SENTINEL(ctx->b))) {
>> +                /* Check for the availability of buckets with known
>> length */
>> +                if (e->length != -1) {
>> +                    len += e->length;
>> +                    e = APR_BUCKET_NEXT(e);
>> +                    continue;
>> +                }
>> +                /*
>> +                 * Read from bucket, but non blocking. If there isn't
>> any
>> +                 * more data, well than this is fine as well, we will
>> +                 * not wait for more since we already got some and we
>> are
>> +                 * only checking if there isn't more.
>> +                 */
>> +                rv = apr_bucket_read(e, &str, &bucket_len,
>> APR_NONBLOCK_READ);
>> +                if (rv == APR_SUCCESS) {
>> +                    len += bucket_len;
>> +                    e = APR_BUCKET_NEXT(e);
>> +                }
>> +            }
>>
> 
> +1 for the concept, but I'm not sure I like the logic flow
> of the continue. This is really an if/else so how about:

Sure it is an if/else. I don't care very much whether it is continue
of if/else. It just saved some indenting. So It can be in the
way you proposed below.

> 
>             while ((len < readbytes) && (rv == APR_SUCCESS)
>                    && (e != APR_BRIGADE_SENTINEL(ctx->b))) {
>                 /* Check for the availability of buckets with known
> length */
>                 if (e->length != -1) {
>                     len += e->length;
>                     e = APR_BUCKET_NEXT(e);
>                 } else {
>                     /*
>                      * Read from bucket, but non blocking. If there
> isn't any
>                      * more data, well than this is fine as well, we will
>                      * not wait for more since we already got some and
> we are
>                      * only checking if there isn't more.
>                      */
>                     rv = apr_bucket_read(e, &str, &bucket_len,
> APR_NONBLOCK_READ);
>                     if (rv == APR_SUCCESS) {
>                         len += bucket_len;
>                         e = APR_BUCKET_NEXT(e);
>                     }
>                 }
>             }

Regards

RĂ¼diger


Mime
View raw message