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 <zw...@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.
