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 2018/08/31 19:11:33 UTC

[trafficserver] branch 8.0.x updated (aadaef5 -> 7da4d94)

This is an automated email from the ASF dual-hosted git repository.

bcall pushed a change to branch 8.0.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git.


    from aadaef5  Enables proxy.config.http.negative_revalidating_enabled by default
     new 0a54f31  Make negative caching accept configured error status codes
     new 7da4d94  Updated docs to reflect default configuration change with proxy.config.http.negative_revalidating_enabled

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 doc/admin-guide/files/records.config.en.rst | 11 ++++--
 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, 82 insertions(+), 19 deletions(-)


[trafficserver] 02/02: Updated docs to reflect default configuration change with proxy.config.http.negative_revalidating_enabled

Posted by bc...@apache.org.
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 7da4d94e1a05f5123899f8e7da5de7903c8aec6f
Author: Bryan Call <bc...@apache.org>
AuthorDate: Fri Aug 31 10:23:52 2018 -0700

    Updated docs to reflect default configuration change with
    proxy.config.http.negative_revalidating_enabled
    
    (cherry picked from commit 08152a732959276f2adcac2e6ea84a6c55839d40)
---
 doc/admin-guide/files/records.config.en.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/admin-guide/files/records.config.en.rst b/doc/admin-guide/files/records.config.en.rst
index f8d51a5..ef7dd5f 100644
--- a/doc/admin-guide/files/records.config.en.rst
+++ b/doc/admin-guide/files/records.config.en.rst
@@ -1503,7 +1503,7 @@ Negative Response Caching
    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
+.. ts:cv:: CONFIG proxy.config.http.negative_revalidating_enabled INT 1
    :reloadable:
    :overridable:
 


[trafficserver] 01/02: Make negative caching accept configured error status codes

Posted by bc...@apache.org.
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 0a54f31dfc718810836baa71edb4e7f9a9041bd0
Author: Xavier Chi <ch...@gmail.com>
AuthorDate: Mon Jul 16 15:51:23 2018 -0500

    Make negative caching accept configured error status codes
    
    (cherry picked from commit 4c34cbc4d0ad432b56b2f61f18f63a2d41cc94b8)
    
     Conflicts:
    	proxy/http/HttpConfig.h
---
 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 a2f0ee3..f8d51a5 100644
--- a/doc/admin-guide/files/records.config.en.rst
+++ b/doc/admin-guide/files/records.config.en.rst
@@ -1468,7 +1468,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
@@ -1478,6 +1478,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
@@ -1496,6 +1497,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 86c3b6e..18f05dc 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 0f52151..344acec 100644
--- a/lib/perl/lib/Apache/TS/AdminClient.pm
+++ b/lib/perl/lib/Apache/TS/AdminClient.pm
@@ -475,6 +475,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 8ea0000..ac4cd3b 100644
--- a/mgmt/RecordsConfig.cc
+++ b/mgmt/RecordsConfig.cc
@@ -499,6 +499,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 7aa54e8..8e40cbc 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<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] = 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<HttpConfigParams *>(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<HttpConfigParams *>(cookie);
+  set_negative_caching_list(r->name, r->data_type, r->data, c, false);
+}
+
 ////////////////////////////////////////////////////////////////
 //
 //  HttpConfig::startup()
@@ -1146,6 +1196,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");
@@ -1434,6 +1486,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 3518dce..3057f58 100644
--- a/proxy/http/HttpConfig.h
+++ b/proxy/http/HttpConfig.h
@@ -50,6 +50,9 @@
 #include "ProxyConfig.h"
 #include "P_RecProcess.h"
 
+static const unsigned HTTP_STATUS_NUMBER = 600;
+using HttpStatusBitset                   = std::bitset<HTTP_STATUS_NUMBER>;
+
 /* Instead of enumerating the stats in DynamicStats.h, each module needs
    to enumerate its stats separately and register them with librecords
    */
@@ -857,6 +860,9 @@ public:
 
   MgmtByte server_session_sharing_pool = TS_SERVER_SESSION_SHARING_POOL_THREAD;
 
+  // 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 83a98b1..c8bb547 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