trafficserver-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From zw...@apache.org
Subject [trafficserver] branch 8.0.x updated: This removes the second CacheSM, and related APIs
Date Tue, 19 Jun 2018 16:48:06 GMT
This is an automated email from the ASF dual-hosted git repository.

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


The following commit(s) were added to refs/heads/8.0.x by this push:
     new 2cbb1a6  This removes the second CacheSM, and related APIs
2cbb1a6 is described below

commit 2cbb1a6b2a1322dcc07c39d2e7c7bbcfa74ff5cb
Author: Leif Hedstrom <zwoop@apache.org>
AuthorDate: Wed Jun 13 09:53:47 2018 -0600

    This removes the second CacheSM, and related APIs
    
    For some details as to why this is important, see:
    
        https://cwiki.apache.org/confluence/display/TS/Cleanup+of+Cache+URLs
    
    Note that this leaves the stale_while_revalidate plugin in a broken state,
    but I rather keep the code there for now, since there's hope someone has a
    fix for this plugin, which does not require this now removed API.
    
    (cherry picked from commit eebc3811851b561fa2b3126b587d9c439afefab9)
---
 .../stale_while_revalidate.c                       |  3 +-
 proxy/api/ts/experimental.h                        |  4 --
 proxy/http/HttpSM.cc                               | 53 ++++-----------
 proxy/http/HttpSM.h                                | 18 ------
 proxy/http/HttpTransact.h                          | 12 +---
 src/traffic_server/InkAPI.cc                       | 75 ----------------------
 6 files changed, 14 insertions(+), 151 deletions(-)

