404 does not delete cached entries using mod_disk_cache
Felix Enning pointed me again to an interesting question regarding mod_cache / mod_disk_cache: The following situation was observed with Apache 2.0.54 (same applies to trunk): 1. A resource gets cached. 2. The original resource gets removed from the backend (e.g on a proxied webserver, on the local disk, wherever). 3. The client sents a request that forces the cache to revalidate this entry. 4. The 404 received from the backend is correctly passed back to the client by mod_cache. 5. The client sents a request that does NOT require the cache to revalidate this entry. 6. Cache delivers the old resource that had been cached before, instead of a 404. Is this behaviour intended and compliant with the RFC? The reason for this behaviour is that the remove_url function of mod_disk_cache is a dummy function (BTW: mod_mem_cache seems to really remove the cache entry in remove_url). If this behaviour is not intended I would have a look into this to create a patch. Regards Rüdiger
Re: 404 does not delete cached entries using mod_disk_cache
[EMAIL PROTECTED] wrote: Felix Enning pointed me again to an interesting question regarding mod_cache / mod_disk_cache: The following situation was observed with Apache 2.0.54 (same applies to trunk): 1. A resource gets cached. 2. The original resource gets removed from the backend (e.g on a proxied webserver, on the local disk, wherever). 3. The client sents a request that forces the cache to revalidate this entry. 4. The 404 received from the backend is correctly passed back to the client by mod_cache. 5. The client sents a request that does NOT require the cache to revalidate this entry. 6. Cache delivers the old resource that had been cached before, instead of a 404. Is this behaviour intended and compliant with the RFC? Not to my knowlegde. Given that mod_mem_cache and mod_disk_cache are doing different things is pretty much indicative that one of the two is wrong ;). The reason for this behaviour is that the remove_url function of mod_disk_cache is a dummy function (BTW: mod_mem_cache seems to really remove the cache entry in remove_url). If this behaviour is not intended I would have a look into this to create a patch. Please do! Sander
Re: 404 does not delete cached entries using mod_disk_cache
Sander Striker wrote:
> [EMAIL PROTECTED] wrote:
[..cut..]
>> Is this behaviour intended and compliant with the RFC?
>
>
> Not to my knowlegde. Given that mod_mem_cache and mod_disk_cache are doing
> different things is pretty much indicative that one of the two is wrong ;).
That was also my thought.
>
>> The reason for this behaviour is that the remove_url function of
>> mod_disk_cache is a dummy function
>> (BTW: mod_mem_cache seems to really remove the cache entry in
>> remove_url).
>> If this behaviour is not intended I would have a look into this to
>> create a patch.
>
>
> Please do!
>
I created a patch but the problem turned out to be more complex than I thought
originally. So a close look on the patch is definitely a good thing. Some
comments:
1. I had to adjust the cache provider API for remove_url as I need the
request_rec
struct to remove the files correctly in mod_disk_cache.
2. It turned out that 404 responses are not passed down the filter chain the
way I expected.
Adjusting the default handler again proved that the changes to
mod_disk_cache worked
(files got deleted), but this broke any error page handling in Apache. So I
tried to address
this problem at other locations of the code. I detected two cases:
1. Apache generated error messages or redirect to external source.
2. Custom local error documents.
In the first case I use the insert_error_filter hook to ensure that the
CACHE_SAVE filter
is reinserted to the filter chain if it has been inserted before during the
request.
In the second case the filter chain is run, but with the wrong URI. So I
checked if there
is a previous request (r->prev) and if it has the same status code (this
happens in a section
where we only handle uncachable status codes). If this is the case I assume
that I should delete
the URL from the previous request from the cache.
So any comments / thoughts on this?
Regards
Rüdiger
Index: httpd-trunk/modules/cache/mod_mem_cache.c
===
--- httpd-trunk/modules/cache/mod_mem_cache.c (revision 178625)
+++ httpd-trunk/modules/cache/mod_mem_cache.c (working copy)
@@ -601,7 +601,7 @@
/* remove_url()
* Notes:
*/
-static int remove_url(const char *key)
+static int remove_url(const char *key, request_rec *r)
{
cache_object_t *obj;
int cleanup = 0;
Index: httpd-trunk/modules/cache/mod_cache.c
===
--- httpd-trunk/modules/cache/mod_cache.c (revision 178625)
+++ httpd-trunk/modules/cache/mod_cache.c (working copy)
@@ -121,6 +121,8 @@
"Adding CACHE_SAVE filter.");
/* add cache_save filter to cache this request */
+apr_table_set(r->notes, CACHE_SAVE_FILTER_INSERTED,
+ CACHE_SAVE_FILTER_INSERTED);
ap_add_output_filter_handle(cache_save_filter_handle, NULL, r,
r->connection);
}
@@ -254,6 +256,7 @@
if (r->no_cache ||
(!conf->store_nostore &&
ap_cache_liststr(NULL, cc_in, "no-store", NULL))) {
+apr_table_unset(r->notes, CACHE_SAVE_FILTER_INSERTED);
ap_remove_output_filter(f);
return ap_pass_brigade(f->next, in);
}
@@ -293,6 +296,7 @@
*/
rv = cache->provider->store_body(cache->handle, r, in);
if (rv != APR_SUCCESS) {
+apr_table_unset(r->notes, CACHE_SAVE_FILTER_INSERTED);
ap_remove_output_filter(f);
}
return ap_pass_brigade(f->next, in);
@@ -440,13 +444,31 @@
if (reason) {
ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
"cache: %s not cached. Reason: %s", url, reason);
+
/* remove this object from the cache
* BillS Asks.. Why do we need to make this call to remove_url?
* leave it in for now..
*/
-cache_remove_url(r, url);
+/* Check if there is a previous request and if it has the same
+ * status code. If yes, it is assumed that we are filtering on
+ * an custom error response. But we want to remove the original
+ * entry from cache, so take the data from previous request for
+ * cache_remove_url.
+ */
+if (r->prev != NULL) {
+request_rec *prev_r = r->prev;
+
+if (r->status == prev_r->status) {
+cache_remove_url(prev_r, prev_r->unparsed_uri);
+}
+}
+else {
+cache_remove_url(r, url);
+}
+
/* remove this filter from the chain */
+apr_table_unset(r->notes, CACHE_SAVE_FILTER_INSERTED);
ap_remove_output_filter(f);
/* ship the data up the stack */
@@ -538,6 +560,7 @@
if (rv != OK) {
/* Caching layer declined the opportunity to cache the response */
+apr_table_
Re: 404 does not delete cached entries using mod_disk_cache
Anybody found some time / has some time to have a look at the patch? This would be really great and appreciated. Thanks Rüdiger [EMAIL PROTECTED] wrote: > Sander Striker wrote: > >>[EMAIL PROTECTED] wrote: > > > [..cut..] > > >>>Is this behaviour intended and compliant with the RFC? >> >> >>Not to my knowlegde. Given that mod_mem_cache and mod_disk_cache are doing >>different things is pretty much indicative that one of the two is wrong ;). > > > That was also my thought. > > >>>The reason for this behaviour is that the remove_url function of >>>mod_disk_cache is a dummy function >>>(BTW: mod_mem_cache seems to really remove the cache entry in >>>remove_url). >>>If this behaviour is not intended I would have a look into this to >>>create a patch. >> >> >>Please do! >> > > > I created a patch but the problem turned out to be more complex than I thought > originally. So a close look on the patch is definitely a good thing. Some > comments: > > 1. I had to adjust the cache provider API for remove_url as I need the > request_rec >struct to remove the files correctly in mod_disk_cache. > > 2. It turned out that 404 responses are not passed down the filter chain the > way I expected. >Adjusting the default handler again proved that the changes to > mod_disk_cache worked >(files got deleted), but this broke any error page handling in Apache. So > I tried to address >this problem at other locations of the code. I detected two cases: > >1. Apache generated error messages or redirect to external source. >2. Custom local error documents. > >In the first case I use the insert_error_filter hook to ensure that the > CACHE_SAVE filter >is reinserted to the filter chain if it has been inserted before during > the request. > >In the second case the filter chain is run, but with the wrong URI. So I > checked if there >is a previous request (r->prev) and if it has the same status code (this > happens in a section >where we only handle uncachable status codes). If this is the case I > assume that I should delete >the URL from the previous request from the cache. > > So any comments / thoughts on this? > > > Regards > > Rüdiger
[Fwd: Re: 404 does not delete cached entries using mod_disk_cache]
Sorry for being persistent, but any news / comments on this?
Regards
Rüdiger
Original Message
Subject: Re: 404 does not delete cached entries using mod_disk_cache
Date: Thu, 26 May 2005 16:51:14 +0200
From: [EMAIL PROTECTED]
Reply-To: dev@httpd.apache.org
To: dev@httpd.apache.org
References: <[EMAIL PROTECTED]> <[EMAIL PROTECTED]>
Sander Striker wrote:
> [EMAIL PROTECTED] wrote:
[..cut..]
>> Is this behaviour intended and compliant with the RFC?
>
>
> Not to my knowlegde. Given that mod_mem_cache and mod_disk_cache are doing
> different things is pretty much indicative that one of the two is wrong ;).
That was also my thought.
>
>> The reason for this behaviour is that the remove_url function of
>> mod_disk_cache is a dummy function
>> (BTW: mod_mem_cache seems to really remove the cache entry in
>> remove_url).
>> If this behaviour is not intended I would have a look into this to
>> create a patch.
>
>
> Please do!
>
I created a patch but the problem turned out to be more complex than I thought
originally. So a close look on the patch is definitely a good thing. Some
comments:
1. I had to adjust the cache provider API for remove_url as I need the
request_rec
struct to remove the files correctly in mod_disk_cache.
2. It turned out that 404 responses are not passed down the filter chain the
way I expected.
Adjusting the default handler again proved that the changes to
mod_disk_cache worked
ke any error page handling in Apache. So I tried to address
this problem at other locations of the code. I detected two cases:
1. Apache generated error messages or redirect to external source.
2. Custom local error documents.
In the first case I use the insert_error_filter hook to ensure that the
CACHE_SAVE filter
is reinserted to the filter chain if it has been inserted before during the
request.
In the second case the filter chain is run, but with the wrong URI. So I
checked if there
is a previous request (r->prev) and if it has the same status code (this
happens in a section
where we only handle uncachable status codes). If this is the case I assume
that I should delete
the URL from the previous request from the cache.
So any comments / thoughts on this?
Regards
Rüdiger
Index: httpd-trunk/modules/cache/mod_mem_cache.c
===
--- httpd-trunk/modules/cache/mod_mem_cache.c (revision 178625)
+++ httpd-trunk/modules/cache/mod_mem_cache.c (working copy)
@@ -601,7 +601,7 @@
/* remove_url()
* Notes:
*/
-static int remove_url(const char *key)
{
cache_object_t *obj;
int cleanup = 0;
Index: httpd-trunk/modules/cache/mod_cache.c
===
--- httpd-trunk/modules/cache/mod_cache.c (revision 178625)
+++ httpd-trunk/modules/cache/mod_cache.c (working copy)
@@ -121,6 +121,8 @@
"Adding CACHE_SAVE filter.");
/* add cache_save filter to cache this request */
+apr_table_set(r->notes, CACHE_SAVE_FILTER_INSERTED,
+ CACHE_SAVE_FILTER_INSERTED);
ap_add_output_filter_handle(cache_save_filter_handle, NULL, r,
r->connection);
}
@@ -254,6 +256,7 @@
if (r->no_cache ||
(!conf->store_nostore &&
ap_cache_liststr(NULL, cc_in, "no-store", NULL))) {
+apr_table_unset(r->notes, CACHE_SAVE_FILTER_INSERTED);
ap_remove_output_filter(f);
return ap_pass_brigade(f->next, in);
}
@@ -293,6 +296,7 @@
*/
rv = cache->provider->store_body(cache->handle, r, in);
if (rv != APR_SUCCESS) {
+apr_table_unset(r->notes, CACHE_SAVE_FILTER_INSERTED);
ap_remove_output_filter(f);
}
return ap_pass_brigade(f->next, in);
@@ -440,13 +444,31 @@
if (reason) {
ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
"cache: %s not cached. Reason: %s", url, reason);
+
/* remove this object from the cache
* BillS Asks.. Why do we need to make this call to remove_url?
* leave it in for now..
*/
-cache_remove_url(r, url);
+/* Check if there is a previous request and if it has the same
+ * status code. If yes, it is assumed that we are filtering on
+ * an custom error response. But we want to remove the original
+ * entry from cache, so take the data from previous request for
+ * cache_remove_url.
+ */
+if (r->prev != NULL) {
+request_rec *prev_r = r->prev;
+
+if (r->status == prev_r->status) {
+cache_remove_u
[Fwd: [PATCH] 404 does not delete cached entries using mod_disk_cache]
Forwarding for Rüdiger, since he's having some problems posting. Sander Original Message Subject: [PATCH] 404 does not delete cached entries using mod_disk_cache Date: Thu, 21 Jul 2005 14:52:19 +0200 From: Plüm, Rüdiger, VIS <[EMAIL PROTECTED]> To: <[EMAIL PROTECTED]> After having a discussion with various people (Sander, Justin, Paul) at the ApacheCon I submit a new version of my 404 does not delete cached entries using mod_disk_cache: Some comments: 1. In the case that there is an Apache generated error message the content filters get removed. This problem is now solved by adding a protocol filter (CACHE_REMOVE_URL) in the quick handler each time we add the cache save filter. If the cache save filter caches the response from the backend it just removes this filter from the chain. As the request CACHE_REMOVE_URL is running on a request that might be different fromi the one where the cache entry should be flushed, the cache request rec is taken from the filter context where it has been saved during insertation. 2. I adjusted the cache provider API for remove_url as I needed to have a pool and (in case of mod_disk_cache) the file name present at remove_url. Thus the prototype for remove url does now look like the following: int (*remove_url) (cache_handle_t *h, apr_pool_t *p); Comments / thoughts? Regards Rüdiger new_404_patch_trunk.diff Description: Binary data
Re: [Fwd: Re: 404 does not delete cached entries using mod_disk_cache]
[EMAIL PROTECTED] wrote: Sorry for being persistent, but any news / comments on this? Thanks for being persistent and patient. Unfortunately, no, I don't think there is any news yet. I have gone over your patch, but not detailed enough to commit it. I'll give it another shot before friday. Sander
Re: [Fwd: Re: 404 does not delete cached entries using mod_disk_cache]
Sander Striker wrote: > [EMAIL PROTECTED] wrote: > >> Sorry for being persistent, but any news / comments on this? > > > Thanks for being persistent and patient. Thanks for the response and your time. I try to follow the three P's: be patient, persistent and polite :-) Regards Rüdiger
Re: [Fwd: [PATCH] 404 does not delete cached entries using mod_disk_cache]
Short ping. Some time has passed. Has anybody found some time to review this patch? Regards Rüdiger Sander Striker wrote: > Forwarding for Rüdiger, since he's having some problems posting. > > Sander > > Original Message > Subject: [PATCH] 404 does not delete cached entries using mod_disk_cache > Date: Thu, 21 Jul 2005 14:52:19 +0200 > From: Plüm, Rüdiger, VIS <[EMAIL PROTECTED]> > To: <[EMAIL PROTECTED]> > > After having a discussion with various people (Sander, Justin, Paul) at > the ApacheCon > I submit a new version of my 404 does not delete cached entries using > mod_disk_cache: > [..cut..]
