Mailing-List: contact dev-help@httpd.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@httpd.apache.org
Message-ID: <4C8DC8E5.9020401@apache.org>
Date: Mon, 13 Sep 2010 08:47:01 +0200
From: Ruediger Pluem <rpluem@apache.org>
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US;
 rv:1.8.1.24) Gecko/20100301 SeaMonkey/1.1.19
MIME-Version: 1.0
To: dev@httpd.apache.org
Subject: Re: svn commit: r996395 - in /httpd/httpd/trunk: CHANGES
 docs/manual/mod/mod_disk_cache.xml
 include/ap_mmn.h modules/cache/mod_cache.c modules/cache/mod_cache.h
 modules/cache/mod_disk_cache.c
 modules/cache/mod_disk_cache.h
References: <20100912231650.1B1722388903@eris.apache.org>
In-Reply-To: <20100912231650.1B1722388903@eris.apache.org>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit


On 09/13/2010 01:16 AM, minfrin@apache.org wrote:
> Author: minfrin
> Date: Sun Sep 12 23:16:49 2010
> New Revision: 996395
> 
> URL: http://svn.apache.org/viewvc?rev=996395&view=rev
> Log:
> mod_cache: Change the signature of the store_body() provider function
> within the mod_cache provider interface to support an "in" brigade
> and an "out" brigade instead of just a single input brigade. This
> gives a cache provider the option to consume only part of the brigade
> passed to it, rather than the whole brigade as was required before.
> This fixes an out of memory and a request timeout condition that would
> occur when the original document was a large file. Update the
> mod_disk_cache provider implementation to take into account the new API.
> Introduce CacheReadSize and CacheReadTime directives to mod_disk_cache
> to control the amount of data to attempt to cache before sending the
> data on to the client in the "out" brigade.
> 
> Modified:
>     httpd/httpd/trunk/CHANGES
>     httpd/httpd/trunk/docs/manual/mod/mod_disk_cache.xml
>     httpd/httpd/trunk/include/ap_mmn.h
>     httpd/httpd/trunk/modules/cache/mod_cache.c
>     httpd/httpd/trunk/modules/cache/mod_cache.h
>     httpd/httpd/trunk/modules/cache/mod_disk_cache.c
>     httpd/httpd/trunk/modules/cache/mod_disk_cache.h
> 

> Modified: httpd/httpd/trunk/modules/cache/mod_disk_cache.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/cache/mod_disk_cache.c?rev=996395&r1=996394&r2=996395&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/cache/mod_disk_cache.c (original)
> +++ httpd/httpd/trunk/modules/cache/mod_disk_cache.c Sun Sep 12 23:16:49 2010

> @@ -1028,22 +1031,65 @@ static apr_status_t store_body(cache_han
>          }
>          dobj->file_size = 0;
>      }
> +    if (!dobj->bb) {
> +        dobj->bb = apr_brigade_create(r->pool, r->connection->bucket_alloc);
> +    }
> +    if (!dobj->offset) {
> +        dobj->offset = dconf->readsize;
> +    }
> +    if (!dobj->timeout && dconf->readtime) {
> +        dobj->timeout = apr_time_now() + dconf->readtime;
> +    }
>  
> -    for (e = APR_BRIGADE_FIRST(bb);
> -         e != APR_BRIGADE_SENTINEL(bb);
> -         e = APR_BUCKET_NEXT(e))
> -    {
> +    if (dobj->offset) {
> +        apr_brigade_partition(in, dobj->offset, &e);
> +    }
> +
> +    while (APR_SUCCESS == rv && !APR_BRIGADE_EMPTY(in)) {
>          const char *str;
>          apr_size_t length, written;
> +
> +        e = APR_BRIGADE_FIRST(in);
> +
> +        /* have we seen eos yet? */
> +        if (APR_BUCKET_IS_EOS(e)) {
> +            seen_eos = 1;
> +            APR_BUCKET_REMOVE(e);
> +            APR_BRIGADE_CONCAT(out, dobj->bb);
> +            APR_BRIGADE_INSERT_TAIL(out, e);
> +            continue;

Can't this lead to a situation where buckets that follow an EOS bucket (the only ones
I can think of are Metabuckets) get swallowed forever by mod_disk_cache?
These possible Metabuckets will be swallowed and added to dobj->bb, but never put
in the out brigade. So shouldn't we just CONCAT the remaining part of in to out and
leave?

Regards

Rüdiger