This is an automated email from the ASF dual-hosted git repository. bcall pushed a commit to branch 8.0.x in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit 6ed35bcb8e042cb383aeca79863bdaaa8e4a828d Author: Bryan Call <bc...@apache.org> AuthorDate: Mon Feb 10 11:14:50 2020 -0800 Revert "Enhance Connection Collapse in ATS core" This reverts commit f76c4c77b09ca0675a82c2a0b50b0699bad8a43f. --- doc/admin-guide/files/records.config.en.rst | 9 ----- proxy/http/HttpCacheSM.cc | 55 ++++++++--------------------- proxy/http/HttpSM.cc | 9 ----- proxy/http/HttpTransact.cc | 22 ++++-------- proxy/http/HttpTransact.h | 1 - 5 files changed, 20 insertions(+), 76 deletions(-) diff --git a/doc/admin-guide/files/records.config.en.rst b/doc/admin-guide/files/records.config.en.rst index 0213466..3dc98a7 100644 --- a/doc/admin-guide/files/records.config.en.rst +++ b/doc/admin-guide/files/records.config.en.rst @@ -2181,9 +2181,6 @@ all the different user-agent versions of documents it encounters. The number of times to attempt a cache open write upon failure to get a write lock. - This config is ignored when :ts:cv:`proxy.config.http.cache.open_write_fail_action` is - set to ``5``. - .. ts:cv:: CONFIG proxy.config.http.cache.open_write_fail_action INT 0 :reloadable: :overridable: @@ -2206,12 +2203,6 @@ all the different user-agent versions of documents it encounters. :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. - ``5`` Retry Cache Read on a Cache Write Lock failure. This option together - with `proxy.config.cache.enable_read_while_writer` configuration - allows to collapse concurrent requests without a need for any plugin. - Make sure to configure Read While Writer feature correctly following - the docs in Cache Basics section. Note that this option may result in - CACHE_LOOKUP_COMPLETE HOOK being called back more than once. ===== ====================================================================== Customizable User Response Pages diff --git a/proxy/http/HttpCacheSM.cc b/proxy/http/HttpCacheSM.cc index daf39c1..52190bc 100644 --- a/proxy/http/HttpCacheSM.cc +++ b/proxy/http/HttpCacheSM.cc @@ -175,8 +175,7 @@ HttpCacheSM::state_cache_open_write(int event, void *data) { STATE_ENTER(&HttpCacheSM::state_cache_open_write, event); ink_assert(captive_action.cancelled == 0); - pending_action = nullptr; - bool read_retry_on_write_fail = false; + pending_action = nullptr; switch (event) { case CACHE_EVENT_OPEN_WRITE: @@ -188,26 +187,9 @@ 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) { - // fall back to open_read_tries - // Note that when CACHE_WL_FAIL_ACTION_READ_RETRY is configured, max_cache_open_write_retries - // is automatically ignored. Make sure to not disable max_cache_open_read_retries - // with CACHE_WL_FAIL_ACTION_READ_RETRY as this results in proxy'ing to origin - // without write retries in both a cache miss or a cache refresh scenario. - if (open_write_tries <= master_sm->t_state.txn_conf->max_cache_open_write_retries) { - Debug("http_cache", "[%" PRId64 "] [state_cache_open_write] cache open write failure %d. read retry triggered", - master_sm->sm_id, open_write_tries); - open_read_tries = 0; - read_retry_on_write_fail = true; - // make sure it doesn't loop indefinitely - open_write_tries = master_sm->t_state.txn_conf->max_cache_open_write_retries + 1; - } - } - if (read_retry_on_write_fail || 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; - // reset captive_action since HttpSM cancelled it - captive_action.cancelled = 0; do_schedule_in(); } else { // The cache is hosed or full or something. @@ -222,27 +204,18 @@ HttpCacheSM::state_cache_open_write(int event, void *data) break; case EVENT_INTERVAL: - if (master_sm->t_state.txn_conf->cache_open_write_fail_action == HttpTransact::CACHE_WL_FAIL_ACTION_READ_RETRY) { - Debug("http_cache", - "[%" PRId64 "] [state_cache_open_write] cache open write failure %d. " - "falling back to read retry...", - master_sm->sm_id, open_write_tries); - open_read_cb = false; - master_sm->handleEvent(CACHE_EVENT_OPEN_READ, data); - } else { - Debug("http_cache", - "[%" PRId64 "] [state_cache_open_write] cache open write failure %d. " - "retrying cache open write...", - master_sm->sm_id, open_write_tries); - - // Retry the cache open write if the number retries is less - // than or equal to the max number of open write retries - ink_assert(open_write_tries <= master_sm->t_state.txn_conf->max_cache_open_write_retries); - open_write(&cache_key, lookup_url, read_request_hdr, master_sm->t_state.cache_info.object_read, - static_cast<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); - } + // Retry the cache open write if the number retries is less + // than or equal to the max number of open write retries + ink_assert(open_write_tries <= master_sm->t_state.txn_conf->max_cache_open_write_retries); + Debug("http_cache", + "[%" PRId64 "] [state_cache_open_write] cache open write failure %d. " + "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, + (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); break; default: diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc index f7a70e9..fe05658 100644 --- a/proxy/http/HttpSM.cc +++ b/proxy/http/HttpSM.cc @@ -2392,15 +2392,6 @@ HttpSM::state_cache_open_write(int event, void *data) // INTENTIONAL FALL THROUGH // Allow for stale object to be served case CACHE_EVENT_OPEN_READ: - if (!t_state.cache_info.object_read) { - t_state.cache_open_write_fail_action = t_state.txn_conf->cache_open_write_fail_action; - // Note that CACHE_LOOKUP_COMPLETE may be invoked more than once - // if CACHE_WL_FAIL_ACTION_READ_RETRY is configured - 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; - } // The write vector was locked and the cache_sm retried // and got the read vector again. cache_sm.cache_read_vc->get_http_info(&t_state.cache_info.object_read); diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc index 3d14bda..c0ddf77 100644 --- a/proxy/http/HttpTransact.cc +++ b/proxy/http/HttpTransact.cc @@ -2885,6 +2885,7 @@ HttpTransact::handle_cache_write_lock(State *s) } TRANSACT_RETURN(SM_ACTION_SEND_ERROR_CACHE_NOOP, nullptr); + return; default: s->cache_info.write_status = CACHE_WRITE_LOCK_MISS; remove_ims = true; @@ -2892,25 +2893,14 @@ HttpTransact::handle_cache_write_lock(State *s) } break; case CACHE_WL_READ_RETRY: - s->request_sent_time = UNDEFINED_TIME; - s->response_received_time = UNDEFINED_TIME; - s->cache_info.action = CACHE_DO_LOOKUP; - if (!s->cache_info.object_read) { - // Write failed and read retry triggered - // Clean up server_request and re-initiate - // Cache Lookup - 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); - } // Write failed but retried and got a vector to read // We need to clean up our state so that transact does // not assert later on. Then handle the open read hit - remove_ims = true; + // + s->request_sent_time = UNDEFINED_TIME; + s->response_received_time = UNDEFINED_TIME; + s->cache_info.action = CACHE_DO_LOOKUP; + remove_ims = true; SET_VIA_STRING(VIA_DETAIL_CACHE_TYPE, VIA_DETAIL_CACHE); break; case CACHE_WL_INIT: diff --git a/proxy/http/HttpTransact.h b/proxy/http/HttpTransact.h index dfcd579..1178f41 100644 --- a/proxy/http/HttpTransact.h +++ b/proxy/http/HttpTransact.h @@ -249,7 +249,6 @@ public: 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_READ_RETRY = 0x05, TOTAL_CACHE_WL_FAIL_ACTION_TYPES };