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
