You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by zw...@apache.org on 2022/02/02 23:38:56 UTC

[trafficserver] branch 9.2.x updated: Revert fixes for #8539 (#8637)

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

zwoop pushed a commit to branch 9.2.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git


The following commit(s) were added to refs/heads/9.2.x by this push:
     new 116890d  Revert fixes for #8539 (#8637)
116890d is described below

commit 116890db8056883ecd0b4d5713a5976b74057341
Author: Brian Neradt <br...@gmail.com>
AuthorDate: Mon Jan 31 14:56:27 2022 -0600

    Revert fixes for #8539 (#8637)
    
    Fixing #8539 is easy to do in a common case, but it causing issues in
    corner cases. For now I'm going to revert the fixes for this and circle
    back around later to see whether this can be solved in a different way.
    
    This reverts the following commits:
    
    Revert "Commenting TSHttpTxnCacheLookupStatusGet need_to_revalidate (#8621)"
    This reverts commit 4678ae34321002c584bd40ff6a1e4a128dae8bb1
    
    Revert "TSHttpTxnCacheLookupStatusGet: call need_to_revalidate (#8617)"
    This reverts commit 235c44a11f70d72168915af2d6504d0e63e880b3.
    
    Revert "TSHttpTxnCacheLookupStatusGet: handle cannot respond cases (#8545)"
    This reverts commit 080e236463490367ef94ce4e56fc95f287a917f5.
    
    (cherry picked from commit 2f1bd0fe3f7f5677d15d7add87c7a55ce1365db1)
---
 src/traffic_server/InkAPI.cc                       | 15 +---
 tests/Makefile.am                                  |  1 -
 .../gold_tests/cache/cache-request-method.test.py  | 30 +-------
 .../print_cache_status_cache_post_disabled.gold    |  7 --
 .../print_cache_status_cache_post_enabled.gold     | 10 ---
 tests/gold_tests/cache/plugins/Makefile.inc        | 18 -----
 .../gold_tests/cache/plugins/print_cache_status.cc | 89 ----------------------
 7 files changed, 3 insertions(+), 167 deletions(-)

diff --git a/src/traffic_server/InkAPI.cc b/src/traffic_server/InkAPI.cc
index edb8b55..da3331e 100644
--- a/src/traffic_server/InkAPI.cc
+++ b/src/traffic_server/InkAPI.cc
@@ -5448,20 +5448,7 @@ TSHttpTxnCacheLookupStatusGet(TSHttpTxn txnp, int *lookup_status)
     break;
   case HttpTransact::CACHE_LOOKUP_HIT_WARNING:
   case HttpTransact::CACHE_LOOKUP_HIT_FRESH:
-    if (HttpTransact::need_to_revalidate(&sm->t_state)) {
-      // Note that in this case the object was determined to be fresh but there
-      // was a problem with the cached object for this request per
-      // need_to_revalidate(). need_to_revalidate() checks for issues such as
-      // the need to authenticate with the origin or the incoming request
-      // method doesn't equal the method of the request for the cached
-      // response. Issues like this are more accurately described as a MISS
-      // rather than STALE because they are not due to freshness (timing)
-      // issues but rather to characteristics that make the cached object
-      // inappropriate as a response to this request.
-      *lookup_status = TS_CACHE_LOOKUP_MISS;
-    } else {
-      *lookup_status = TS_CACHE_LOOKUP_HIT_FRESH;
-    }
+    *lookup_status = TS_CACHE_LOOKUP_HIT_FRESH;
     break;
   case HttpTransact::CACHE_LOOKUP_SKIPPED:
     *lookup_status = TS_CACHE_LOOKUP_SKIPPED;
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 81e7a15..3910788 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -31,7 +31,6 @@ AM_LDFLAGS += $(TS_PLUGIN_LD_FLAGS)
 AM_LDFLAGS += -rpath $(abs_builddir)
 
 include gold_tests/bigobj/Makefile.inc
-include gold_tests/cache/plugins/Makefile.inc
 include gold_tests/continuations/plugins/Makefile.inc
 include gold_tests/chunked_encoding/Makefile.inc
 include gold_tests/pluginTest/tsapi/Makefile.inc
diff --git a/tests/gold_tests/cache/cache-request-method.test.py b/tests/gold_tests/cache/cache-request-method.test.py
index 2db75f8..5715e81 100644
--- a/tests/gold_tests/cache/cache-request-method.test.py
+++ b/tests/gold_tests/cache/cache-request-method.test.py
@@ -17,8 +17,6 @@ Verify correct caching behavior with respect to request method.
 #  See the License for the specific language governing permissions and
 #  limitations under the License.
 
-import os
-
 Test.Summary = '''
 Verify correct caching behavior with respect to request method.
 '''
