httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jim Jagielski <...@jaguNET.com>
Subject Re: strange usage pattern for child processes
Date Sun, 26 Oct 2008 15:58:56 GMT

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:

             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);
                     }
                 }
             }


Mime
View raw message