You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Graham Leggett <mi...@sharp.fm> on 2011/11/22 16:13:45 UTC

Re: svn commit: r1070179 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_cache.xml modules/cache/cache_storage.c modules/cache/cache_storage.h modules/cache/mod_cache.c modules/cache/mod_cache.h

On 13 Feb 2011, at 9:59 AM, Roy T. Fielding wrote:

> On Feb 12, 2011, at 6:03 PM, minfrin@apache.org wrote:
>
>> Author: minfrin
>> Date: Sun Feb 13 02:03:29 2011
>> New Revision: 1070179
>>
>> URL: http://svn.apache.org/viewvc?rev=1070179&view=rev
>> Log:
>> mod_cache: When a request other than GET or HEAD arrives, we must
>> invalidate existing cache entities as per RFC2616 13.10. PR 15868.
>
> Cache entries should not be invalidated unless the response comes
> back as a success (2xx).  Likewise, it only applies to methods with
> known write effects (not including TRACE, OPTIONS, PROPFIND, etc.).
>
> This has already been updated in httpbis p6.

Reviving this unresolved issue.

I need to update r1070179 to fix this, do I a) revert r1070179 on  
v2.4.x and continue to fix on trunk, or b) fix this on trunk and  
v2.4.x at the risk of potential further delay?

Regards,
Graham
--


Re: svn commit: r1070179 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_cache.xml modules/cache/cache_storage.c modules/cache/cache_storage.h modules/cache/mod_cache.c modules/cache/mod_cache.h

Posted by Graham Leggett <mi...@sharp.fm>.
On 30 Nov 2011, at 1:30 PM, Graham Leggett wrote:

> The change has been reverted on v2.4, and the following patch gives the API changes that allow us to fix this in the v2.4 series:

Applied in r 1208822 and backported in r1208824.

Regards,
Graham
--


Re: svn commit: r1070179 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_cache.xml modules/cache/cache_storage.c modules/cache/cache_storage.h modules/cache/mod_cache.c modules/cache/mod_cache.h

Posted by Graham Leggett <mi...@sharp.fm>.
On 23 Nov 2011, at 1:15 PM, Graham Leggett wrote:

> In order to make invalidation possible, we would need to add an invalidate() function to the mod_cache provider, and keep AP_CACHE_INVALIDATE inside ap_cache_status_e. Invalidation involves marking the entry as invalidated and "to be freshened on next hit", as opposed to deleted.
> 
> What I could do is back out the change, and make just the above API changes, allowing us to backport this to v2.4 when we are ready.
> 
> This releases v2.4 to get on with it, and keeps the option open for us to fix this properly at a future date in the v2.4 cycle when we're ready.

The change has been reverted on v2.4, and the following patch gives the API changes that allow us to fix this in the v2.4 series:

Index: modules/cache/mod_cache.h
===================================================================
--- modules/cache/mod_cache.h	(revision 1208384)
+++ modules/cache/mod_cache.h	(working copy)
@@ -109,17 +109,20 @@
                            const char *urlkey);
     int (*remove_url) (cache_handle_t *h, request_rec *r);
     apr_status_t (*commit_entity)(cache_handle_t *h, request_rec *r);
+    apr_status_t (*invalidate_entity)(cache_handle_t *h, request_rec *r);
 } cache_provider;
 
 typedef enum {
     AP_CACHE_HIT,
     AP_CACHE_REVALIDATE,
-    AP_CACHE_MISS
+    AP_CACHE_MISS,
+    AP_CACHE_INVALIDATE
 } ap_cache_status_e;
 
 #define AP_CACHE_HIT_ENV "cache-hit"
 #define AP_CACHE_REVALIDATE_ENV "cache-revalidate"
 #define AP_CACHE_MISS_ENV "cache-miss"
+#define AP_CACHE_INVALIDATE_ENV "cache-invalidate"
 #define AP_CACHE_STATUS_ENV "cache-status"
 
 
Index: modules/cache/mod_cache_disk.c
===================================================================
--- modules/cache/mod_cache_disk.c	(revision 1208379)
+++ modules/cache/mod_cache_disk.c	(working copy)
@@ -1325,6 +1325,11 @@
     return APR_SUCCESS;
 }
 
+static apr_status_t invalidate_entity(cache_handle_t *h, request_rec *r)
+{
+    return APR_ENOTIMPL;
+}
+
 static void *create_dir_config(apr_pool_t *p, char *dummy)
 {
     disk_cache_dir_conf *dconf = apr_pcalloc(p, sizeof(disk_cache_dir_conf));
@@ -1502,7 +1507,8 @@
     &create_entity,
     &open_entity,
     &remove_url,
-    &commit_entity
+    &commit_entity,
+    &invalidate_entity
 };
 
 static void disk_cache_register_hook(apr_pool_t *p)

Regards,
Graham
--


Re: svn commit: r1070179 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_cache.xml modules/cache/cache_storage.c modules/cache/cache_storage.h modules/cache/mod_cache.c modules/cache/mod_cache.h

Posted by Graham Leggett <mi...@sharp.fm>.
On 22 Nov 2011, at 11:58 PM, Stefan Fritsch wrote:

> From the thread, it is not clear to me that there was consensous on
> how to fix it. Therefore I would vote for reverting r1070179 from 2.4,
> fixing it in trunk, and then backporting the whole thing to 2.4 again
> (maybe after 2.4.0).


In order to make invalidation possible, we would need to add an  
invalidate() function to the mod_cache provider, and keep  
AP_CACHE_INVALIDATE inside ap_cache_status_e. Invalidation involves  
marking the entry as invalidated and "to be freshened on next hit", as  
opposed to deleted.

What I could do is back out the change, and make just the above API  
changes, allowing us to backport this to v2.4 when we are ready.

This releases v2.4 to get on with it, and keeps the option open for us  
to fix this properly at a future date in the v2.4 cycle when we're  
ready.

Thoughts?

Regards,
Graham
--


Re: svn commit: r1070179 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_cache.xml modules/cache/cache_storage.c modules/cache/cache_storage.h modules/cache/mod_cache.c modules/cache/mod_cache.h

Posted by Stefan Fritsch <sf...@sfritsch.de>.
On Tuesday 22 November 2011, Graham Leggett wrote:
> On 13 Feb 2011, at 9:59 AM, Roy T. Fielding wrote:
> > On Feb 12, 2011, at 6:03 PM, minfrin@apache.org wrote:
> >> Author: minfrin
> >> Date: Sun Feb 13 02:03:29 2011
> >> New Revision: 1070179
> >> 
> >> URL: http://svn.apache.org/viewvc?rev=1070179&view=rev
> >> Log:
> >> mod_cache: When a request other than GET or HEAD arrives, we
> >> must invalidate existing cache entities as per RFC2616 13.10.
> >> PR 15868.
> > 
> > Cache entries should not be invalidated unless the response comes
> > back as a success (2xx).  Likewise, it only applies to methods
> > with known write effects (not including TRACE, OPTIONS,
> > PROPFIND, etc.).
> > 
> > This has already been updated in httpbis p6.
> 
> Reviving this unresolved issue.
> 
> I need to update r1070179 to fix this, do I a) revert r1070179 on
> v2.4.x and continue to fix on trunk, or b) fix this on trunk and
> v2.4.x at the risk of potential further delay?

From the thread, it is not clear to me that there was consensous on 
how to fix it. Therefore I would vote for reverting r1070179 from 2.4, 
fixing it in trunk, and then backporting the whole thing to 2.4 again 
(maybe after 2.4.0).