From a7509ed3eb2540c75a8a0f0fffd84960b27e993d Mon Sep 17 00:00:00 2001 From: Evan Zelkowitz <19699200+ezelkow1@users.noreply.github.com> Date: Thu, 24 May 2018 14:33:13 -0600 Subject: [PATCH 1/3] Collapsed forwarding support in ATS core This adds a new write fail action, CACHE_WL_FAIL_READ_RETRY. If it fails to obtain a write lock then it will loop for max_open_read_retries, each time doing a cache read. In the end if it fails to ever either obtain a write lock or find the object to read then it does the default write fail action of going to the origin. On an origin fail it will do the default of sending back an ERR_CONNECT_FAIL --- doc/admin-guide/files/records.config.en.rst | 9 +------ proxy/http/HttpCacheSM.cc | 22 +++++++++++++--- proxy/http/HttpSM.cc | 12 +++++++-- proxy/http/HttpTransact.cc | 29 ++++++++++++++------- proxy/http/HttpTransact.h | 8 +++--- 5 files changed, 51 insertions(+), 29 deletions(-) diff --git a/doc/admin-guide/files/records.config.en.rst b/doc/admin-guide/files/records.config.en.rst index 8176406604e..4c17ec4a0a2 100644 --- a/doc/admin-guide/files/records.config.en.rst +++ b/doc/admin-guide/files/records.config.en.rst @@ -2304,14 +2304,7 @@ Dynamic Content & Content Negotiation ===== ====================================================================== ``0`` Default. Disable cache and go to origin server. ``1`` Return a ``502`` error on a cache miss. - ``2`` Serve stale if object's age is under - :ts:cv:`proxy.config.http.cache.max_stale_age`. Otherwise, go to - origin server. - ``3`` Return a ``502`` error on a cache miss or serve stale on a cache - revalidate if object's age is under - :ts:cv:`proxy.config.http.cache.max_stale_age`. Otherwise, go to - origin server. - ``4`` Return a ``502`` error on either a cache miss or on a revalidation. + ``2`` Retry a cache read. This will do another cache lookup for max_open_read_retries attempts, collapsing requests ===== ====================================================================== Customizable User Response Pages diff --git a/proxy/http/HttpCacheSM.cc b/proxy/http/HttpCacheSM.cc index 731ebf2ee76..ad82fd60044 100644 --- a/proxy/http/HttpCacheSM.cc +++ b/proxy/http/HttpCacheSM.cc @@ -171,6 +171,13 @@ HttpCacheSM::state_cache_open_write(int event, void *data) break; case CACHE_EVENT_OPEN_WRITE_FAILED: + if (master_sm->t_state.txn_conf->cache_open_write_fail_action == HttpTransact::CACHE_WL_FAIL_ACTION_READ_RETRY) { + // Use open read retry settings, if we dont fail here we will do write_retries*read_retries nested retries + Debug("http_cache", + "state cache openwrite, cachesm, got open_write_failed, wfail is read retry, set retries to max to schedule"); + open_read_tries = 0; + open_write_tries = master_sm->t_state.txn_conf->max_cache_open_write_retries; + } if (open_write_tries <= master_sm->t_state.txn_conf->max_cache_open_write_retries) { // Retry open write; open_write_cb = false; @@ -196,10 +203,17 @@ HttpCacheSM::state_cache_open_write(int event, void *data) "retrying cache open write...", master_sm->sm_id, open_write_tries); - open_write(&cache_key, lookup_url, read_request_hdr, master_sm->t_state.cache_info.object_read, - static_cast( - (master_sm->t_state.cache_control.pin_in_cache_for < 0) ? 0 : master_sm->t_state.cache_control.pin_in_cache_for), - retry_write, false); + if (master_sm->t_state.txn_conf->cache_open_write_fail_action != HttpTransact::CACHE_WL_FAIL_ACTION_READ_RETRY) { + open_write( + &cache_key, lookup_url, read_request_hdr, master_sm->t_state.cache_info.object_read, + (time_t)((master_sm->t_state.cache_control.pin_in_cache_for < 0) ? 0 : master_sm->t_state.cache_control.pin_in_cache_for), + retry_write, false); + } else { + Debug("http_cache", "[%" PRId64 "] [state_cache_open_write] cache open write failure INTERVAL. Saw CF, not attempting write", + master_sm->sm_id); + open_write_cb = false; + master_sm->handleEvent(CACHE_EVENT_OPEN_READ, data); + } break; default: diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc index 3bec8b592db..cfd8a9beb9f 100644 --- a/proxy/http/HttpSM.cc +++ b/proxy/http/HttpSM.cc @@ -2439,8 +2439,7 @@ HttpSM::state_cache_open_write(int event, void *data) break; } else { t_state.cache_open_write_fail_action = t_state.txn_conf->cache_open_write_fail_action; - if (!t_state.cache_info.object_read || - (t_state.cache_open_write_fail_action == HttpTransact::CACHE_WL_FAIL_ACTION_ERROR_ON_MISS_OR_REVALIDATE)) { + if (!t_state.cache_info.object_read) { // cache miss, set wl_state to fail SMDebug("http", "[%" PRId64 "] cache object read %p, cache_wl_fail_action %d", sm_id, t_state.cache_info.object_read, t_state.cache_open_write_fail_action); @@ -2453,6 +2452,14 @@ HttpSM::state_cache_open_write(int event, void *data) case CACHE_EVENT_OPEN_READ: // The write vector was locked and the cache_sm retried // and got the read vector again. + // We might not have an object here, if so we want to retry + if (!t_state.cache_info.object_read) { + ink_assert(t_state.cache_open_write_fail_action == HttpTransact::CACHE_WL_FAIL_ACTION_READ_RETRY); + t_state.cache_lookup_result = HttpTransact::CACHE_LOOKUP_NONE; + t_state.cache_info.write_lock_state = HttpTransact::CACHE_WL_READ_RETRY; + break; + } + cache_sm.cache_read_vc->get_http_info(&t_state.cache_info.object_read); // ToDo: Should support other levels of cache hits here, but the cache does not support it (yet) if (cache_sm.cache_read_vc->is_ram_cache_hit()) { @@ -2463,6 +2470,7 @@ HttpSM::state_cache_open_write(int event, void *data) ink_assert(t_state.cache_info.object_read != nullptr); t_state.source = HttpTransact::SOURCE_CACHE; + // clear up CACHE_LOOKUP_MISS, let Freshness function decide // hit status t_state.cache_lookup_result = HttpTransact::CACHE_LOOKUP_NONE; diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc index 2fbf7be4e6b..052284b0445 100644 --- a/proxy/http/HttpTransact.cc +++ b/proxy/http/HttpTransact.cc @@ -2874,8 +2874,6 @@ HttpTransact::handle_cache_write_lock(State *s) s->cache_info.action = CACHE_DO_NO_ACTION; switch (s->cache_open_write_fail_action) { case CACHE_WL_FAIL_ACTION_ERROR_ON_MISS: - case CACHE_WL_FAIL_ACTION_ERROR_ON_MISS_STALE_ON_REVALIDATE: - case CACHE_WL_FAIL_ACTION_ERROR_ON_MISS_OR_REVALIDATE: TxnDebug("http_error", "cache_open_write_fail_action %d, cache miss, return error", s->cache_open_write_fail_action); s->cache_info.write_status = CACHE_WRITE_ERROR; build_error_response(s, HTTP_STATUS_BAD_GATEWAY, "Connection Failed", "connect#failed_connect"); @@ -2897,6 +2895,14 @@ HttpTransact::handle_cache_write_lock(State *s) TRANSACT_RETURN(SM_ACTION_SEND_ERROR_CACHE_NOOP, nullptr); return; + /* + case CACHE_WL_FAIL_ACTION_READ_RETRY: + s->request_sent_time = UNDEFINED_TIME; + s->response_received_time = UNDEFINED_TIME; + s->cache_info.action = CACHE_DO_LOOKUP; + remove_ims = true; + TRANSACT_RETURN(SM_ACTION_CACHE_LOOKUP, nullptr); + return;*/ default: s->cache_info.write_status = CACHE_WRITE_LOCK_MISS; remove_ims = true; @@ -2911,7 +2917,17 @@ HttpTransact::handle_cache_write_lock(State *s) s->request_sent_time = UNDEFINED_TIME; s->response_received_time = UNDEFINED_TIME; s->cache_info.action = CACHE_DO_LOOKUP; - remove_ims = true; + if (!s->cache_info.object_read) { + ink_assert(s->cache_open_write_fail_action == HttpTransact::CACHE_WL_FAIL_ACTION_READ_RETRY); + s->cache_info.write_status = CACHE_WRITE_LOCK_MISS; + StateMachineAction_t next; + next = SM_ACTION_CACHE_LOOKUP; + s->next_action = next; + s->hdr_info.server_request.destroy(); + TRANSACT_RETURN(next, nullptr); + return; + } + remove_ims = true; SET_VIA_STRING(VIA_DETAIL_CACHE_TYPE, VIA_DETAIL_CACHE); break; case CACHE_WL_INIT: @@ -7079,13 +7095,6 @@ HttpTransact::what_is_document_freshness(State *s, HTTPHdr *client_request, HTTP uint32_t cc_mask, cooked_cc_mask; uint32_t os_specifies_revalidate; - if (s->cache_open_write_fail_action & CACHE_WL_FAIL_ACTION_STALE_ON_REVALIDATE) { - if (is_stale_cache_response_returnable(s)) { - TxnDebug("http_match", "[what_is_document_freshness] cache_serve_stale_on_write_lock_fail, return FRESH"); - return (FRESHNESS_FRESH); - } - } - ////////////////////////////////////////////////////// // If config file has a ttl-in-cache field set, // // it has priority over any other http headers and // diff --git a/proxy/http/HttpTransact.h b/proxy/http/HttpTransact.h index f7bfef58cad..3c54bd5544d 100644 --- a/proxy/http/HttpTransact.h +++ b/proxy/http/HttpTransact.h @@ -246,11 +246,9 @@ class HttpTransact }; enum CacheOpenWriteFailAction_t { - CACHE_WL_FAIL_ACTION_DEFAULT = 0x00, - CACHE_WL_FAIL_ACTION_ERROR_ON_MISS = 0x01, - CACHE_WL_FAIL_ACTION_STALE_ON_REVALIDATE = 0x02, - CACHE_WL_FAIL_ACTION_ERROR_ON_MISS_STALE_ON_REVALIDATE = 0x03, - CACHE_WL_FAIL_ACTION_ERROR_ON_MISS_OR_REVALIDATE = 0x04, + CACHE_WL_FAIL_ACTION_DEFAULT = 0x00, + CACHE_WL_FAIL_ACTION_ERROR_ON_MISS = 0x01, + CACHE_WL_FAIL_ACTION_READ_RETRY = 0x02, TOTAL_CACHE_WL_FAIL_ACTION_TYPES }; From 5f60f48da6f29a6c50e4dd831ccae309b8eb1c7e Mon Sep 17 00:00:00 2001 From: ezelko260 Date: Fri, 11 Oct 2019 19:15:34 +0000 Subject: [PATCH 2/3] Add increment on open write tries to ensure we skip at some point. Open write retries must be at least 1 for collapsing to enable --- proxy/http/HttpCacheSM.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proxy/http/HttpCacheSM.cc b/proxy/http/HttpCacheSM.cc index ad82fd60044..b569b2142d7 100644 --- a/proxy/http/HttpCacheSM.cc +++ b/proxy/http/HttpCacheSM.cc @@ -176,7 +176,7 @@ HttpCacheSM::state_cache_open_write(int event, void *data) Debug("http_cache", "state cache openwrite, cachesm, got open_write_failed, wfail is read retry, set retries to max to schedule"); open_read_tries = 0; - open_write_tries = master_sm->t_state.txn_conf->max_cache_open_write_retries; + open_write_tries = master_sm->t_state.txn_conf->max_cache_open_write_retries + 1; } if (open_write_tries <= master_sm->t_state.txn_conf->max_cache_open_write_retries) { // Retry open write; From 663bd984ea9b6cd8bb2a5d78f804a9d610d43608 Mon Sep 17 00:00:00 2001 From: Evan Zelkowitz <19699200+ezelkow1@users.noreply.github.com> Date: Wed, 16 Oct 2019 11:20:31 -0600 Subject: [PATCH 3/3] Remove commented case --- proxy/http/HttpTransact.cc | 8 -------- 1 file changed, 8 deletions(-) diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc index 052284b0445..9dfcbe28adf 100644 --- a/proxy/http/HttpTransact.cc +++ b/proxy/http/HttpTransact.cc @@ -2895,14 +2895,6 @@ HttpTransact::handle_cache_write_lock(State *s) TRANSACT_RETURN(SM_ACTION_SEND_ERROR_CACHE_NOOP, nullptr); return; - /* - case CACHE_WL_FAIL_ACTION_READ_RETRY: - s->request_sent_time = UNDEFINED_TIME; - s->response_received_time = UNDEFINED_TIME; - s->cache_info.action = CACHE_DO_LOOKUP; - remove_ims = true; - TRANSACT_RETURN(SM_ACTION_CACHE_LOOKUP, nullptr); - return;*/ default: s->cache_info.write_status = CACHE_WRITE_LOCK_MISS; remove_ims = true;