httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Justin Erenkrantz <jus...@erenkrantz.com>
Subject [PATCH] Updating stale response in mod_cache
Date Fri, 24 Sep 2004 21:50:02 GMT
--On Friday, September 24, 2004 11:25 AM -0700 Justin Erenkrantz 
<justin@erenkrantz.com> wrote:

> caching-proxied-conditional requests.  My preliminary thought is:
>
> 1) If cache_select_url() determines the cached response is out-of-date,
> it can then add the 'If-None-Match' header (and any other headers as
> required) to the request.
> 2) We'll unconditionally add cache_save_filter because we don't have a
> usable cached entity.
> 3) cache_save_filter can then determine if we receive a 304 back by
> looking at r->status.
> 4) If we receive a 304, then cache_save_filter can update the cached
> headers and leave the body alone.  [We'd have to add a lookup function at
> this point so we can identify which provider needs to be updated.]
> 5) If we did get a 304 *and* the original request was conditional,
> cache_save_filter can then remove itself (after updating headers) and get
> out of the way.
> 6) If we did get a 304 *and* the original request was *not* conditional
> (somehow we'd have to signal that, not sure how at the moment), then
> cache_save_filter can serve the cached body by calling recall_body and
> then eating all saved output.
> 7) If we didn't get a 304 from the origin server, cache_save_filter
> continues normally.
>
> What do you think?  -- justin

More or less, the following patch does the above.  I haven't had a chance
to test it, and won't likely to be able to do so until early next week.

There are a couple of FIXMEs it'd be nice to resolve first.  -- justin

Index: modules/experimental/cache_storage.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/experimental/cache_storage.c,v
retrieving revision 1.39
diff -u -r1.39 cache_storage.c
--- modules/experimental/cache_storage.c	24 Sep 2004 01:22:47 -0000	1.39
+++ modules/experimental/cache_storage.c	24 Sep 2004 21:42:35 -0000
@@ -245,10 +245,32 @@
                 }
             }

+            cache->provider = list->provider;
+            cache->provider_name = list->provider_name;
+
             /* Is our cached response fresh enough? */
             fresh = ap_cache_check_freshness(h, r);
             if (!fresh) {
-                list->provider->remove_entity(h);
+                cache_info *info = &(h->cache_obj->info);
+
+                /* Make response into a conditional */
+                /* FIXME: What if the request is already conditional? */
+                if (info && info->etag) {
+                    /* if we have a cached etag */
+                    cache->stale_headers = apr_table_copy(r->pool,
+                                                          r->headers_in);
+                    apr_table_set(r->headers_in, "If-None-Match", 
info->etag);
+                    cache->stale_handle = h;
+                }
+                else if (info && info->lastmods) {
+                    /* if we have a cached Last-Modified header */
+                    cache->stale_headers = apr_table_copy(r->pool,
+                                                          r->headers_in);
+                    apr_table_set(r->headers_in, "If-Modified-Since",
+                                  info->lastmods);
+                    cache->stale_handle = h;
+                }
+
                 return DECLINED;
             }

@@ -258,8 +280,6 @@
             r->filename = apr_pstrdup(r->pool, 
h->cache_obj->info.filename);
             accept_headers(h, r);

-            cache->provider = list->provider;
-            cache->provider_name = list->provider_name;
             cache->handle = h;
             return OK;
         }
Index: modules/experimental/cache_util.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/experimental/cache_util.c,v
retrieving revision 1.34
diff -u -r1.34 cache_util.c
--- modules/experimental/cache_util.c	21 Sep 2004 22:56:23 -0000	1.34
+++ modules/experimental/cache_util.c	24 Sep 2004 21:42:35 -0000
@@ -22,12 +22,12 @@
 /* -------------------------------------------------------------- */

 /* return true if the request is conditional */
