Mailing-List: contact dev-help@httpd.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@httpd.apache.org
Message-ID: <49049791.8030008@apache.org>
Date: Sun, 26 Oct 2008 17:15:13 +0100
From: Ruediger Pluem <rpluem@apache.org>
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US;
 rv:1.8.1.17) Gecko/20080829 SeaMonkey/1.1.12
MIME-Version: 1.0
To: dev@httpd.apache.org
Subject: Re: strange usage pattern for child processes
References: <20081015115548.GA9872@eilebrecht.net> <48F651DE.5060003@sharp.fm>
 <48F657A1.2070806@apache.org> <48F6752F.5030000@sharp.fm>
 <F2CA43B0-8CB5-4CED-9C94-5A251CFD2D23@jaguNET.com>
 <48F8EDEE.3080908@apache.org> <48F9E231.7060401@sharp.fm>
 <48F9F5C2.60209@apache.org> <48FA4590.2010106@sharp.fm>
 <48FB1826.4040607@apache.org> <48FB30CA.4060602@apache.org>
 <49033872.3020009@apache.org>
 <1D6916F2-7C97-403D-BF6A-7ED7A9BF346A@jaguNET.com>
In-Reply-To: <1D6916F2-7C97-403D-BF6A-7ED7A9BF346A@jaguNET.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit


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