You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by r....@t-online.de on 2005/05/21 22:06:22 UTC

mod_cache deliver 304 instead of (not so) stale cache entries

Hi all,

I think I found a problem with mod_cache of the httpd trunk (revision 171201).
First off all I added the following configuration directives to the default httpd.conf:

CacheRoot /home/ruediger/apache_head/apache_trunk/cache
CacheEnable disk /test
CacheMaxFilesize 100000000
CacheDirLevels 5
CacheDirLength 3
CacheMaxExpire 60
CacheIgnoreNoLastMod On
CacheDefaultExpire 60
CacheIgnoreHeaders BLAH set-cookie

/test is a subdirectory below the htdocs document root and contains a single static file
called check.html. If I request the file for the first time everything is fine:


ruediger@euler:~> telnet 192.168.2.4 80
Trying 192.168.2.4...
Connected to 192.168.2.4.
Escape character is '^]'.
GET /test/check.html HTTP/1.1
Host: 192.168.2.4

HTTP/1.1 200 OK
Date: Sat, 21 May 2005 18:54:52 GMT
Server: Apache/2.1.5-dev (Unix) mod_ssl/2.1.5-dev OpenSSL/0.9.6g DAV/2
Last-Modified: Sat, 21 May 2005 18:54:43 GMT
ETag: "125f-29-5f22a2c0"
Accept-Ranges: bytes
Content-Length: 41
Expires: Sat, 21 May 2005 18:54:54 GMT
Content-Type: text/html

<html><body><h1>Check</h1></body></html>
Connection closed by foreign host.

But if I request it for a second time after 60 seconds (thus the cache entry is expired,
see CacheDefaultExpire) in the same way I get a 304:

ruediger@euler:~> telnet 192.168.2.4 80
Trying 192.168.2.4...
Connected to 192.168.2.4.
Escape character is '^]'.
GET /test/check.html HTTP/1.1
Host: 192.168.2.4

HTTP/1.1 304 Not Modified
Date: Sat, 21 May 2005 18:55:16 GMT
Server: Apache/2.1.5-dev (Unix) mod_ssl/2.1.5-dev OpenSSL/0.9.6g DAV/2
ETag: "125f-29-5f22a2c0"

Connection closed by foreign host.

Using two different browsers that would access /test/check.html with a time difference
of one minute this leads to an empty page in the second browser, since the browser does not
know how to handle a 304 for a first time request of a resource.

I found out that during the second request which returns a 304 the CACHE_SAVE filter,
which would be able to deal with such things (-> (not so) stale cache entries) is never
used.
The change of the conditionals in cache_storage.c starting at line 269 leads to the creation
of a 304 code in the default handler and the default handler does not pass 304 responses down
the filter chain.
So the 304 response is delivered instead of the (not so) stale cache entry. So I created the following
patch to cache_storage.c which prevents the modification or better creation of any conditionals
if the original request did not contain any:

