httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "William A. Rowe, Jr." <wr...@rowe-clan.net>
Subject Re: [PATCH] mod_cache, don't always run as a quick handler.
Date Thu, 12 May 2005 16:34:38 GMT
Let me suggest, instead, that every proxy between here and
Timbuktu suffers the same problem from your example.

Better, methinks, is for mod_rewrite to toggle Vary: for the
envvar that triggered its rewrite.  This is symptomatic of a
more sinister side effect of mod_rewrite.

Bill

At 03:29 AM 5/12/2005, Paul Querna wrote:
>Okay, because of the quirky behavior of a 'sometimes' cached page, this
>one had me going in circles for a little while.
>
>What this patch does, is add a new command to mod_cache,
>'CacheRunAfterOthers_RenameThisCmd'[1]
>
>The effect this has is to run mod_cache as a normal handler, instead of
>a Quick Handler.  This means all the other Translate Name, Map to
>Storage, and Fixup hooks are ran.  These include mod_rewrite and
>mod_alias, among others.
>
>What this allows, is configurations like the following:
>CacheEnable disk /
>CacheRoot /tmp/cacheroot
>RewriteEngine On
>CacheIgnoreNoLastMod on
>CacheDefaultExpire 90
>CacheMaxExpire 90
>RewriteCond %{HTTP_USER_AGENT} ^Mozilla.*\sFirefox/1\.0\.[1234]\b
>RewriteRule .* http://www.mozilla.org/products/firefox/upgrade/ [R=302,L]
>
>Since mod_cache is ran as a quick handler, once the / page was cached,
>the RewriteRule would never be ran.  So, after the first non-Firefox
>Client connected, all clients, even those that are Firefox, would get a
>cached page, not the redirect.
>
>Add this:
>CacheRunAfterOthers_RenameThisCmd on
>
>And now it works. It does take a slight performance hit, but this is
>nothing compared to using a mod_proxy backend.
>
>Moving the Cache handler to be ran as an APR_HOOK_REALLY_FIRST means
>mod_rewrite will get it's chance to rewrite it, but it will still run
>before a really expensive handler, like a Proxy Backend or a PHP Script.
> This won't be useful to everyone, but for most configurations, the
>initial hooks are insignificant compared to the Content Handler, so
>allowing mod_cache to be called like this still makes sense.
>
>I have lightly tested the attached patch, and it fixes my test cases.
>
>Some details on this are also in PR 34885:
>http://issues.apache.org/bugzilla/show_bug.cgi?id=34885
>
>Thoughts/Comments/Code Review... all welcome.
>
>-Paul
>
>[1] Yes, I did name it 'CacheRunAfterOthers_RenameThisCmd'.  I hope that
>before this patch is committed, someone thinks of a good name for this
>behavior, because I can't think of one.
>
>Index: modules/cache/mod_cache.c ===================================================================
--- modules/cache/mod_cache.c      (revision 169788) +++ modules/cache/mod_cache.c (working
copy) @@ -45,7 +45,8 @@   *     oh well.   */ -static int cache_url_handler(request_rec *r,
int lookup) +static int cache_url_real_handler(request_rec *r, int lookup, +             
                    cache_server_conf *conf, int is_quick) {      apr_status_t rv;      const
char *auth; @@ -55,22 +56,13 @@      cache_provider_list *providers;      cache_info *info;
     cache_request_rec *cache; -    cache_server_conf *conf;      apr_bucket_brigade *out;
-    /* Delay initialization until we know we are handling a GET */ -    if (r->method_number
!= M_GET) { -        return DECLINED; -    } -      uri = r->parsed_uri;      url = r->unparsed_uri;
     path = uri.path;      info = NULL; -    conf = (cache_server_conf *) ap_get_module_config(r->server->module_config,
-                                                      &cache_module); -      /*     
 * Which cache module (if any) should handle this request?       */ @@ -156,9 +148,12 @@ 
    /* We are in the quick handler hook, which means that no output       * filters have been
set. So lets run the insert_filter hook.       */ -    ap_run_insert_filter(r); +    if (is_quick)
{ +        ap_run_insert_filter(r); +    } +      ap_add_output_filter_handle(cache_out_filter_handle,
NULL, -                                r, r->connection); +                           
        r, r->connection);      /* kick off the filter stack */      out = apr_brigade_create(r->pool,
r->connection->bucket_alloc); @@ -174,6 +169,45 @@      return OK; } +static int cache_url_quick_handler(request_rec
*r, int lookup) +{ +    cache_server_conf *conf; + +    /* Delay initialization until we know
we are handling a GET */ +    if (r->method_number != M_GET) { +        return DECLINED;
+    } + +    conf = (cache_server_conf *) ap_get_module_config(r->server->module_config,
+                                                      &cache_module); + +    if (conf->run_handler_later)
{ +        return DECLINED; +    } + +    return cache_url_real_handler(r, lookup, conf, 1);
+} + +static int cache_url_late_handler(request_rec *r) +{ +    cache_server_conf *conf; +
+    /* Delay initialization until we know we are handling a GET */ +    if (r->method_number
!= M_GET) { +        return DECLINED; +    } + +    conf = (cache_server_conf *) ap_get_module_config(r->server->module_config,
+                                                      &cache_module); + +    if (conf->run_handler_later)
{ +        return cache_url_real_handler(r, 0, conf, 0); +    } + +    return DECLINED; +}
+ + /*   * CACHE_OUT filter   * ---------------- @@ -742,6 +776,7 @@      /* array of headers
that should not be stored in cache */      ps->ignore_headers = apr_array_make(p, 10, sizeof(char
*));      ps->ignore_headers_set = CACHE_IGNORE_HEADERS_UNSET; +    ps->run_handler_later
= 0;      return ps; } @@ -787,6 +822,10 @@          (overrides->ignore_headers_set ==
CACHE_IGNORE_HEADERS_UNSET)          ? base->ignore_headers          : overrides->ignore_headers;
+    ps->run_handler_later = +        (overrides->run_handler_later == 0) +        ?
base->run_handler_later +        : overrides->run_handler_later;      return ps; } static
const char *set_cache_ignore_no_last_mod(cmd_parms *parms, void *dummy, @@ -829,6 +868,18
@@      return NULL; } +static const char *set_cache_run_later(cmd_parms *parms, void *dummy,
+                                           int flag) +{ +    cache_server_conf *conf; + +
   conf = +        (cache_server_conf *)ap_get_module_config(parms->server->module_config,
+                                                  &cache_module); +    conf->run_handler_later
= flag; +    return NULL; +} + static const char *set_cache_store_nostore(cmd_parms *parms,
void *dummy,                                             int flag) { @@ -993,6 +1044,9 @@
     AP_INIT_FLAG("CacheStorePrivate", set_cache_store_private,                   NULL, RSRC_CONF,
                  "Ignore 'Cache-Control: private' and store private content"), +    AP_INIT_FLAG("CacheRunAfterOthers_RenameThisCmd",
set_cache_run_later, +                 NULL, RSRC_CONF, +                 "Run the Cache Handler
as a Normal Handler."),      AP_INIT_FLAG("CacheStoreNoStore", set_cache_store_nostore,  
                NULL, RSRC_CONF,                   "Ignore 'Cache-Control: no-store' and store
sensitive content"), @@ -1007,9 +1061,12 @@ static void register_hooks(apr_pool_t *p) { -
   /* cache initializer */ -    /* cache handler */ -    ap_hook_quick_handler(cache_url_handler,
NULL, NULL, APR_HOOK_FIRST); +    /* cache handlers */ +    ap_hook_quick_handler(cache_url_quick_handler,
NULL, NULL, +                          APR_HOOK_FIRST); +    ap_hook_handler(cache_url_late_handler,
NULL, NULL, +                    APR_HOOK_REALLY_FIRST); +      /* cache filters       * XXX
The cache filters need to run right after the handlers and before       * any other filters.
Consider creating AP_FTYPE_CACHE for this purpose. @@ -1030,6 +1087,8 @@                 
                  cache_out_filter,                                    NULL,             
                      AP_FTYPE_CONTENT_SET-1); + +    /* cache initializer */      ap_hook_post_config(cache_post_config,
NULL, NULL, APR_HOOK_REALLY_FIRST); } Index: modules/cache/mod_cache.h ===================================================================
--- modules/cache/mod_cache.h       (revision 169788) +++ modules/cache/mod_cache.h (working
copy) @@ -145,6 +145,8 @@      #define CACHE_IGNORE_HEADERS_SET   1      #define CACHE_IGNORE_HEADERS_UNSET
0      int ignore_headers_set; +    /** Flag to disable the Quick Handler, and run after meta-hooks
*/ +    int run_handler_later; } cache_server_conf; /* cache info information */ 



Mime
View raw message