httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Bill Stoddard <b...@wstoddard.com>
Subject Re: cvs commit: httpd-2.0/modules/experimental cache_storage.c cache_util.c mod_cache.c mod_cache.h mod_disk_cache.c mod_mem_cache.c
Date Thu, 23 Sep 2004 19:10:36 GMT
                         ap_get_module_config(r->request_config,
>> &cache_module);
>>      -    rv =  cache_generate_key(r,r->pool,&key);
>>   +    rv = cache_generate_key(r, r->pool, &key);
>>        if (rv != APR_SUCCESS) {
>>            return rv;
>>        }
>>        /* go through the cache types till we get a match */
>>   -    h = cache->handle = apr_palloc(r->pool, sizeof(cache_handle_t));
>>   +    h = apr_palloc(r->pool, sizeof(cache_handle_t));
>>    
> 
> 
> This little gem causes a regression. Because cache->handle is left NULL, 
> we never cleanup stale entries in the cache (in cache_save_filter). Once 
> CacheMaxExpires hits, the stale entry will never be garbage collected 
> from the cache and replaced by a new entry.  Two ways to fix this come 
> to mind right off hand (neither are optimal i expect):
> 
> Patch 1:
> Index: cache_storage.c
> ===================================================================
> RCS file: /home/cvs/httpd-2.0/modules/experimental/cache_storage.c,v
> retrieving revision 1.38
> diff -u -r1.38 cache_storage.c
> --- cache_storage.c    22 Sep 2004 22:25:45 -0000    1.38
> +++ cache_storage.c    23 Sep 2004 18:49:17 -0000
> @@ -248,6 +248,7 @@
>              /* Is our cached response fresh enough? */
>              fresh = ap_cache_check_freshness(h, r);
>              if (!fresh) {
> +                cache->provider->remove_entity(h);
>                  return DECLINED;
>              }
> 
> 
> Patch 2:
> Index: cache_storage.c
> ===================================================================
> RCS file: /home/cvs/httpd-2.0/modules/experimental/cache_storage.c,v
> retrieving revision 1.38
> diff -u -r1.38 cache_storage.c
> --- cache_storage.c    22 Sep 2004 22:25:45 -0000    1.38
> +++ cache_storage.c    23 Sep 2004 18:51:10 -0000
> @@ -248,6 +248,7 @@
>              /* Is our cached response fresh enough? */
>              fresh = ap_cache_check_freshness(h, r);
>              if (!fresh) {
> +                cache->provider->remove_entity(h);
>                  return DECLINED;
>              }
> 
> 
> 
> If no one ventures an opinion by this evening, I'll dig into it and come 
> up with a solution. No time at the moment.
> 
> Bill
> 
Last post before evening... This patch solves the regression w/o segfaulting. So the question
boils down to 
this: Do we want to make stale cache entries available to mod_cache or not? This patch does
not, I would 
suggest that mod_cache -does- need to have access to stale cache entries because sometimes
it may need to 
serve up the stale entry. Thoughts?

Index: cache_storage.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/experimental/cache_storage.c,v
retrieving revision 1.38
diff -u -r1.38 cache_storage.c
--- cache_storage.c	22 Sep 2004 22:25:45 -0000	1.38
+++ cache_storage.c	23 Sep 2004 19:06:34 -0000
@@ -248,6 +248,7 @@
              /* Is our cached response fresh enough? */
              fresh = ap_cache_check_freshness(h, r);
              if (!fresh) {
+                list->provider->remove_entity(h);
                  return DECLINED;
              }





Mime
View raw message