--- cache_storage.c.orig        2005-05-21 10:28:28.000000000 +0200
+++ cache_storage.c     2005-05-21 21:56:27.000000000 +0200
@@ -254,6 +254,7 @@ int cache_select_url(request_rec *r, cha
             fresh = ap_cache_check_freshness(h, r);
             if (!fresh) {
                 const char *etag, *lastmod;
+                const char *ma_req, *mo_req, *nm_req, *ra_req, *us_req;

                 ap_log_error(APLOG_MARK, APLOG_DEBUG, APR_SUCCESS, r->server,
                   "Cached response for %s isn't fresh.  Adding/replacing "
@@ -263,6 +264,18 @@ int cache_select_url(request_rec *r, cha
                 cache->stale_headers = apr_table_copy(r->pool,
                                                       r->headers_in);

+                /* Check if revalidation is ok. Revalidation is not ok if
+                   the original request did not contain any conditionals */
+                ma_req = apr_table_get(r->headers_in, "If-Match");
+                mo_req = apr_table_get(r->headers_in, "If-Modified-Since");
+                nm_req = apr_table_get(r->headers_in, "If-None-Match");
+                ra_req = apr_table_get(r->headers_in, "If-Range");
+                us_req = apr_table_get(r->headers_in, "If-Unmodified-Since");
+
+                if (!(ma_req || mo_req || nm_req || ra_req || us_req)) {
+                   return DECLINED;
+                }
+
                 /* We can only revalidate with our own conditionals: remove the
                  * conditions from the original request.
                  */

I am aware that this forces a full request to the backend for requests without conditionals
to expired resources. So I am not very happy with this solution. Maybe it is better to let
the default handler pass 304 responses down the filter chain.

Some might also say that my configuration seems stupid (do disk caching for static resources,
which was actually born during some tests for another problem I am currently hunting), but
this problem also applies to other providers than mod_disk_cache and the document root might
be on a non local disk.

Comments/Thoughts/Flames?


Regards

RĂ¼diger

Re: mod_cache deliver 304 instead of (not so) stale cache entries

Posted by Sander Striker <st...@apache.org>.
> r.pluem@t-online.de wrote:
>> Sander Striker wrote:
>>> r.pluem@t-online.de wrote:

[...]
>> Might as well not do revalidation in that case; actually that would be
>> better, because the 304's that are returned may not even be correct.  The
>> conditions are replaced with the ones from the cache, remember?
 
> Yes, I remember, but I must admit that I am slightly confused now. When should
> we avoid revalidation with the conditionals from the cache?

Well, we shouldn't; it's a workaround for a different bug.  The workaround however
would be to _never_ use the conditionals from the cache.

> If the original request does not contain any conditionals? This is what my patch does

What I am trying to point out is that you can't use the conditionals from the
cache at all if the CACHE_SAVE filter isn't being invoked.  You will get a 304 based
on the conditionals from the cache, which may not be correct with respect to
the conditionals from the request.
 
> [..cut..]
>> I can see the application.  Are you up for submitting a patch to the
>> default handler? :)
 
> I have attached a patch for this. Two comments:
> 
> 1. I am not very familar with buckets and brigades, so please check closely if
>    I did it correct (my tests make me think so). If I did something wrong feedback
>    is appreciated such that I can do it better next time :-)
> 2. ap_meets_conditions returns 3 different values: OK, HTTP_PRECONDITION_FAILED and
>    HTTP_NOT_MODIFIED. In my patch I assume that in all cases the response should
>    go down the filter chain.

I'll review your patch.  I'll massage it into shape if needed, given your
comments.

Thanks,

Sander

Re: mod_cache deliver 304 instead of (not so) stale cache entries

Posted by r....@t-online.de.

Sander Striker wrote:
>> r.pluem@t-online.de wrote:
> 
> [...]

[...]

> 
>> I am aware that this forces a full request to the backend for requests
>> without conditionals
>> to expired resources. So I am not very happy with this solution. Maybe
>> it is better to let
>> the default handler pass 304 responses down the filter chain.
> 
> 
> Might as well not do revalidation in that case; actually that would be
> better, because the
> 304's that are returned may not even be correct.  The conditions are
> replaced with the
> ones from the cache, remember?
> 

Yes, I remember, but I must admit that I am slightly confused now. When should
we avoid revalidation with the conditionals from the cache?

If the original request does not contain any conditionals? This is what my patch does

[..cut..]

> 
> I can see the application.  Are you up for submitting a patch to the
> default handler? :)

I have attached a patch for this. Two comments:

1. I am not very familar with buckets and brigades, so please check closely if
   I did it correct (my tests make me think so). If I did something wrong feedback
   is appreciated such that I can do it better next time :-)
2. ap_meets_conditions returns 3 different values: OK, HTTP_PRECONDITION_FAILED and
   HTTP_NOT_MODIFIED. In my patch I assume that in all cases the response should
   go down the filter chain.

Regards

RĂ¼diger


Re: mod_cache deliver 304 instead of (not so) stale cache entries

Posted by Sander Striker <st...@apache.org>.
> r.pluem@t-online.de wrote:
[...]
> I found out that during the second request which returns a 304 the CACHE_SAVE filter,
> which would be able to deal with such things (-> (not so) stale cache entries) is never
> used.
> The change of the conditionals in cache_storage.c starting at line 269 leads to the creation
> of a 304 code in the default handler and the default handler does not pass 304 responses down
> the filter chain.

Then that is a bug.  We've seen the same problem in mod_proxy.

> So the 304 response is delivered instead of the (not so) stale cache entry. So I created the following
> patch to cache_storage.c which prevents the modification or better creation of any conditionals
> if the original request did not contain any:

No, this is what we were trying to prevent.
 
> I am aware that this forces a full request to the backend for requests without conditionals
> to expired resources. So I am not very happy with this solution. Maybe it is better to let
> the default handler pass 304 responses down the filter chain.

Might as well not do revalidation in that case; actually that would be better, because the
304's that are returned may not even be correct.  The conditions are replaced with the
ones from the cache, remember?
 
> Some might also say that my configuration seems stupid (do disk caching for static resources,
> which was actually born during some tests for another problem I am currently hunting), but
> this problem also applies to other providers than mod_disk_cache and the document root might
> be on a non local disk.

I can see the application.  Are you up for submitting a patch to the default handler? :)

Sander