Attached is a patch for mod_cache (patch is for httpd-2.2.4) that implements what I suggested in May (see the entire thread at http://mail-archives.apache.org/mod_mbox/httpd-dev/200705.mbox/[EMAIL PROTECTED] ).

The problem is that cached objects that gets hammered with Cache-Control: max-age=0 requests will get their on-disk headers rewritten for each request, and since max-age=0 are always revalidated (hence the rewriting in the first place) those rewritten on-disk headers will never be used. Since the ground rule of caching is to cache stuff that's being reused this is rather suboptimal.

The solution is to NOT rewrite the on-disk headers when the following conditions are true:
- The body is NOT stale (ie. HTTP_NOT_MODIFIED when revalidating)
- The on-disk header hasn't expired.
- The request has max-age=0

This is perfectly OK with RFC2616 10.3.5 and does NOT break anything.

Patch is tested on httpd-2.2.4 and works as expected according to my tests.


/Nikke
--
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
 Niklas Edmundsson, Admin @ {acc,hpc2n}.umu.se      |     [EMAIL PROTECTED]
---------------------------------------------------------------------------
 A pretty .GIF is like a melody
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
--- ../dist/modules/cache/mod_cache.c   2006-12-08 13:56:00.000000000 +0100
+++ modules/cache/mod_cache.c   2007-07-28 22:17:48.000000000 +0200
@@ -305,7 +305,7 @@
     cache_server_conf *conf;
     const char *cc_out, *cl;
     const char *exps, *lastmods, *dates, *etag;
-    apr_time_t exp, date, lastmod, now;
+    apr_time_t exp, date, lastmod, now, staleexp=APR_DATE_BAD;
     apr_off_t size;
     cache_info *info = NULL;
     char *reason;
@@ -582,6 +582,8 @@
             /* Oh, hey.  It isn't that stale!  Yay! */
             cache->handle = cache->stale_handle;
             info = &cache->handle->cache_obj->info;
+            /* Save stale expiry timestamp for later perusal */
+            staleexp = info->expire;
             rv = OK;
         }
         else {
@@ -736,14 +738,41 @@
         ap_cache_accept_headers(cache->handle, r, 1);
     }
 
-    /* Write away header information to cache. It is possible that we are
-     * trying to update headers for an entity which has already been cached.
-     *
-     * This may fail, due to an unwritable cache area. E.g. filesystem full,
-     * permissions problems or a read-only (re)mount. This must be handled
-     * later.
-     */
-    rv = cache->provider->store_headers(cache->handle, r, info);
+    /* Avoid storing on-disk headers that are never used. When the following
+     * conditions are fulfilled:
+     * - The body is NOT stale (ie. HTTP_NOT_MODIFIED when revalidating)
+     * - The on-disk header hasn't expired.
+     * - The request has max-age=0
+     * Then there is no use to update the on-disk header since it won't be used
+     * by other max-age=0 requests since they are always revalidated, and we
+     * know it's likely there will be more max-age=0 requests since objects
+     * tend to have the same access pattern.
+     * Luckily for us RFC2616 10.3.5 last paragraph allows us to NOT update the
+     * on-disk headers if we don't want to on HTTP_NOT_MODIFIED.
+     */
+    rv = APR_EGENERAL;
+    if(cache->stale_handle && staleexp != APR_DATE_BAD && now < staleexp) {
+        const char *cc_req;
+        char *val;
+
+        cc_req = apr_table_get(r->headers_in, "Cache-Control");
+        if(cc_req && ap_cache_liststr(r->pool, cc_req, "max-age", &val) &&
+                val != NULL && apr_atoi64(val) == 0) 
+        {
+            /* Yay, we can skip storing the on-disk header */
+            rv = APR_SUCCESS;
+        }
+    }
+    if(rv != APR_SUCCESS) {
+        /* Write away header information to cache. It is possible that we are
+         * trying to update headers for an entity which has already been 
cached.
+         *
+         * This may fail, due to an unwritable cache area. E.g. filesystem 
full,
+         * permissions problems or a read-only (re)mount. This must be handled
+         * later.
+         */
+        rv = cache->provider->store_headers(cache->handle, r, info);
+    }
 
     /* Did we just update the cached headers on a revalidated response?
      *

Reply via email to