From: Eric Covener Date: Tue, 8 Jul 2014 16:14:05 +0000 (+0000) Subject: backport some consistency with trunks mod_cache: X-Git-Tag: 2.2.28~69 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=213b94349fdd0640deec03f3e6c02efa7e527b6d;p=thirdparty%2Fapache%2Fhttpd.git backport some consistency with trunks mod_cache: * mod_cache, mod_disk_cache: Try to use the key of a possible open but stale cache entry if there is one. This fixes problem when two different cache locks have been created for single stale cache entry leading to two requests sent to backend. PR 50317 * Remove useless apr_file_remove() before renaming the cache entry in mod_disk_cache. This fixes small time-frame during which stale cache entry can be seen as not-cached. PR 50317 Subitted By: rpluem, jkaluza, ylavic Reviewed By: ylavic, rpluem, jkaluza git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.2.x@1608838 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/CHANGES b/CHANGES index d07443274d9..2d1b08a54b5 100644 --- a/CHANGES +++ b/CHANGES @@ -1,12 +1,17 @@ -*- coding: utf-8 -*- Changes with Apache 2.2.28 + *) mod_cache, mod_disk_cache: With CacheLock enabled, responses with a Vary + header might not get the benefit of the thundering herd protection due to + an incorrect internal cache key. PR 50317. + [Ruediger Pluem, Jan Kaluza, Yann Ylavic] + *) mod_rewrite: Support session cookies with the CO= flag when later parameters are used. The doc for this implied the feature had been backported for quite some time. PR56014 [Eric Covener] *) mod_cache: Don't remove stale cache entries that cannot be conditionally - revalidated. This prevents the thundring herd protection from serving + revalidated. This prevents the thundering herd protection from serving stale responses during a revalidation. PR 50317. [Eric Covener, Jan Kaluza, Ruediger Pluem] diff --git a/STATUS b/STATUS index a844325b205..ac2269d4e2b 100644 --- a/STATUS +++ b/STATUS @@ -99,16 +99,6 @@ RELEASE SHOWSTOPPERS: PATCHES ACCEPTED TO BACKPORT FROM TRUNK: [ start all new proposals below, under PATCHES PROPOSED. ] - * mod_cache, mod_disk_cache: Try to use the key of a possible open but - stale cache entry if there is one. This fixes problem when two different - cache locks have been created for single stale cache entry leading to two - requests sent to backend. - Remove useless apr_file_remove() before renaming the cache entry in - mod_disk_cache. This fixes small time-frame during which stale cache - entry can be seen as not-cached. - PR 50317 - 2.2.x patch: http://people.apache.org/~jkaluza/patches/httpd-2.2.x-thundering-herd.patch - +1: ylavic, rpluem, jkaluza PATCHES PROPOSED TO BACKPORT FROM TRUNK: [ New proposals should be added at the end of the list ] diff --git a/modules/cache/mod_cache.c b/modules/cache/mod_cache.c index d2dfff9622d..8492374298a 100644 --- a/modules/cache/mod_cache.c +++ b/modules/cache/mod_cache.c @@ -113,6 +113,27 @@ static int cache_url_handler(request_rec *r, int lookup) if (rv != OK) { if (rv == DECLINED) { if (!lookup) { + char *key; + cache_handle_t *h; + + /* + * Try to use the key of a possible open but stall cache + * entry if we have one. + */ + if (cache->handle != NULL) { + h = cache->handle; + } + else { + h = cache->stale_handle; + } + if ((h != NULL) && + (h->cache_obj != NULL) && + (h->cache_obj->key != NULL)) { + key = apr_pstrdup(r->pool, h->cache_obj->key); + } + else { + key = NULL; + } /* try to obtain a cache lock at this point. if we succeed, * we are the first to try and cache this url. if we fail, @@ -121,7 +142,7 @@ static int cache_url_handler(request_rec *r, int lookup) * backend without any attempt to cache. this stops * duplicated simultaneous attempts to cache an entity. */ - rv = ap_cache_try_lock(conf, r, NULL); + rv = ap_cache_try_lock(conf, r, key); if (APR_SUCCESS == rv) { /* diff --git a/modules/cache/mod_disk_cache.c b/modules/cache/mod_disk_cache.c index 13d6c8b300b..305fa6b293e 100644 --- a/modules/cache/mod_disk_cache.c +++ b/modules/cache/mod_disk_cache.c @@ -962,15 +962,6 @@ static apr_status_t store_headers(cache_handle_t *h, request_rec *r, cache_info apr_file_close(dobj->hfd); /* flush and close */ - /* Remove old file with the same name. If remove fails, then - * perhaps we need to create the directory tree where we are - * about to write the new headers file. - */ - rv = apr_file_remove(dobj->hdrsfile, r->pool); - if (rv != APR_SUCCESS) { - mkdir_structure(conf, dobj->hdrsfile, r->pool); - } - rv = safe_file_rename(conf, dobj->tempfile, dobj->hdrsfile, r->pool); if (rv != APR_SUCCESS) { ap_log_error(APLOG_MARK, APLOG_WARNING, rv, r->server,