This is an automated email from the ASF dual-hosted git repository. zwoop pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push: new eebc381 This removes the second CacheSM, and related APIs eebc381 is described below commit eebc3811851b561fa2b3126b587d9c439afefab9 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. --- .../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.