diff --git a/plugins/experimental/stale_while_revalidate/stale_while_revalidate.c b/plugins/experimental/stale_while_revalidate/stale_while_revalidate.c
index 150275a..be137bb 100644
--- a/plugins/experimental/stale_while_revalidate/stale_while_revalidate.c
+++ b/plugins/experimental/stale_while_revalidate/stale_while_revalidate.c
@@ -378,7 +378,8 @@ consume_resource(TSCont cont, TSEvent event ATS_UNUSED, void *edata ATS_UNUSED)
       } else {
         TSDebug(PLUGIN_NAME, "Attempting new cache lookup");
         if (TSHttpHdrUrlGet(state->req_info->buf, state->req_info->http_hdr_loc,
&url_loc) == TS_SUCCESS) {
-          TSHttpTxnNewCacheLookupDo(state->txn, state->req_info->buf, url_loc);
+          // ToDo: This is now completely broken, and we need a different logic here than
the second cache lookup
+          // TSHttpTxnNewCacheLookupDo(state->txn, state->req_info->buf, url_loc);
           TSHandleMLocRelease(state->req_info->buf, state->req_info->http_hdr_loc,
url_loc);
         } else {
           TSError("[%s] Error getting the URL from the transaction", PLUGIN_NAME);
diff --git a/proxy/api/ts/experimental.h b/proxy/api/ts/experimental.h
index 9d1895e..b6cfd35 100644
--- a/proxy/api/ts/experimental.h
+++ b/proxy/api/ts/experimental.h
@@ -200,10 +200,6 @@ tsapi TSReturnCode TSHttpTxnServerRespIgnore(TSHttpTxn txnp);
 tsapi TSReturnCode TSHttpTxnShutDown(TSHttpTxn txnp, TSEvent event);
 tsapi TSReturnCode TSHttpTxnCloseAfterResponse(TSHttpTxn txnp, int should_close);
 
-/* TS-1996: These API swill be removed after v3.4.0 is cut. Do not use them! */
-tsapi TSReturnCode TSHttpTxnNewCacheLookupDo(TSHttpTxn txnp, TSMBuffer bufp, TSMLoc url_loc);
-tsapi TSReturnCode TSHttpTxnSecondUrlTryLock(TSHttpTxn txnp);
-
 /****************************************************************************
  *  ??
  *  Return ??
diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc
index 292a335..6e4167b 100644
--- a/proxy/http/HttpSM.cc
+++ b/proxy/http/HttpSM.cc
@@ -276,10 +276,6 @@ HttpSM::cleanup()
   tunnel.mutex.clear();
   cache_sm.mutex.clear();
   transform_cache_sm.mutex.clear();
-  if (second_cache_sm) {
-    second_cache_sm->mutex.clear();
-    delete second_cache_sm;
-  }
   magic    = HTTP_SM_MAGIC_DEAD;
   debug_on = false;
 }
@@ -2436,19 +2432,6 @@ HttpSM::state_cache_open_write(int event, void *data)
     ink_release_assert(0);
   }
 
-  if (t_state.api_lock_url != HttpTransact::LOCK_URL_FIRST) {
-    if (event == CACHE_EVENT_OPEN_WRITE || event == CACHE_EVENT_OPEN_WRITE_FAILED) {
-      if (t_state.api_lock_url == HttpTransact::LOCK_URL_SECOND) {
-        t_state.api_lock_url = HttpTransact::LOCK_URL_ORIGINAL;
-        do_cache_prepare_action(second_cache_sm, t_state.cache_info.second_object_read, true);
-        return 0;
-      } else {
-        t_state.api_lock_url = HttpTransact::LOCK_URL_DONE;
-      }
-    } else if (event != CACHE_EVENT_OPEN_READ || t_state.api_lock_url != HttpTransact::LOCK_URL_SECOND)
{
-      t_state.api_lock_url = HttpTransact::LOCK_URL_QUIT;
-    }
-  }
   // The write either succeeded or failed, notify transact
   call_transact_and_set_next_state(nullptr);
 
@@ -4539,11 +4522,8 @@ HttpSM::do_cache_delete_all_alts(Continuation *cont)
 inline void
 HttpSM::do_cache_prepare_write()
 {
-  // statistically no need to retry when we are trying to lock
-  // LOCK_URL_SECOND url because the server's behavior is unlikely to change
   milestones[TS_MILESTONE_CACHE_OPEN_WRITE_BEGIN] = Thread::get_hrtime();
-  bool retry                                      = (t_state.api_lock_url == HttpTransact::LOCK_URL_FIRST);
-  do_cache_prepare_action(&cache_sm, t_state.cache_info.object_read, retry);
+  do_cache_prepare_action(&cache_sm, t_state.cache_info.object_read, true);
 }
 
 inline void
@@ -4586,26 +4566,18 @@ HttpSM::do_cache_prepare_action(HttpCacheSM *c_sm, CacheHTTPInfo *object_read_in
 
   ink_assert(!pending_action);
 
-  if (t_state.api_lock_url == HttpTransact::LOCK_URL_FIRST) {
-    if (t_state.redirect_info.redirect_in_process) {
-      o_url = &(t_state.redirect_info.original_url);
-      ink_assert(o_url->valid());
-      restore_client_request = true;
-      s_url                  = o_url;
+  if (t_state.redirect_info.redirect_in_process) {
+    o_url = &(t_state.redirect_info.original_url);
+    ink_assert(o_url->valid());
+    restore_client_request = true;
+    s_url                  = o_url;
+  } else {
+    o_url = &(t_state.cache_info.original_url);
+    if (o_url->valid()) {
+      s_url = o_url;
     } else {
-      o_url = &(t_state.cache_info.original_url);
-      if (o_url->valid()) {
-        s_url = o_url;
-      } else {
-        s_url = t_state.cache_info.lookup_url;
-      }
+      s_url = t_state.cache_info.lookup_url;
     }
-  } else if (t_state.api_lock_url == HttpTransact::LOCK_URL_SECOND) {
-    s_url = &t_state.cache_info.lookup_url_storage;
-  } else {
-    ink_assert(t_state.api_lock_url == HttpTransact::LOCK_URL_ORIGINAL);
-    s_url                  = &(t_state.cache_info.original_url);
-    restore_client_request = true;
   }
 
   // modify client request to make it have the url we are going to
@@ -6792,9 +6764,6 @@ HttpSM::kill_this()
     }
 
     cache_sm.end_both();
-    if (second_cache_sm) {
-      second_cache_sm->end_both();
-    }
     transform_cache_sm.end_both();
     vc_table.cleanup_all();
 
diff --git a/proxy/http/HttpSM.h b/proxy/http/HttpSM.h
index 6deb150..6948d60 100644
--- a/proxy/http/HttpSM.h
+++ b/proxy/http/HttpSM.h
@@ -299,7 +299,6 @@ public:
   void txn_hook_prepend(TSHttpHookID id, INKContInternal *cont);
   APIHook *txn_hook_get(TSHttpHookID id);
 
-  void add_cache_sm();
   bool is_private();
   bool is_redirect_required();
 
@@ -388,7 +387,6 @@ protected:
 
   HttpCacheSM cache_sm;
   HttpCacheSM transform_cache_sm;
-  HttpCacheSM *second_cache_sm = nullptr;
 
   HttpSMHandler default_handler = nullptr;
   Action *pending_action        = nullptr;
@@ -696,22 +694,6 @@ HttpSM::txn_hook_get(TSHttpHookID id)
   return api_hooks.get(id);
 }
 
-inline void
-HttpSM::add_cache_sm()
-{
-  if (second_cache_sm == nullptr) {
-    second_cache_sm = new HttpCacheSM;
-    second_cache_sm->init(this, mutex);
-    if (t_state.cache_info.object_read != nullptr) {
-      second_cache_sm->cache_read_vc        = cache_sm.cache_read_vc;
-      cache_sm.cache_read_vc                = nullptr;
-      second_cache_sm->read_locked          = cache_sm.read_locked;
-      t_state.cache_info.second_object_read = t_state.cache_info.object_read;
-      t_state.cache_info.object_read        = nullptr;
-    }
-  }
-}
-
 inline bool
 HttpSM::is_transparent_passthrough_allowed()
 {
diff --git a/proxy/http/HttpTransact.h b/proxy/http/HttpTransact.h
index 72b6946..3ad3211 100644
--- a/proxy/http/HttpTransact.h
+++ b/proxy/http/HttpTransact.h
@@ -480,14 +480,6 @@ public:
     UPDATE_CACHED_OBJECT_FAIL
   };
 
-  enum LockUrl_t {
-    LOCK_URL_FIRST = 0,
-    LOCK_URL_SECOND,
-    LOCK_URL_ORIGINAL,
-    LOCK_URL_DONE,
-    LOCK_URL_QUIT,
-  };
-
   enum RangeSetup_t {
     RANGE_NONE = 0,
     RANGE_REQUESTED,
@@ -529,11 +521,10 @@ public:
     URL *lookup_url = nullptr;
     URL lookup_url_storage;
     URL original_url;
-    HTTPInfo *object_read        = nullptr;
-    HTTPInfo *second_object_read = nullptr;
     HTTPInfo object_store;
     HTTPInfo transform_store;
     CacheDirectives directives;
+    HTTPInfo *object_read             = nullptr;
     int open_read_retries             = 0;
     int open_write_retries            = 0;
     CacheWriteLock_t write_lock_state = CACHE_WL_INIT;
@@ -818,7 +809,6 @@ public:
     bool api_resp_cacheable                       = false;
     bool api_server_addr_set                      = false;
     UpdateCachedObject_t api_update_cached_object = UPDATE_CACHED_OBJECT_NONE;
-    LockUrl_t api_lock_url                        = LOCK_URL_FIRST;
     StateMachineAction_t saved_update_next_action = SM_ACTION_UNDEFINED;
     CacheAction_t saved_update_cache_action       = CACHE_DO_UNDEFINED;
 
diff --git a/src/traffic_server/InkAPI.cc b/src/traffic_server/InkAPI.cc
index 8dbebbc..b962561 100644
--- a/src/traffic_server/InkAPI.cc
+++ b/src/traffic_server/InkAPI.cc
@@ -5113,81 +5113,6 @@ TSHttpTxnCacheLookupUrlSet(TSHttpTxn txnp, TSMBuffer bufp, TSMLoc obj)
   return TS_SUCCESS;
 }
 
-// TS-1996: This API will be removed after v3.4.0 is cut. Do not use it!
-TSReturnCode
-TSHttpTxnNewCacheLookupDo(TSHttpTxn txnp, TSMBuffer bufp, TSMLoc url_loc)
-{
-  sdk_assert(sdk_sanity_check_txn(txnp) == TS_SUCCESS);
-  sdk_assert(sdk_sanity_check_mbuffer(bufp) == TS_SUCCESS);
-  sdk_assert(sdk_sanity_check_url_handle(url_loc) == TS_SUCCESS);
-
-  URL new_url, *client_url, *l_url, *o_url;
-  CryptoHash crypto_hash1, crypto_hash2;
-
-  new_url.m_heap     = ((HdrHeapSDKHandle *)bufp)->m_heap;
-  new_url.m_url_impl = (URLImpl *)url_loc;
-  if (!new_url.valid()) {
-    return TS_ERROR;
-  }
-
-  HttpSM *sm             = (HttpSM *)txnp;
-  HttpTransact::State *s = &(sm->t_state);
-
-  client_url = s->hdr_info.client_request.url_get();
-  if (!(client_url->valid())) {
-    return TS_ERROR;
-  }
-
-  // if l_url is not valid, then no cache lookup has been done yet
-  // so we shouldn't be calling TSHttpTxnNewCacheLookupDo right now
-  l_url = s->cache_info.lookup_url;
-  if (!l_url || !l_url->valid()) {
-    s->cache_info.lookup_url_storage.create(nullptr);
-    s->cache_info.lookup_url = &(s->cache_info.lookup_url_storage);
-    l_url                    = s->cache_info.lookup_url;
-  } else {
-    l_url->hash_get(&crypto_hash1);
-    new_url.hash_get(&crypto_hash2);
-    if (crypto_hash1 == crypto_hash2) {
-      return TS_ERROR;
-    }
-    o_url = &(s->cache_info.original_url);
-    if (!o_url->valid()) {
-      o_url->create(nullptr);
-      o_url->copy(l_url);
-    }
-  }
-
-  // copy the new_url to lookup_url
-  l_url->copy(&new_url);
-
-  // bypass HttpTransact::HandleFiltering
-  s->transact_return_point = HttpTransact::DecideCacheLookup;
-  s->cache_info.action     = HttpTransact::CACHE_DO_LOOKUP;
-  sm->add_cache_sm();
-  s->api_cleanup_cache_read = true;
-
-  return TS_SUCCESS;
-}
-
-// TS-1996: This API will be removed after v3.4.0 is cut. Do not use it!
-TSReturnCode
-TSHttpTxnSecondUrlTryLock(TSHttpTxn txnp)
-{
-  sdk_assert(sdk_sanity_check_txn(txnp) == TS_SUCCESS);
-
-  HttpSM *sm             = (HttpSM *)txnp;
-  HttpTransact::State *s = &(sm->t_state);
-  // TSHttpTxnNewCacheLookupDo didn't continue
-  if (!s->cache_info.original_url.valid()) {
-    return TS_ERROR;
-  }
-  sm->add_cache_sm();
-  s->api_lock_url = HttpTransact::LOCK_URL_SECOND;
-
-  return TS_SUCCESS;
-}
-
 /*
  * TSHttpTxnRedirectRequest is very odd.  It is only in experimental.h.
  * It is not used in any checked in code.  We should probably remove this.


Mime
View raw message