-CACHE_DECLARE(int) ap_cache_request_is_conditional(request_rec *r)
+CACHE_DECLARE(int) ap_cache_request_is_conditional(apr_table_t *table)
 {
-    if (apr_table_get(r->headers_in, "If-Match") ||
-        apr_table_get(r->headers_in, "If-None-Match") ||
-        apr_table_get(r->headers_in, "If-Modified-Since") ||
-        apr_table_get(r->headers_in, "If-Unmodified-Since")) {
+    if (apr_table_get(table, "If-Match") ||
+        apr_table_get(table, "If-None-Match") ||
+        apr_table_get(table, "If-Modified-Since") ||
+        apr_table_get(table, "If-Unmodified-Since")) {
         return 1;
     }
     return 0;
Index: modules/experimental/mod_cache.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/experimental/mod_cache.c,v
retrieving revision 1.91
diff -u -r1.91 mod_cache.c
--- modules/experimental/mod_cache.c	22 Sep 2004 22:02:13 -0000	1.91
+++ modules/experimental/mod_cache.c	24 Sep 2004 21:42:35 -0000
@@ -219,7 +219,7 @@
     ap_log_error(APLOG_MARK, APLOG_DEBUG, APR_SUCCESS, r->server,
                  "cache: running CACHE_OUT filter");

-    /* recall_body() was called in cache_select_url() */
+    /* recall_headers() was called in cache_select_url() */
     cache->provider->recall_body(cache->handle, r->pool, bb);

     /* This filter is done once it has served up its content */
@@ -290,6 +290,12 @@
      * This section passes the brigades into the cache modules, but only
      * if the setup section (see below) is complete.
      */
+    if (cache->block_response) {
+        /* We've already sent down the EOS and response.  So, ignore
+         * whatever comes now.
+         */
+        return APR_SUCCESS;
+    }

     /* have we already run the cachability check and set up the
      * cached file handle?
@@ -312,7 +318,6 @@
      * parameters, and decides whether this URL should be cached at
      * all. This section is* run before the above section.
      */
-    info = apr_pcalloc(r->pool, sizeof(cache_info));

     /* read expiry date; if a bad date, then leave it so the client can
      * read it
@@ -332,7 +337,7 @@

     /* read the last-modified date; if the date is bad, then delete it */
     lastmods = apr_table_get(r->err_headers_out, "Last-Modified");
-    if (lastmods ==NULL) {
+    if (lastmods == NULL) {
         lastmods = apr_table_get(r->headers_out, "Last-Modified");
     }
     if (lastmods != NULL) {
@@ -384,7 +389,8 @@
          */
         reason = "Query string present but no expires header";
     }
-    else if (r->status == HTTP_NOT_MODIFIED && (NULL == cache->handle)) {
+    else if (r->status == HTTP_NOT_MODIFIED &&
+             !cache->handle && !cache->stale_handle) {
         /* if the server said 304 Not Modified but we have no cache
          * file - pass this untouched to the user agent, it's not for us.
          */
@@ -503,35 +509,32 @@
      * - cache->handle == NULL. In this case there is no previously
      * cached entity anywhere on the system. We must create a brand
      * new entity and store the response in it.
-     * - cache->handle != NULL. In this case there is a stale
+     * - cache->stale_handle != NULL. In this case there is a stale
      * entity in the system which needs to be replaced by new
      * content (unless the result was 304 Not Modified, which means
      * the cached entity is actually fresh, and we should update
      * the headers).
      */
+
+    /* Did we have a stale cache entry that really is stale? */
+    if (cache->stale_handle) {
+        if (r->status == HTTP_NOT_MODIFIED) {
+            /* Oh, hey.  It isn't that stale!  Yay! */
+            cache->handle = cache->stale_handle;
+        }
+        else {
+            /* Oh, well.  Toss it. */
+            cache->provider->remove_entity(cache->stale_handle);
+            /* Treat the request as if it wasn't conditional. */
+            cache->stale_handle = NULL;
+        }
+    }
+
     /* no cache handle, create a new entity */
     if (!cache->handle) {
         rv = cache_create_entity(r, url, size);
     }
-    /* pre-existing cache handle and 304, make entity fresh */
-    else if (r->status == HTTP_NOT_MODIFIED) {
-        /* update headers: TODO */

-        /* remove this filter ??? */
-
-        /* XXX is this right?  we must set rv to something other than OK
-         * in this path
-         */
-        rv = HTTP_NOT_MODIFIED;
-    }
-    /* pre-existing cache handle and new entity, replace entity
-     * with this one
-     */
-    else {
-        cache->provider->remove_entity(cache->handle);
-        rv = cache_create_entity(r, url, size);
-    }
-
     if (rv != OK) {
         /* Caching layer declined the opportunity to cache the response */
         ap_remove_output_filter(f);
@@ -550,6 +553,7 @@
      * In addition, we make HTTP/1.1 age calculations and write them away
      * too.
      */
+    info = apr_pcalloc(r->pool, sizeof(cache_info));

     /* Read the date. Generate one if one is not supplied */
     dates = apr_table_get(r->err_headers_out, "Date");
@@ -640,6 +644,36 @@
      * Write away header information to cache.
      */
     rv = cache->provider->store_headers(cache->handle, r, info);
+
+    /* Did we actually find an entity before, but it wasn't really stale? 
*/
+    if (rv == APR_SUCCESS && cache->stale_handle) {
+        apr_bucket_brigade *bb;
+        apr_bucket *bkt;
+
+        bb = apr_brigade_create(r->pool, r->connection->bucket_alloc);
+
+        /* Were we initially a conditional request? */
+        if (ap_cache_request_is_conditional(cache->stale_headers)) {
+            /* FIXME: Should we now go and make sure it's really not
+             * modified since what the user thought? */
+            bkt = apr_bucket_eos_create(bb->bucket_alloc);
+            APR_BRIGADE_INSERT_TAIL(bb, bkt);
+        }
+        else {
+            /* FIXME: This can't be correct!  But we can *not* return 304.
+             * Is it acceptable to return the cached status of what we had
+             * before?
+             * Add status field to cache_info?
+             * r->status = cache->stale_handle->status;
+             */
+            r->status = OK;
+            cache->provider->recall_body(cache->handle, r->pool, bb);
+        }
+
+        cache->block_response = 1;
+        return ap_pass_brigade(f->next, bb);
+    }
+
     if (rv == APR_SUCCESS) {
         rv = cache->provider->store_body(cache->handle, r, in);
     }
Index: modules/experimental/mod_cache.h
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/experimental/mod_cache.h,v
retrieving revision 1.50
diff -u -r1.50 mod_cache.h
--- modules/experimental/mod_cache.h	21 Sep 2004 22:56:23 -0000	1.50
+++ modules/experimental/mod_cache.h	24 Sep 2004 21:42:35 -0000
@@ -217,7 +217,10 @@
     const char *provider_name;              /* current cache provider name 
*/
     int fresh;				/* is the entitey fresh? */
     cache_handle_t *handle;		/* current cache handle */
-    int in_checked;			/* CACHE_IN must cache the entity */
+    cache_handle_t *stale_handle;	/* stale cache handle */
+    apr_table_t *stale_headers;		/* original request headers. */
+    int in_checked;			/* CACHE_SAVE must cache the entity */
+    int block_response;			/* CACHE_SAVE must block response. */
     apr_bucket_brigade *saved_brigade;  /* copy of partial response */
     apr_off_t saved_size;               /* length of saved_brigade */
     apr_time_t exp;                     /* expiration */
@@ -243,7 +246,7 @@
 CACHE_DECLARE(char *) generate_name(apr_pool_t *p, int dirlevels,
                                     int dirlength,
                                     const char *name);
-CACHE_DECLARE(int) ap_cache_request_is_conditional(request_rec *r);
+CACHE_DECLARE(int) ap_cache_request_is_conditional(apr_table_t *table);
 CACHE_DECLARE(cache_provider_list *)ap_cache_get_providers(request_rec *r, 
cache_server_conf *conf, const char *url);
 CACHE_DECLARE(int) ap_cache_liststr(apr_pool_t *p, const char *list,
                                     const char *key, char **val);



Mime
View raw message