You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by bn...@apache.org on 2022/04/20 15:38:24 UTC
[trafficserver] branch master updated: add proxy.config.http.cache.max_open_write_retry_timeout parameter (#8591)
This is an automated email from the ASF dual-hosted git repository.
bnolsen pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push:
new 982e0ed0f add proxy.config.http.cache.max_open_write_retry_timeout parameter (#8591)
982e0ed0f is described below
commit 982e0ed0f6c4e0c9b7f532d29595d4df67d94195
Author: Brian Olsen <br...@comcast.com>
AuthorDate: Wed Apr 20 09:38:19 2022 -0600
add proxy.config.http.cache.max_open_write_retry_timeout parameter (#8591)
---
doc/admin-guide/files/records.config.en.rst | 9 +++++++
include/ts/apidefs.h.in | 1 +
plugins/lua/ts_lua_http_config.c | 2 ++
proxy/http/HttpCacheSM.cc | 42 ++++++++++++++++++++++-------
proxy/http/HttpCacheSM.h | 7 +++--
proxy/http/HttpConfig.cc | 2 ++
proxy/http/HttpConfig.h | 5 ++--
src/shared/overridable_txn_vars.cc | 2 ++
src/traffic_server/InkAPI.cc | 3 +++
src/traffic_server/InkAPITest.cc | 1 +
10 files changed, 61 insertions(+), 13 deletions(-)
diff --git a/doc/admin-guide/files/records.config.en.rst b/doc/admin-guide/files/records.config.en.rst
index c311690c8..af7f2a2a3 100644
--- a/doc/admin-guide/files/records.config.en.rst
+++ b/doc/admin-guide/files/records.config.en.rst
@@ -2539,6 +2539,15 @@ Dynamic Content & Content Negotiation
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`` or :ts:cv:`proxy.config.http.cache.max_open_write_retry_timeout` is set to gt ``0``.
+
+.. ts:cv:: CONFIG proxy.config.http.cache.max_open_write_retry_timeout INT 0
+ :reloadable:
+ :overridable:
+
+ A timeout for attempting 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``.
diff --git a/include/ts/apidefs.h.in b/include/ts/apidefs.h.in
index ac550a95f..5dfc88fec 100644
--- a/include/ts/apidefs.h.in
+++ b/include/ts/apidefs.h.in
@@ -825,6 +825,7 @@ typedef enum {
TS_CONFIG_HTTP_CACHE_OPEN_WRITE_FAIL_ACTION,
TS_CONFIG_HTTP_NUMBER_OF_REDIRECTIONS,
TS_CONFIG_HTTP_CACHE_MAX_OPEN_WRITE_RETRIES,
+ TS_CONFIG_HTTP_CACHE_MAX_OPEN_WRITE_RETRY_TIMEOUT,
TS_CONFIG_HTTP_REDIRECT_USE_ORIG_CACHE_KEY,
TS_CONFIG_HTTP_ATTACH_SERVER_SESSION_TO_CLIENT,
TS_CONFIG_WEBSOCKET_NO_ACTIVITY_TIMEOUT,
diff --git a/plugins/lua/ts_lua_http_config.c b/plugins/lua/ts_lua_http_config.c
index 5bcc583dd..5c7d40aad 100644
--- a/plugins/lua/ts_lua_http_config.c
+++ b/plugins/lua/ts_lua_http_config.c
@@ -109,6 +109,7 @@ typedef enum {
TS_LUA_CONFIG_HTTP_CACHE_OPEN_WRITE_FAIL_ACTION = TS_CONFIG_HTTP_CACHE_OPEN_WRITE_FAIL_ACTION,
TS_LUA_CONFIG_HTTP_NUMBER_OF_REDIRECTIONS = TS_CONFIG_HTTP_NUMBER_OF_REDIRECTIONS,
TS_LUA_CONFIG_HTTP_CACHE_MAX_OPEN_WRITE_RETRIES = TS_CONFIG_HTTP_CACHE_MAX_OPEN_WRITE_RETRIES,
+ TS_LUA_CONFIG_HTTP_CACHE_MAX_OPEN_WRITE_RETRY_TIMEOUT = TS_CONFIG_HTTP_CACHE_MAX_OPEN_WRITE_RETRY_TIMEOUT,
TS_LUA_CONFIG_HTTP_REDIRECT_USE_ORIG_CACHE_KEY = TS_CONFIG_HTTP_REDIRECT_USE_ORIG_CACHE_KEY,
TS_LUA_CONFIG_HTTP_ATTACH_SERVER_SESSION_TO_CLIENT = TS_CONFIG_HTTP_ATTACH_SERVER_SESSION_TO_CLIENT,
TS_LUA_CONFIG_HTTP_MAX_PROXY_CYCLES = TS_CONFIG_HTTP_MAX_PROXY_CYCLES,
@@ -246,6 +247,7 @@ ts_lua_var_item ts_lua_http_config_vars[] = {
TS_LUA_MAKE_VAR_ITEM(TS_LUA_CONFIG_HTTP_CACHE_OPEN_WRITE_FAIL_ACTION),
TS_LUA_MAKE_VAR_ITEM(TS_LUA_CONFIG_HTTP_NUMBER_OF_REDIRECTIONS),
TS_LUA_MAKE_VAR_ITEM(TS_LUA_CONFIG_HTTP_CACHE_MAX_OPEN_WRITE_RETRIES),
+ TS_LUA_MAKE_VAR_ITEM(TS_LUA_CONFIG_HTTP_CACHE_MAX_OPEN_WRITE_RETRY_TIMEOUT),
TS_LUA_MAKE_VAR_ITEM(TS_LUA_CONFIG_HTTP_REDIRECT_USE_ORIG_CACHE_KEY),
TS_LUA_MAKE_VAR_ITEM(TS_LUA_CONFIG_HTTP_ATTACH_SERVER_SESSION_TO_CLIENT),
TS_LUA_MAKE_VAR_ITEM(TS_LUA_CONFIG_HTTP_MAX_PROXY_CYCLES),
diff --git a/proxy/http/HttpCacheSM.cc b/proxy/http/HttpCacheSM.cc
index 025b8ee9e..3d503b6a2 100644
--- a/proxy/http/HttpCacheSM.cc
+++ b/proxy/http/HttpCacheSM.cc
@@ -159,6 +159,19 @@ HttpCacheSM::state_cache_open_read(int event, void *data)
return VC_EVENT_CONT;
}
+bool
+HttpCacheSM::write_retry_done() const
+{
+ MgmtInt const timeout_ms = master_sm->t_state.txn_conf->max_cache_open_write_retry_timeout;
+ if (0 < timeout_ms && 0 < open_write_start) {
+ ink_hrtime const elapsed = Thread::get_hrtime() - open_write_start;
+ MgmtInt const msecs = ink_hrtime_to_msec(elapsed);
+ return timeout_ms < msecs;
+ } else {
+ return master_sm->t_state.txn_conf->max_cache_open_write_retries < open_write_tries;
+ }
+}
+
int
HttpCacheSM::state_cache_open_write(int event, void *data)
{
@@ -180,14 +193,15 @@ HttpCacheSM::state_cache_open_write(int event, void *data)
master_sm->handleEvent(event, &captive_action);
break;
- case CACHE_EVENT_OPEN_WRITE_FAILED:
+ case CACHE_EVENT_OPEN_WRITE_FAILED: {
if (master_sm->t_state.txn_conf->cache_open_write_fail_action == 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) {
+
+ if (write_retry_done()) {
Debug("http_cache", "[%" PRId64 "] [state_cache_open_write] cache open write failure %d. read retry triggered",
master_sm->sm_id, open_write_tries);
if (master_sm->t_state.txn_conf->max_cache_open_read_retries <= 0) {
@@ -196,13 +210,19 @@ HttpCacheSM::state_cache_open_write(int event, void *data)
" read retry, but, max_cache_open_read_retries is not enabled",
master_sm->sm_id);
}
- open_read_tries = 0;
+ 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;
+ open_write_tries = master_sm->t_state.txn_conf->max_cache_open_write_retries + 1;
+ MgmtInt const retry_timeout = master_sm->t_state.txn_conf->max_cache_open_write_retry_timeout;
+ if (0 < retry_timeout) {
+ open_write_start = Thread::get_hrtime() - ink_hrtime_from_msec(retry_timeout);
+ }
}
}
- if (read_retry_on_write_fail || open_write_tries <= master_sm->t_state.txn_conf->max_cache_open_write_retries) {
+
+ if (read_retry_on_write_fail || !write_retry_done()) {
// Retry open write;
open_write_cb = false;
do_schedule_in();
@@ -217,7 +237,7 @@ HttpCacheSM::state_cache_open_write(int event, void *data)
err_code = reinterpret_cast<intptr_t>(data);
master_sm->handleEvent(event, &captive_action);
}
- break;
+ } break;
case EVENT_INTERVAL:
if (master_sm->t_state.txn_conf->cache_open_write_fail_action == CACHE_WL_FAIL_ACTION_READ_RETRY) {
@@ -235,7 +255,8 @@ HttpCacheSM::state_cache_open_write(int event, void *data)
// 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);
+ ink_assert(!write_retry_done());
+
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),
@@ -344,6 +365,9 @@ HttpCacheSM::open_write(const HttpCacheKey *key, URL *url, HTTPHdr *request, Cac
// INKqa12119
open_write_cb = false;
open_write_tries++;
+ if (0 == open_write_start) {
+ open_write_start = Thread::get_hrtime();
+ }
this->retry_write = retry;
// We should be writing the same document we did
@@ -360,8 +384,8 @@ HttpCacheSM::open_write(const HttpCacheKey *key, URL *url, HTTPHdr *request, Cac
// a new write (could happen on a very busy document
// that must be revalidated every time)
// Changed by YTS Team, yamsat Plugin
- if (open_write_tries > master_sm->redirection_tries &&
- open_write_tries > master_sm->t_state.txn_conf->max_cache_open_write_retries) {
+ // two criteria, either write retries over the amount OR timeout
+ if (open_write_tries > master_sm->redirection_tries && write_retry_done()) {
err_code = -ECACHE_DOC_BUSY;
master_sm->handleEvent(CACHE_EVENT_OPEN_WRITE_FAILED, &captive_action);
return ACTION_RESULT_DONE;
diff --git a/proxy/http/HttpCacheSM.h b/proxy/http/HttpCacheSM.h
index 3a35a84db..d727ef87c 100644
--- a/proxy/http/HttpCacheSM.h
+++ b/proxy/http/HttpCacheSM.h
@@ -211,6 +211,8 @@ private:
void do_schedule_in();
Action *do_cache_open_read(const HttpCacheKey &);
+ bool write_retry_done() const;
+
int state_cache_open_read(int event, void *data);
int state_cache_open_write(int event, void *data);
@@ -225,8 +227,9 @@ private:
time_t read_pin_in_cache = 0;
// Open write parameters
- bool retry_write = true;
- int open_write_tries = 0;
+ bool retry_write = true;
+ int open_write_tries = 0;
+ ink_hrtime open_write_start = 0; // overrides open_write_tries
// Common parameters
URL *lookup_url = nullptr;
diff --git a/proxy/http/HttpConfig.cc b/proxy/http/HttpConfig.cc
index 8f64efbbd..d7798dbb1 100644
--- a/proxy/http/HttpConfig.cc
+++ b/proxy/http/HttpConfig.cc
@@ -1315,6 +1315,8 @@ HttpConfig::startup()
// open write failure retries
HttpEstablishStaticConfigLongLong(c.oride.max_cache_open_write_retries, "proxy.config.http.cache.max_open_write_retries");
+ HttpEstablishStaticConfigLongLong(c.oride.max_cache_open_write_retry_timeout,
+ "proxy.config.http.cache.max_open_write_retry_timeout");
HttpEstablishStaticConfigByte(c.oride.cache_http, "proxy.config.http.cache.http");
HttpEstablishStaticConfigByte(c.oride.cache_ignore_client_no_cache, "proxy.config.http.cache.ignore_client_no_cache");
diff --git a/proxy/http/HttpConfig.h b/proxy/http/HttpConfig.h
index dad111156..03c4904a0 100644
--- a/proxy/http/HttpConfig.h
+++ b/proxy/http/HttpConfig.h
@@ -693,11 +693,12 @@ struct OverridableHttpConfigParams {
// open read failure retries.
MgmtInt max_cache_open_read_retries = -1;
- MgmtInt cache_open_read_retry_time = 10; // time is in mseconds
+ MgmtInt cache_open_read_retry_time = 10; // time in mseconds
MgmtInt cache_generation_number = -1;
// open write failure retries.
- MgmtInt max_cache_open_write_retries = 1;
+ MgmtInt max_cache_open_write_retries = 1;
+ MgmtInt max_cache_open_write_retry_timeout = 0; // time in mseconds
MgmtInt background_fill_active_timeout = 60;
diff --git a/src/shared/overridable_txn_vars.cc b/src/shared/overridable_txn_vars.cc
index 5b6f3db58..cdc83c3ae 100644
--- a/src/shared/overridable_txn_vars.cc
+++ b/src/shared/overridable_txn_vars.cc
@@ -110,6 +110,8 @@ const std::unordered_map<std::string_view, std::tuple<const TSOverridableConfigK
{"proxy.config.http.insert_squid_x_forwarded_for", {TS_CONFIG_HTTP_INSERT_SQUID_X_FORWARDED_FOR, TS_RECORDDATATYPE_INT}},
{"proxy.config.http.connect_attempts_max_retries", {TS_CONFIG_HTTP_CONNECT_ATTEMPTS_MAX_RETRIES, TS_RECORDDATATYPE_INT}},
{"proxy.config.http.cache.max_open_write_retries", {TS_CONFIG_HTTP_CACHE_MAX_OPEN_WRITE_RETRIES, TS_RECORDDATATYPE_INT}},
+ {"proxy.config.http.cache.max_open_write_retry_timeout",
+ {TS_CONFIG_HTTP_CACHE_MAX_OPEN_WRITE_RETRY_TIMEOUT, TS_RECORDDATATYPE_INT}},
{"proxy.config.http.forward.proxy_auth_to_parent", {TS_CONFIG_HTTP_FORWARD_PROXY_AUTH_TO_PARENT, TS_RECORDDATATYPE_INT}},
{"proxy.config.http.parent_proxy.mark_down_hostdb", {TS_CONFIG_PARENT_FAILURES_UPDATE_HOSTDB, TS_RECORDDATATYPE_INT}},
{"proxy.config.http.negative_revalidating_enabled", {TS_CONFIG_HTTP_NEGATIVE_REVALIDATING_ENABLED, TS_RECORDDATATYPE_INT}},
diff --git a/src/traffic_server/InkAPI.cc b/src/traffic_server/InkAPI.cc
index f8d92e722..1779487a6 100644
--- a/src/traffic_server/InkAPI.cc
+++ b/src/traffic_server/InkAPI.cc
@@ -8922,6 +8922,9 @@ _conf_to_memberp(TSOverridableConfigKey conf, OverridableHttpConfigParams *overr
case TS_CONFIG_HTTP_CACHE_MAX_OPEN_WRITE_RETRIES:
ret = _memberp_to_generic(&overridableHttpConfig->max_cache_open_write_retries, conv);
break;
+ case TS_CONFIG_HTTP_CACHE_MAX_OPEN_WRITE_RETRY_TIMEOUT:
+ ret = _memberp_to_generic(&overridableHttpConfig->max_cache_open_write_retry_timeout, conv);
+ break;
case TS_CONFIG_HTTP_REDIRECT_USE_ORIG_CACHE_KEY:
ret = _memberp_to_generic(&overridableHttpConfig->redirect_use_orig_cache_key, conv);
break;
diff --git a/src/traffic_server/InkAPITest.cc b/src/traffic_server/InkAPITest.cc
index 21d61abfa..f6b070082 100644
--- a/src/traffic_server/InkAPITest.cc
+++ b/src/traffic_server/InkAPITest.cc
@@ -8665,6 +8665,7 @@ std::array<std::string_view, TS_CONFIG_LAST_ENTRY> SDK_Overridable_Configs = {
"proxy.config.http.cache.open_write_fail_action",
"proxy.config.http.number_of_redirections",
"proxy.config.http.cache.max_open_write_retries",
+ "proxy.config.http.cache.max_open_write_retry_timeout",
"proxy.config.http.redirect_use_orig_cache_key",
"proxy.config.http.attach_server_session_to_client",
"proxy.config.websocket.no_activity_timeout",