You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Ruediger Pluem <rp...@apache.org> on 2010/09/13 08:47:01 UTC
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
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
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
Posted by Ruediger Pluem <rp...@apache.org>.
On 09/14/2010 12:38 AM, Graham Leggett wrote:
> On 13 Sep 2010, at 8:47 AM, Ruediger Pluem wrote:
>
>> 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?
>
> We do need to do this, yes, thanks for catching this. Can you verify
> r996713 and confirm that it fixes it? After seeing the eos bucket, we
> enter a simple "passthrough" mode for all remaining buckets (and any
> further buckets that might arrive) and exit.
I don't have a test case at hand, but it looks fine from inspection.
BTW: Can't we avoid the loop if dobj->done and just concat the in brigade on
the out brigade and leave?
Regards
Rüdiger
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
Posted by Graham Leggett <mi...@sharp.fm>.
On 13 Sep 2010, at 8:47 AM, Ruediger Pluem wrote:
> 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?
We do need to do this, yes, thanks for catching this. Can you verify
r996713 and confirm that it fixes it? After seeing the eos bucket, we
enter a simple "passthrough" mode for all remaining buckets (and any
further buckets that might arrive) and exit.
Regards,
Graham
--