This is an automated email from the ASF dual-hosted git repository.
cmcfarlen pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/10.1.x by this push:
new c8668c6354 Add proxy.config.http.negative_revalidating_list (#12212)
c8668c6354 is described below
commit c8668c635429188dd3eae04f3ee78a12412927d3
Author: Masaori Koshiba <[email protected]>
AuthorDate: Wed Apr 30 20:05:28 2025 +0900
Add proxy.config.http.negative_revalidating_list (#12212)
* Add proxy.config.http.negative_revalidating_list
* Address comments
(cherry picked from commit db850f1c5dc0f874af19134bacf5ceaa7697fec4)
---
doc/admin-guide/files/records.yaml.en.rst | 6 +
include/proxy/http/HttpConfig.h | 3 +
src/proxy/http/HttpConfig.cc | 90 +++++++++++--
src/proxy/http/HttpTransact.cc | 9 +-
src/records/RecordsConfig.cc | 2 +
.../gold_tests/cache/negative-revalidating.test.py | 77 +++++++----
.../replay/negative-revalidating-list.replay.yaml | 150 +++++++++++++++++++++
7 files changed, 294 insertions(+), 43 deletions(-)
diff --git a/doc/admin-guide/files/records.yaml.en.rst
b/doc/admin-guide/files/records.yaml.en.rst
index 3a544064e3..7596741621 100644
--- a/doc/admin-guide/files/records.yaml.en.rst
+++ b/doc/admin-guide/files/records.yaml.en.rst
@@ -1928,6 +1928,12 @@ Negative Response Caching
This configuration defaults to 1,800 seconds (30 minutes).
+.. ts:cv:: CONFIG proxy.config.http.negative_revalidating_list STRING 500 502
503 504
+ :reloadable:
+
+ The HTTP status codes for which the negative revalidating feature applies.
Note that this is a
+ `STRING` configuration containing a space separated list of the desired
HTTP status codes.
+
Proxy User Variables
====================
diff --git a/include/proxy/http/HttpConfig.h b/include/proxy/http/HttpConfig.h
index 9228635f38..63257470ac 100644
--- a/include/proxy/http/HttpConfig.h
+++ b/include/proxy/http/HttpConfig.h
@@ -802,6 +802,9 @@ public:
// bitset to hold the status codes that will BE cached with negative caching
enabled
HttpStatusBitset negative_caching_list;
+ // bitset to hold the status codes that will used by nagative revalidating
enabled
+ HttpStatusBitset negative_revalidating_list;
+
// All the overridable configurations goes into this class member, but they
// are not copied over until needed ("lazy").
OverridableHttpConfigParams oride;
diff --git a/src/proxy/http/HttpConfig.cc b/src/proxy/http/HttpConfig.cc
index 8989a32e7e..0d6e6e87b9 100644
--- a/src/proxy/http/HttpConfig.cc
+++ b/src/proxy/http/HttpConfig.cc
@@ -635,33 +635,51 @@ register_stat_callbacks()
});
}
+/**
+ Parse list of HTTP status code and return HttpStatusBitset
+ - e.g. "204 305 403 404 414 500 501 502 503 504"
+ */
+static HttpStatusBitset
+parse_http_status_code_list(swoc::TextView status_list)
+{
+ HttpStatusBitset set;
+
+ auto is_sep{[](char c) { return isspace(c) || ',' == c || ';' == c; }};
+
+ while (!status_list.ltrim_if(is_sep).empty()) {
+ swoc::TextView span;
+ swoc::TextView token{status_list.take_prefix_if(is_sep)};
+ auto n = swoc::svtoi(token, &span);
+ if (span.size() != token.size()) {
+ Error("Invalid status code '%.*s': not a number",
static_cast<int>(token.size()), token.data());
+ } else if (n <= 0 || n >= HTTP_STATUS_NUMBER) {
+ Error("Invalid status code '%.*s': out of range",
static_cast<int>(token.size()), token.data());
+ } else {
+ set[n] = true;
+ }
+ }
+
+ return set;
+}
+
static bool
set_negative_caching_list(const char *name, RecDataT dtype, RecData data,
HttpConfigParams *c, bool update)
{
bool ret = false;
HttpStatusBitset set;
+
// values from proxy.config.http.negative_caching_list
if (0 == strcasecmp("proxy.config.http.negative_caching_list", name) &&
RECD_STRING == dtype && data.rec_string) {
// parse the list of status codes
- swoc::TextView status_list(data.rec_string, strlen(data.rec_string));
- auto is_sep{[](char c) { return isspace(c) || ',' == c || ';' ==
c; }};
- while (!status_list.ltrim_if(is_sep).empty()) {
- swoc::TextView span, token{status_list.take_prefix_if(is_sep)};
- auto n = swoc::svtoi(token, &span);
- if (span.size() != token.size()) {
- Error("Invalid status code '%.*s' for negative caching: not a number",
static_cast<int>(token.size()), token.data());
- } else if (n <= 0 || n >= HTTP_STATUS_NUMBER) {
- Error("Invalid status code '%.*s' for negative caching: out of range",
static_cast<int>(token.size()), token.data());
- } else {
- set[n] = true;
- }
- }
+ set = parse_http_status_code_list({data.rec_string,
strlen(data.rec_string)});
}
+
// set the return value
if (set != c->negative_caching_list) {
c->negative_caching_list = set;
ret = ret || update;
}
+
return ret;
}
@@ -685,6 +703,47 @@ load_negative_caching_var(RecRecord const *r, void *cookie)
set_negative_caching_list(r->name, r->data_type, r->data, c, false);
}
+static bool
+set_negative_revalidating_list(const char *name, RecDataT dtype, RecData data,
HttpConfigParams *c, bool update)
+{
+ bool ret = false;
+ HttpStatusBitset set;
+
+ // values from proxy.config.http.negative_revalidating_list
+ if (0 == strcasecmp("proxy.config.http.negative_revalidating_list", name) &&
RECD_STRING == dtype && data.rec_string) {
+ // parse the list of status codes
+ set = parse_http_status_code_list({data.rec_string,
strlen(data.rec_string)});
+ }
+
+ // set the return value
+ if (set != c->negative_revalidating_list) {
+ c->negative_revalidating_list = set;
+ ret = ret || update;
+ }
+
+ return ret;
+}
+
+// Method of getting the status code bitset
+static int
+negative_revalidating_list_cb(const char *name, RecDataT dtype, RecData data,
void *cookie)
+{
+ HttpConfigParams *c = static_cast<HttpConfigParams *>(cookie);
+ // Signal an update if valid value arrived.
+ if (set_negative_revalidating_list(name, dtype, data, c, true)) {
+ http_config_cb(name, dtype, data, cookie);
+ }
+ return REC_ERR_OKAY;
+}
+
+// Method of loading the negative caching config bitset
+void
+load_negative_revalidating_var(RecRecord const *r, void *cookie)
+{
+ HttpConfigParams *c = static_cast<HttpConfigParams *>(cookie);
+ set_negative_revalidating_list(r->name, r->data_type, r->data, c, false);
+}
+
/** Template for creating conversions and initialization for @c std::chrono
based configuration variables.
*
* @tparam V The exact type of the configuration variable.
@@ -1020,6 +1079,8 @@ HttpConfig::startup()
HttpEstablishStaticConfigLongLong(c.oride.negative_revalidating_lifetime,
"proxy.config.http.negative_revalidating_lifetime");
RecRegisterConfigUpdateCb("proxy.config.http.negative_caching_list",
&negative_caching_list_cb, &c);
RecLookupRecord("proxy.config.http.negative_caching_list",
&load_negative_caching_var, &c, true);
+ RecRegisterConfigUpdateCb("proxy.config.http.negative_revalidating_list",
&negative_revalidating_list_cb, &c);
+ RecLookupRecord("proxy.config.http.negative_revalidating_list",
&load_negative_revalidating_var, &c, true);
// Buffer size and watermark
HttpEstablishStaticConfigLongLong(c.oride.default_buffer_size_index,
"proxy.config.http.default_buffer_size");
@@ -1339,7 +1400,8 @@ HttpConfig::reconfigure()
params->oride.ssl_client_sni_policy =
ats_strdup(m_master.oride.ssl_client_sni_policy);
params->oride.ssl_client_alpn_protocols =
ats_strdup(m_master.oride.ssl_client_alpn_protocols);
- params->negative_caching_list = m_master.negative_caching_list;
+ params->negative_caching_list = m_master.negative_caching_list;
+ params->negative_revalidating_list = m_master.negative_revalidating_list;
params->oride.host_res_data = m_master.oride.host_res_data;
params->oride.host_res_data.conf_value =
ats_strdup(m_master.oride.host_res_data.conf_value);
diff --git a/src/proxy/http/HttpTransact.cc b/src/proxy/http/HttpTransact.cc
index ff72cf5ac5..2a0bab4cb9 100644
--- a/src/proxy/http/HttpTransact.cc
+++ b/src/proxy/http/HttpTransact.cc
@@ -4319,15 +4319,12 @@
HttpTransact::handle_cache_operation_on_forward_server_response(State *s)
SET_VIA_STRING(VIA_SERVER_RESULT, VIA_SERVER_SERVED);
SET_VIA_STRING(VIA_PROXY_RESULT, VIA_PROXY_SERVED);
- /* if we receive a 500, 502, 503 or 504 while revalidating
+ /* By default, if we receive a 500, 502, 503 or 504 while revalidating
a document, treat the response as a 304 and in effect revalidate the
document for
negative_revalidating_lifetime. (negative revalidating)
*/
-
- if ((server_response_code == HTTP_STATUS_INTERNAL_SERVER_ERROR ||
server_response_code == HTTP_STATUS_GATEWAY_TIMEOUT ||
- server_response_code == HTTP_STATUS_BAD_GATEWAY ||
server_response_code == HTTP_STATUS_SERVICE_UNAVAILABLE) &&
- s->cache_info.action == CACHE_DO_UPDATE &&
s->txn_conf->negative_revalidating_enabled &&
- is_stale_cache_response_returnable(s)) {
+ if (s->txn_conf->negative_revalidating_enabled &&
s->http_config_param->negative_revalidating_list[server_response_code] &&
+ s->cache_info.action == CACHE_DO_UPDATE &&
is_stale_cache_response_returnable(s)) {
HTTPStatus cached_response_code =
s->cache_info.object_read->response_get()->status_get();
if (!(cached_response_code == HTTP_STATUS_INTERNAL_SERVER_ERROR ||
cached_response_code == HTTP_STATUS_GATEWAY_TIMEOUT ||
cached_response_code == HTTP_STATUS_BAD_GATEWAY ||
cached_response_code == HTTP_STATUS_SERVICE_UNAVAILABLE)) {
diff --git a/src/records/RecordsConfig.cc b/src/records/RecordsConfig.cc
index 73deb6e06c..c612e2d438 100644
--- a/src/records/RecordsConfig.cc
+++ b/src/records/RecordsConfig.cc
@@ -488,6 +488,8 @@ static const RecordElement RecordsConfig[] =
,
{RECT_CONFIG, "proxy.config.http.negative_caching_list", RECD_STRING, "204
305 403 404 414 500 501 502 503 504", RECU_DYNAMIC, RR_NULL, RECC_NULL,
nullptr, RECA_NULL}
,
+ {RECT_CONFIG, "proxy.config.http.negative_revalidating_list", RECD_STRING,
"500 502 503 504", RECU_DYNAMIC, RR_NULL, RECC_NULL, nullptr, RECA_NULL}
+ ,
// #########################
// # proxy users variables #
diff --git a/tests/gold_tests/cache/negative-revalidating.test.py
b/tests/gold_tests/cache/negative-revalidating.test.py
index 453814d9ef..e80c577827 100644
--- a/tests/gold_tests/cache/negative-revalidating.test.py
+++ b/tests/gold_tests/cache/negative-revalidating.test.py
@@ -21,47 +21,78 @@ Test.Summary = '''
Test the negative revalidating feature.
'''
+
+class NegativeRevalidatingTest:
+ _test_num: int = 0
+
+ def __init__(self, name: str, records_config: dict, replay_file: str):
+ self._tr = Test.AddTestRun(name)
+ self._replay_file = replay_file
+
+ self.__setupOriginServer()
+ self.__setupTS(records_config)
+ self.__setupClient()
+
+ NegativeRevalidatingTest._test_num += 1
+
+ def __setupClient(self):
+ self._tr.AddVerifierClientProcess(
+ f"client-{NegativeRevalidatingTest._test_num}", self._replay_file,
http_ports=[self._ts.Variables.port])
+
+ def __setupOriginServer(self):
+ self._server =
self._tr.AddVerifierServerProcess(f"server-{NegativeRevalidatingTest._test_num}",
self._replay_file)
+
+ def __setupTS(self, records_config):
+ self._ts =
Test.MakeATSProcess(f"ts-{NegativeRevalidatingTest._test_num}")
+ self._ts.Disk.records_config.update(records_config)
+ self._ts.Disk.remap_config.AddLine('map /
http://127.0.0.1:{0}'.format(self._server.Variables.http_port))
+
+ def run(self):
+ self._tr.Processes.Default.StartBefore(self._ts)
+ self._tr.StillRunningAfter = self._ts
+
+
#
# Verify disabled negative_revalidating behavior.
#
-ts = Test.MakeATSProcess("ts-negative-revalidating-disabled")
-ts.Disk.records_config.update(
- {
+NegativeRevalidatingTest(
+ "Verify negative revalidating disabled", {
'proxy.config.diags.debug.enabled': 1,
'proxy.config.diags.debug.tags': 'http|cache',
'proxy.config.http.insert_age_in_response': 0,
+ 'proxy.config.http.insert_response_via_str': 2,
'proxy.config.http.negative_revalidating_enabled': 0,
'proxy.config.http.cache.max_stale_age': 6
- })
-tr = Test.AddTestRun("Verify disabled negative revalidating behavior.")
-replay_file = "replay/negative-revalidating-disabled.replay.yaml"
-server = tr.AddVerifierServerProcess("server1", replay_file)
-server_port = server.Variables.http_port
-tr.AddVerifierClientProcess("client1", replay_file,
http_ports=[ts.Variables.port])
-ts.Disk.remap_config.AddLine('map / http://127.0.0.1:{0}'.format(server_port))
-tr.Processes.Default.StartBefore(ts)
-tr.StillRunningAfter = ts
+ }, "replay/negative-revalidating-disabled.replay.yaml").run()
#
# Verify enabled negative_revalidating behavior.
#
-ts = Test.MakeATSProcess("ts-negative-revalidating-enabled")
-ts.Disk.records_config.update(
+NegativeRevalidatingTest(
+ "Verify negative revalidating enabled",
{
'proxy.config.diags.debug.enabled': 1,
'proxy.config.diags.debug.tags': 'http|cache',
'proxy.config.http.insert_age_in_response': 0,
+ 'proxy.config.http.insert_response_via_str': 2,
# Negative revalidating is on by default. Verify this by leaving out
the
# following line and expect negative_revalidating to be enabled.
# 'proxy.config.http.negative_revalidating_enabled': 1,
'proxy.config.http.cache.max_stale_age': 6
- })
-tr = Test.AddTestRun("Verify negative revalidating behavior.")
-replay_file = "replay/negative-revalidating-enabled.replay.yaml"
-server = tr.AddVerifierServerProcess("server2", replay_file)
-server_port = server.Variables.http_port
-tr.AddVerifierClientProcess("client2", replay_file,
http_ports=[ts.Variables.port])
-ts.Disk.remap_config.AddLine('map / http://127.0.0.1:{0}'.format(server_port))
-tr.Processes.Default.StartBefore(ts)
-tr.StillRunningAfter = ts
+ },
+ "replay/negative-revalidating-enabled.replay.yaml").run()
+
+#
+# Verify negative_revalidating list behavior.
+#
+NegativeRevalidatingTest(
+ "Verify negative_revalidating_list behavior", {
+ 'proxy.config.diags.debug.enabled': 1,
+ 'proxy.config.diags.debug.tags': 'http|cache',
+ 'proxy.config.http.insert_age_in_response': 0,
+ 'proxy.config.http.insert_response_via_str': 2,
+ 'proxy.config.http.cache.max_stale_age': 6,
+ 'proxy.config.http.negative_revalidating_enabled': 1,
+ 'proxy.config.http.negative_revalidating_list': "403 404"
+ }, "replay/negative-revalidating-list.replay.yaml").run()
diff --git
a/tests/gold_tests/cache/replay/negative-revalidating-list.replay.yaml
b/tests/gold_tests/cache/replay/negative-revalidating-list.replay.yaml
new file mode 100644
index 0000000000..8b757b94d2
--- /dev/null
+++ b/tests/gold_tests/cache/replay/negative-revalidating-list.replay.yaml
@@ -0,0 +1,150 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+#
+# Verify negative_revalidating disabled behavior. This replay file assumes:
+# * ATS is configured with negative_revalidating disabled.
+# * max_stale_age is set to 6 seconds.
+#
+
+meta:
+ version: "1.0"
+
+sessions:
+- transactions:
+
+ #
+ # Test 1: Negative revalidating for a cached content-length response.
+ #
+ - client-request:
+ method: "GET"
+ version: "1.1"
+ url: /path/reques_item
+ headers:
+ fields:
+ - [ Host, example.com ]
+ - [ uuid, 31 ]
+
+ # Populate the cache with a 200 response.
+ server-response:
+ status: 200
+ reason: "OK"
+ headers:
+ fields:
+ - [ Content-Length, 32 ]
+ - [ Cache-Control, max-age=2 ]
+
+ proxy-response:
+ status: 200
+
+ # Verify we serve the 200 OK out of the cache if it is not stale.
+ - client-request:
+ method: "GET"
+ version: "1.1"
+ url: /path/reques_item
+ headers:
+ fields:
+ - [ Host, example.com ]
+ - [ uuid, 32 ]
+
+ # Give cache IO enough time to finish.
+ delay: 100ms
+
+ # This should not reach the origin server.
+ server-response:
+ status: 503
+ reason: "Service Unavailable"
+ headers:
+ fields:
+ - [ Content-Length, 32 ]
+
+ # Again, we should serve this out of the cache.
+ proxy-response:
+ status: 200
+
+ # Verify that with negative_revalidating enabled, we serve the 200 OK out of
+ # the cache even though it is stale (but younger than max_stale_age) for
configured status code.
+ - client-request:
+ method: "GET"
+ version: "1.1"
+ url: /path/reques_item
+ headers:
+ fields:
+ - [ Host, example.com ]
+ - [ uuid, 33 ]
+
+ # Make sure the item is stale per it's 2 second max-age.
+ delay: 4s
+
+ server-response:
+ status: 403
+ reason: "Forbidden"
+ headers:
+ fields:
+ - [ Content-Length, 32 ]
+
+ # With negative_revalidating enabled, the cached response should be served
+ # even though it is stale.
+ proxy-response:
+ status: 200
+
+ # Verify that with negative_revalidating enabled, we serve the response from
origin if it's not configured status code.
+ - client-request:
+ method: "GET"
+ version: "1.1"
+ url: /path/reques_item
+ headers:
+ fields:
+ - [ Host, example.com ]
+ - [ uuid, 34 ]
+
+ server-response:
+ status: 503
+ reason: "Service Unavailable"
+ headers:
+ fields:
+ - [ Content-Length, 32 ]
+
+ # With negative_revalidating enabled, but status code is not configured
one, return it directly
+ proxy-response:
+ status: 503
+
+ # Verify that max_stale_age is respected.
+ - client-request:
+ method: "GET"
+ version: "1.1"
+ url: /path/reques_item
+ headers:
+ fields:
+ - [ Host, example.com ]
+ - [ uuid, 35 ]
+
+ # After this delay, the item is 8 seconds old. This makes it:
+ # 6 seconds beyond the server's max-age of 2 seconds and
+ # 2 seconds beyond ATS's max_stale_age of 6 seconds.
+ delay: 4s
+
+ server-response:
+ status: 503
+ reason: "Service Unavailable"
+ headers:
+ fields:
+ - [ Content-Length, 32 ]
+
+ # negative_revalidating is enabled, but now the cached item is older than
+ # max_stale_age.
+ proxy-response:
+ status: 503
