diff --git a/doc/admin-guide/files/records.config.en.rst b/doc/admin-guide/files/records.config.en.rst index 54a8d823902..16f9a8921cd 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 ac550a95fd6..5dfc88fec4a 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 5bcc583dd69..5c7d40aadf4 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 025b8ee9e85..3d503b6a238 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(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( (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 3a35a84db2b..d727ef87ca6 100644 --- a/proxy/http/HttpCacheSM.h +++ b/proxy/http/HttpCacheSM.h @@ -211,6 +211,8 @@ class HttpCacheSM : public Continuation 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 @@ class HttpCacheSM : public Continuation 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 8f64efbbdf5..d7798dbb1c0 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 dad11115613..03c4904a02c 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 5b6f3db5817..cdc83c3aeb4 100644 --- a/src/shared/overridable_txn_vars.cc +++ b/src/shared/overridable_txn_vars.cc @@ -110,6 +110,8 @@ const std::unordered_mapmax_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 21d61abfa50..f6b07008292 100644 --- a/src/traffic_server/InkAPITest.cc +++ b/src/traffic_server/InkAPITest.cc @@ -8665,6 +8665,7 @@ std::array 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",