From 35f25ce7d9fe02b0d61d8ff9833a8b702027ca98 Mon Sep 17 00:00:00 2001 From: Xavier Chi Date: Mon, 16 Jul 2018 15:51:23 -0500 Subject: [PATCH] Make negative caching accept configured error status codes --- doc/admin-guide/files/records.config.en.rst | 9 +++- doc/admin-guide/performance/index.en.rst | 4 +- lib/perl/lib/Apache/TS/AdminClient.pm | 1 + mgmt/RecordsConfig.cc | 2 + proxy/http/HttpConfig.cc | 54 +++++++++++++++++++++ proxy/http/HttpConfig.h | 6 +++ proxy/http/HttpTransact.cc | 23 +++------ 7 files changed, 81 insertions(+), 18 deletions(-) diff --git a/doc/admin-guide/files/records.config.en.rst b/doc/admin-guide/files/records.config.en.rst index a474b6fa5c0..597fc946270 100644 --- a/doc/admin-guide/files/records.config.en.rst +++ b/doc/admin-guide/files/records.config.en.rst @@ -1520,7 +1520,7 @@ Negative Response Caching When disabled (``0``), |TS| will only cache the response if the response has ``Cache-Control`` headers. - The following negative responses are cached by Traffic Server: + The following negative responses are cached by Traffic Server by default: ====================== ===================================================== HTTP Response Code Description @@ -1530,6 +1530,7 @@ Negative Response Caching ``400`` Bad Request ``403`` Forbidden ``404`` Not Found + ``414`` URI Too Long ``405`` Method Not Allowed ``500`` Internal Server Error ``501`` Not Implemented @@ -1548,6 +1549,12 @@ Negative Response Caching How long (in seconds) Traffic Server keeps the negative responses valid in cache. This value only affects negative responses that do NOT have explicit ``Expires:`` or ``Cache-Control:`` lifetimes set by the server. +.. ts:cv:: CONFIG proxy.config.http.negative_caching_list STRING 204 305 403 404 405 414 500 501 502 503 504 + :reloadable: + + The HTTP status code for negative caching. Default values are mentioned above. The unwanted status codes can be + taken out from the list. Other status codes can be added. The variable is a list but parsed as STRING. + .. ts:cv:: CONFIG proxy.config.http.negative_revalidating_enabled INT 0 :reloadable: :overridable: diff --git a/doc/admin-guide/performance/index.en.rst b/doc/admin-guide/performance/index.en.rst index 86c3b6e7707..18f05dc6fc1 100644 --- a/doc/admin-guide/performance/index.en.rst +++ b/doc/admin-guide/performance/index.en.rst @@ -497,10 +497,12 @@ client request to result in an origin request. This behavior is controlled by both enabling the feature via :ts:cv:`proxy.config.http.negative_caching_enabled` and setting the cache time -(in seconds) with :ts:cv:`proxy.config.http.negative_caching_lifetime`. :: +(in seconds) with :ts:cv:`proxy.config.http.negative_caching_lifetime`. Configured +status code for negative caching can be set with :ts:cv:`proxy.config.http.negative_caching_list`. :: CONFIG proxy.config.http.negative_caching_enabled INT 1 CONFIG proxy.config.http.negative_caching_lifetime INT 10 + CONFIG proxy.config.http.negative_caching_list STRING 204 305 403 404 405 414 500 501 502 503 504 SSL-Specific Options ~~~~~~~~~~~~~~~~~~~~ diff --git a/lib/perl/lib/Apache/TS/AdminClient.pm b/lib/perl/lib/Apache/TS/AdminClient.pm index e83830aa5ae..efd6b4b559e 100644 --- a/lib/perl/lib/Apache/TS/AdminClient.pm +++ b/lib/perl/lib/Apache/TS/AdminClient.pm @@ -474,6 +474,7 @@ The Apache Traffic Server Administration Manual will explain what these strings proxy.config.http.keep_alive_no_activity_timeout_out proxy.config.http.keep_alive_post_out proxy.config.http.negative_caching_enabled + proxy.config.http.negative_caching_list proxy.config.http.negative_caching_lifetime proxy.config.http.negative_revalidating_enabled proxy.config.http.negative_revalidating_lifetime diff --git a/mgmt/RecordsConfig.cc b/mgmt/RecordsConfig.cc index 0ba68b07f7e..3d6dc14dfba 100644 --- a/mgmt/RecordsConfig.cc +++ b/mgmt/RecordsConfig.cc @@ -502,6 +502,8 @@ static const RecordElement RecordsConfig[] = , {RECT_CONFIG, "proxy.config.http.negative_caching_lifetime", RECD_INT, "1800", RECU_DYNAMIC, RR_NULL, RECC_NULL, nullptr, RECA_NULL} , + {RECT_CONFIG, "proxy.config.http.negative_caching_list", RECD_STRING, "204 305 403 404 405 414 500 501 502 503 504", RECU_DYNAMIC, RR_NULL, RECC_NULL, nullptr, RECA_NULL} + , // ######################### // # proxy users variables # diff --git a/proxy/http/HttpConfig.cc b/proxy/http/HttpConfig.cc index 1e742c16550..0e4e13ab132 100644 --- a/proxy/http/HttpConfig.cc +++ b/proxy/http/HttpConfig.cc @@ -900,6 +900,56 @@ register_stat_callbacks() (int)http_sm_finish_time_stat, RecRawStatSyncSum); } +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 code + ts::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()) { + ts::TextView span, token{status_list.take_prefix_if(is_sep)}; + auto n = ts::svtoi(token, &span); + if (span.size() != token.size()) { + Error("Invalid status code '%.*s' for negative caching: not a number", static_cast(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(token.size()), token.data()); + } else { + set[n] = 1; + } + } + } + // set the return value + if (set != c->negative_caching_list) { + c->negative_caching_list = set; + ret = ret || update; + } + return ret; +} + +// Method of getting the status code bitset +static int +negative_caching_list_cb(const char *name, RecDataT dtype, RecData data, void *cookie) +{ + HttpConfigParams *c = static_cast(cookie); + // Signal an update if valid value arrived. + if (set_negative_caching_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_caching_var(RecRecord const *r, void *cookie) +{ + HttpConfigParams *c = static_cast(cookie); + set_negative_caching_list(r->name, r->data_type, r->data, c, false); +} + //////////////////////////////////////////////////////////////// // // HttpConfig::startup() @@ -1144,6 +1194,8 @@ HttpConfig::startup() HttpEstablishStaticConfigLongLong(c.oride.negative_caching_lifetime, "proxy.config.http.negative_caching_lifetime"); HttpEstablishStaticConfigByte(c.oride.negative_revalidating_enabled, "proxy.config.http.negative_revalidating_enabled"); 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); // Buffer size and watermark HttpEstablishStaticConfigLongLong(c.oride.default_buffer_size_index, "proxy.config.http.default_buffer_size"); @@ -1439,6 +1491,8 @@ HttpConfig::reconfigure() params->oride.client_cert_filename = ats_strdup(m_master.oride.client_cert_filename); params->oride.client_cert_filepath = ats_strdup(m_master.oride.client_cert_filepath); + params->negative_caching_list = m_master.negative_caching_list; + m_id = configProcessor.set(m_id, params); #undef INT_TO_BOOL diff --git a/proxy/http/HttpConfig.h b/proxy/http/HttpConfig.h index d28969948f2..eb97757a069 100644 --- a/proxy/http/HttpConfig.h +++ b/proxy/http/HttpConfig.h @@ -51,6 +51,9 @@ #include "P_RecProcess.h" #include "HttpConnectionCount.h" +static const unsigned HTTP_STATUS_NUMBER = 600; +using HttpStatusBitset = std::bitset; + /* Instead of enumerating the stats in DynamicStats.h, each module needs to enumerate its stats separately and register them with librecords */ @@ -858,6 +861,9 @@ struct HttpConfigParams : public ConfigInfo { OutboundConnTrack::GlobalConfig outbound_conntrack; + // bitset to hold the status codes that will BE cached with negative caching enabled + HttpStatusBitset negative_caching_list; + // All the overridable configurations goes into this class member, but they // are not copied over until needed ("lazy"). OverridableHttpConfigParams oride; diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc index 24171cb171b..1bffffd5b01 100644 --- a/proxy/http/HttpTransact.cc +++ b/proxy/http/HttpTransact.cc @@ -239,24 +239,15 @@ is_negative_caching_appropriate(HttpTransact::State *s) return false; } - switch (s->hdr_info.server_response.status_get()) { - case HTTP_STATUS_NO_CONTENT: - case HTTP_STATUS_USE_PROXY: - case HTTP_STATUS_FORBIDDEN: - case HTTP_STATUS_NOT_FOUND: - case HTTP_STATUS_METHOD_NOT_ALLOWED: - case HTTP_STATUS_REQUEST_URI_TOO_LONG: - case HTTP_STATUS_INTERNAL_SERVER_ERROR: - case HTTP_STATUS_NOT_IMPLEMENTED: - case HTTP_STATUS_BAD_GATEWAY: - case HTTP_STATUS_SERVICE_UNAVAILABLE: - case HTTP_STATUS_GATEWAY_TIMEOUT: + int status = s->hdr_info.server_response.status_get(); + auto params = s->http_config_param; + if (params->negative_caching_list[status]) { + TxnDebug("http_trans", "%d is eligible for negative caching", status); return true; - default: - break; + } else { + TxnDebug("http_trans", "%d is NOT eligible for negative caching", status); + return false; } - - return false; } inline static HttpTransact::LookingUp_t