You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by bc...@apache.org on 2020/02/10 19:19:12 UTC
[trafficserver] 01/02: Revert "Enhance Connection Collapse in ATS
core"
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
};