@@ -26,15 +24,11 @@ Verify correct caching behavior with respect to request method.
 # Test 0: Verify correct POST response handling when caching POST responses is
 # disabled.
 ts = Test.MakeATSProcess("ts")
-Test.PrepareTestPlugin(os.path.join(Test.Variables.AtsBuildGoldTestsDir,
-                                    'cache', 'plugins', '.libs', 'print_cache_status.so'), ts)
-Test.Disk.File(os.path.join(ts.Variables.LOGDIR, 'print_cache_status.log'),
-               exists=True, content='gold/print_cache_status_cache_post_disabled.gold')
 replay_file = "replay/post_with_post_caching_disabled.replay.yaml"
 server = Test.MakeVerifierServerProcess("server0", replay_file)
 ts.Disk.records_config.update({
     'proxy.config.diags.debug.enabled': 1,
-    'proxy.config.diags.debug.tags': 'http|cache|print_cache_status',
+    'proxy.config.diags.debug.tags': 'http.*|cache.*',
     'proxy.config.http.insert_age_in_response': 0,
 
     # Caching of POST responses is disabled by default. Verify default behavior
@@ -49,26 +43,14 @@ tr.Processes.Default.StartBefore(server)
 tr.Processes.Default.StartBefore(ts)
 tr.AddVerifierClientProcess("client0", replay_file, http_ports=[ts.Variables.port])
 
-# Wait for log file to appear, then wait one extra second to make sure TS is done writing it.
-test_run = Test.AddTestRun()
-test_run.Processes.Default.Command = (
-    os.path.join(Test.Variables.AtsTestToolsDir, 'condwait') + ' 60 1 -f ' +
-    os.path.join(ts.Variables.LOGDIR, 'print_cache_status.log')
-)
-test_run.Processes.Default.ReturnCode = 0
-
 # Test 1: Verify correct POST response handling when caching POST responses is
 # enabled.
 ts = Test.MakeATSProcess("ts-cache-post")
-Test.PrepareTestPlugin(os.path.join(Test.Variables.AtsBuildGoldTestsDir,
-                                    'cache', 'plugins', '.libs', 'print_cache_status.so'), ts)
-Test.Disk.File(os.path.join(ts.Variables.LOGDIR, 'print_cache_status.log'),
-               exists=True, content='gold/print_cache_status_cache_post_enabled.gold')
 replay_file = "replay/post_with_post_caching_enabled.replay.yaml"
 server = Test.MakeVerifierServerProcess("server1", replay_file)
 ts.Disk.records_config.update({
     'proxy.config.diags.debug.enabled': 1,
-    'proxy.config.diags.debug.tags': 'http|cache|print_cache_status',
+    'proxy.config.diags.debug.tags': 'http.*|cache.*',
     'proxy.config.http.insert_age_in_response': 0,
     'proxy.config.http.cache.post_method': 1,
 })
@@ -79,11 +61,3 @@ tr = Test.AddTestRun("Verify correct with POST response caching enabled.")
 tr.Processes.Default.StartBefore(server)
 tr.Processes.Default.StartBefore(ts)
 tr.AddVerifierClientProcess("client1", replay_file, http_ports=[ts.Variables.port])
-
-# Wait for log file to appear, then wait one extra second to make sure TS is done writing it.
-test_run = Test.AddTestRun()
-test_run.Processes.Default.Command = (
-    os.path.join(Test.Variables.AtsTestToolsDir, 'condwait') + ' 60 1 -f ' +
-    os.path.join(ts.Variables.LOGDIR, 'print_cache_status.log')
-)
-test_run.Processes.Default.ReturnCode = 0
diff --git a/tests/gold_tests/cache/gold/print_cache_status_cache_post_disabled.gold b/tests/gold_tests/cache/gold/print_cache_status_cache_post_disabled.gold
deleted file mode 100644
index 90e2e8d..0000000
--- a/tests/gold_tests/cache/gold/print_cache_status_cache_post_disabled.gold
+++ /dev/null
@@ -1,7 +0,0 @@
-`` Cache lookup status: TS_CACHE_LOOKUP_MISS
-`` Cache lookup status: TS_CACHE_LOOKUP_HIT_FRESH
-`` Cache lookup status: TS_CACHE_LOOKUP_MISS
-`` Cache lookup status: TS_CACHE_LOOKUP_MISS
-`` Cache lookup status: TS_CACHE_LOOKUP_HIT_FRESH
-`` Cache lookup status: TS_CACHE_LOOKUP_MISS
-`` Cache lookup status: TS_CACHE_LOOKUP_HIT_FRESH
diff --git a/tests/gold_tests/cache/gold/print_cache_status_cache_post_enabled.gold b/tests/gold_tests/cache/gold/print_cache_status_cache_post_enabled.gold
deleted file mode 100644
index f795b29..0000000
--- a/tests/gold_tests/cache/gold/print_cache_status_cache_post_enabled.gold
+++ /dev/null
@@ -1,10 +0,0 @@
-`` Cache lookup status: TS_CACHE_LOOKUP_MISS
-`` Cache lookup status: TS_CACHE_LOOKUP_HIT_FRESH
-`` Cache lookup status: TS_CACHE_LOOKUP_MISS
-`` Cache lookup status: TS_CACHE_LOOKUP_HIT_FRESH
-`` Cache lookup status: TS_CACHE_LOOKUP_MISS
-`` Cache lookup status: TS_CACHE_LOOKUP_HIT_FRESH
-`` Cache lookup status: TS_CACHE_LOOKUP_MISS
-`` Cache lookup status: TS_CACHE_LOOKUP_MISS
-`` Cache lookup status: TS_CACHE_LOOKUP_MISS
-`` Cache lookup status: TS_CACHE_LOOKUP_HIT_FRESH
diff --git a/tests/gold_tests/cache/plugins/Makefile.inc b/tests/gold_tests/cache/plugins/Makefile.inc
deleted file mode 100644
index 1b0b7ba..0000000
--- a/tests/gold_tests/cache/plugins/Makefile.inc
+++ /dev/null
@@ -1,18 +0,0 @@
-#  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.
-
-noinst_LTLIBRARIES += gold_tests/cache/plugins/print_cache_status.la
-gold_tests_cache_plugins_print_cache_status_la_SOURCES = gold_tests/cache/plugins/print_cache_status.cc
diff --git a/tests/gold_tests/cache/plugins/print_cache_status.cc b/tests/gold_tests/cache/plugins/print_cache_status.cc
deleted file mode 100644
index 531b6aa..0000000
--- a/tests/gold_tests/cache/plugins/print_cache_status.cc
+++ /dev/null
@@ -1,89 +0,0 @@
-/**
-  @file
-  @brief A plugin that prints the cache lookup status.
-
-  @section license License
-
-  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.
-*/
-#include <ts/ts.h>   // for debug
-#include <cstdlib>   // for abort
-#include <cinttypes> // for PRId64
-#include <string_view>
-#include <unordered_map>
-
-namespace
-{
-constexpr char const *PLUGIN_NAME = "print_cache_status";
-static TSTextLogObject pluginlog;
-
-std::unordered_map<int, std::string_view> lookup_status_to_string = {
-  {TS_CACHE_LOOKUP_MISS, "TS_CACHE_LOOKUP_MISS"},
-  {TS_CACHE_LOOKUP_HIT_STALE, "TS_CACHE_LOOKUP_HIT_STALE"},
-  {TS_CACHE_LOOKUP_HIT_FRESH, "TS_CACHE_LOOKUP_HIT_FRESH"},
-  {TS_CACHE_LOOKUP_SKIPPED, "TS_CACHE_LOOKUP_SKIPPED"},
-};
-
-int
-global_handler(TSCont continuation, TSEvent event, void *data)
-{
-  TSHttpTxn txnp = static_cast<TSHttpTxn>(data);
-
-  switch (event) {
-  case TS_EVENT_HTTP_CACHE_LOOKUP_COMPLETE: {
-    int obj_status = 0;
-    if (TS_ERROR == TSHttpTxnCacheLookupStatusGet(txnp, &obj_status)) {
-      TSError("[%s] TSHttpTxnCacheLookupStatusGet failed", PLUGIN_NAME);
-    }
-    TSTextLogObjectWrite(pluginlog, "Cache lookup status: %s", lookup_status_to_string[obj_status].data());
-  } break;
-
-  default:
-    TSError("[%s] Unexpected event: %d", PLUGIN_NAME, event);
-    return 0;
-  }
-
-  TSHttpTxnReenable(txnp, TS_EVENT_HTTP_CONTINUE);
-
-  return 0;
-}
-} // anonymous namespace
-
-void
-TSPluginInit(int argc, const char *argv[])
-{
-  TSDebug(PLUGIN_NAME, "initializing plugin");
-
-  TSPluginRegistrationInfo info;
-
-  info.plugin_name   = PLUGIN_NAME;
-  info.vendor_name   = "Apache";
-  info.support_email = "bneradt@apache.org";
-
-  if (TSPluginRegister(&info) != TS_SUCCESS) {
-    TSError("[%s] Plugin registration failed.", PLUGIN_NAME);
-  }
-  TSAssert(TS_SUCCESS == TSTextLogObjectCreate(PLUGIN_NAME, TS_LOG_MODE_ADD_TIMESTAMP, &pluginlog));
-
-  TSCont contp = TSContCreate(global_handler, TSMutexCreate());
-  if (contp == nullptr) {
-    TSError("[%s] could not create continuation.", PLUGIN_NAME);
-    std::abort();
-  } else {
-    TSHttpHookAdd(TS_HTTP_CACHE_LOOKUP_COMPLETE_HOOK, contp);
-  }
-}