From 3d0492f7276bb93aff499f637ff843c45908b193 Mon Sep 17 00:00:00 2001 From: Kazuhiko SHIOZAKI Date: Mon, 25 Jan 2021 13:24:11 +0000 Subject: [PATCH 1/4] Do not provide a stale negative cache (#7422) --- proxy/http/HttpTransact.cc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc index ea38992e569..43460132fe5 100644 --- a/proxy/http/HttpTransact.cc +++ b/proxy/http/HttpTransact.cc @@ -4328,6 +4328,7 @@ HttpTransact::handle_cache_operation_on_forward_server_response(State *s) return; default: + HTTPStatus cached_response_code = s->cache_info.object_read->response_get()->status_get(); TxnDebug("http_trans", "[hcoofsr] response code: %d", server_response_code); SET_VIA_STRING(VIA_SERVER_RESULT, VIA_SERVER_SERVED); SET_VIA_STRING(VIA_PROXY_RESULT, VIA_PROXY_SERVED); @@ -4340,7 +4341,9 @@ HttpTransact::handle_cache_operation_on_forward_server_response(State *s) if ((server_response_code == HTTP_STATUS_INTERNAL_SERVER_ERROR || server_response_code == HTTP_STATUS_GATEWAY_TIMEOUT || server_response_code == HTTP_STATUS_BAD_GATEWAY || server_response_code == HTTP_STATUS_SERVICE_UNAVAILABLE) && s->cache_info.action == CACHE_DO_UPDATE && s->txn_conf->negative_revalidating_enabled && - is_stale_cache_response_returnable(s)) { + is_stale_cache_response_returnable(s) && + (!(cached_response_code == HTTP_STATUS_INTERNAL_SERVER_ERROR || cached_response_code == HTTP_STATUS_GATEWAY_TIMEOUT || + cached_response_code == HTTP_STATUS_BAD_GATEWAY || cached_response_code == HTTP_STATUS_SERVICE_UNAVAILABLE))) { TxnDebug("http_trans", "[hcoofsr] negative revalidating: revalidate stale object and serve from cache"); s->cache_info.object_store.create(); From c6f525986816cb698ba8bcf4b2d432b4e94f8997 Mon Sep 17 00:00:00 2001 From: Kazuhiko SHIOZAKI Date: Thu, 28 Jan 2021 12:17:48 +0000 Subject: [PATCH 2/4] Revert "Do not provide a stale negative cache (#7422)" This reverts commit 3d0492f7276bb93aff499f637ff843c45908b193. --- proxy/http/HttpTransact.cc | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc index 43460132fe5..ea38992e569 100644 --- a/proxy/http/HttpTransact.cc +++ b/proxy/http/HttpTransact.cc @@ -4328,7 +4328,6 @@ HttpTransact::handle_cache_operation_on_forward_server_response(State *s) return; default: - HTTPStatus cached_response_code = s->cache_info.object_read->response_get()->status_get(); TxnDebug("http_trans", "[hcoofsr] response code: %d", server_response_code); SET_VIA_STRING(VIA_SERVER_RESULT, VIA_SERVER_SERVED); SET_VIA_STRING(VIA_PROXY_RESULT, VIA_PROXY_SERVED); @@ -4341,9 +4340,7 @@ HttpTransact::handle_cache_operation_on_forward_server_response(State *s) if ((server_response_code == HTTP_STATUS_INTERNAL_SERVER_ERROR || server_response_code == HTTP_STATUS_GATEWAY_TIMEOUT || server_response_code == HTTP_STATUS_BAD_GATEWAY || server_response_code == HTTP_STATUS_SERVICE_UNAVAILABLE) && s->cache_info.action == CACHE_DO_UPDATE && s->txn_conf->negative_revalidating_enabled && - is_stale_cache_response_returnable(s) && - (!(cached_response_code == HTTP_STATUS_INTERNAL_SERVER_ERROR || cached_response_code == HTTP_STATUS_GATEWAY_TIMEOUT || - cached_response_code == HTTP_STATUS_BAD_GATEWAY || cached_response_code == HTTP_STATUS_SERVICE_UNAVAILABLE))) { + is_stale_cache_response_returnable(s)) { TxnDebug("http_trans", "[hcoofsr] negative revalidating: revalidate stale object and serve from cache"); s->cache_info.object_store.create(); From 254e9e22181ca369673407bd3fcd93e7287275ac Mon Sep 17 00:00:00 2001 From: Kazuhiko SHIOZAKI Date: Thu, 28 Jan 2021 12:24:32 +0000 Subject: [PATCH 3/4] Do not provide a stale negative cache (#7422) --- proxy/http/HttpTransact.cc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc index ea38992e569..fe99b7eb89f 100644 --- a/proxy/http/HttpTransact.cc +++ b/proxy/http/HttpTransact.cc @@ -4341,6 +4341,9 @@ HttpTransact::handle_cache_operation_on_forward_server_response(State *s) server_response_code == HTTP_STATUS_BAD_GATEWAY || server_response_code == HTTP_STATUS_SERVICE_UNAVAILABLE) && s->cache_info.action == CACHE_DO_UPDATE && s->txn_conf->negative_revalidating_enabled && is_stale_cache_response_returnable(s)) { + HTTPStatus cached_response_code = s->cache_info.object_read->response_get()->status_get(); + if (!(cached_response_code == HTTP_STATUS_INTERNAL_SERVER_ERROR || cached_response_code == HTTP_STATUS_GATEWAY_TIMEOUT || + cached_response_code == HTTP_STATUS_BAD_GATEWAY || cached_response_code == HTTP_STATUS_SERVICE_UNAVAILABLE)) { TxnDebug("http_trans", "[hcoofsr] negative revalidating: revalidate stale object and serve from cache"); s->cache_info.object_store.create(); @@ -4396,6 +4399,7 @@ HttpTransact::handle_cache_operation_on_forward_server_response(State *s) } return; + } } s->next_action = SM_ACTION_SERVER_READ; From f1229dc02a6a0fcc3200e794db74054a1e53bd0a Mon Sep 17 00:00:00 2001 From: Kazuhiko SHIOZAKI Date: Thu, 28 Jan 2021 12:25:48 +0000 Subject: [PATCH 4/4] Format change only --- proxy/http/HttpTransact.cc | 98 +++++++++++++++++++------------------- 1 file changed, 49 insertions(+), 49 deletions(-) diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc index fe99b7eb89f..e2c0e383fba 100644 --- a/proxy/http/HttpTransact.cc +++ b/proxy/http/HttpTransact.cc @@ -4343,62 +4343,62 @@ HttpTransact::handle_cache_operation_on_forward_server_response(State *s) is_stale_cache_response_returnable(s)) { HTTPStatus cached_response_code = s->cache_info.object_read->response_get()->status_get(); if (!(cached_response_code == HTTP_STATUS_INTERNAL_SERVER_ERROR || cached_response_code == HTTP_STATUS_GATEWAY_TIMEOUT || - cached_response_code == HTTP_STATUS_BAD_GATEWAY || cached_response_code == HTTP_STATUS_SERVICE_UNAVAILABLE)) { - TxnDebug("http_trans", "[hcoofsr] negative revalidating: revalidate stale object and serve from cache"); - - s->cache_info.object_store.create(); - s->cache_info.object_store.request_set(&s->hdr_info.client_request); - s->cache_info.object_store.response_set(s->cache_info.object_read->response_get()); - base_response = s->cache_info.object_store.response_get(); - time_t exp_time = s->txn_conf->negative_revalidating_lifetime + ink_local_time(); - base_response->set_expires(exp_time); - - SET_VIA_STRING(VIA_CACHE_FILL_ACTION, VIA_CACHE_UPDATED); - HTTP_INCREMENT_DYN_STAT(http_cache_updates_stat); - - // unset Cache-control: "need-revalidate-once" (if it's set) - // This directive is used internally by T.S. to invalidate - // documents so that an invalidated document needs to be - // revalidated again. - base_response->unset_cooked_cc_need_revalidate_once(); - - if (is_request_conditional(&s->hdr_info.client_request) && - HttpTransactCache::match_response_to_request_conditionals(&s->hdr_info.client_request, - s->cache_info.object_read->response_get(), - s->response_received_time) == HTTP_STATUS_NOT_MODIFIED) { - s->next_action = SM_ACTION_INTERNAL_CACHE_UPDATE_HEADERS; - client_response_code = HTTP_STATUS_NOT_MODIFIED; - } else { - if (s->method == HTTP_WKSIDX_HEAD) { - s->cache_info.action = CACHE_DO_UPDATE; - s->next_action = SM_ACTION_INTERNAL_CACHE_NOOP; + cached_response_code == HTTP_STATUS_BAD_GATEWAY || cached_response_code == HTTP_STATUS_SERVICE_UNAVAILABLE)) { + TxnDebug("http_trans", "[hcoofsr] negative revalidating: revalidate stale object and serve from cache"); + + s->cache_info.object_store.create(); + s->cache_info.object_store.request_set(&s->hdr_info.client_request); + s->cache_info.object_store.response_set(s->cache_info.object_read->response_get()); + base_response = s->cache_info.object_store.response_get(); + time_t exp_time = s->txn_conf->negative_revalidating_lifetime + ink_local_time(); + base_response->set_expires(exp_time); + + SET_VIA_STRING(VIA_CACHE_FILL_ACTION, VIA_CACHE_UPDATED); + HTTP_INCREMENT_DYN_STAT(http_cache_updates_stat); + + // unset Cache-control: "need-revalidate-once" (if it's set) + // This directive is used internally by T.S. to invalidate + // documents so that an invalidated document needs to be + // revalidated again. + base_response->unset_cooked_cc_need_revalidate_once(); + + if (is_request_conditional(&s->hdr_info.client_request) && + HttpTransactCache::match_response_to_request_conditionals(&s->hdr_info.client_request, + s->cache_info.object_read->response_get(), + s->response_received_time) == HTTP_STATUS_NOT_MODIFIED) { + s->next_action = SM_ACTION_INTERNAL_CACHE_UPDATE_HEADERS; + client_response_code = HTTP_STATUS_NOT_MODIFIED; } else { - s->cache_info.action = CACHE_DO_SERVE_AND_UPDATE; - s->next_action = SM_ACTION_SERVE_FROM_CACHE; - } + if (s->method == HTTP_WKSIDX_HEAD) { + s->cache_info.action = CACHE_DO_UPDATE; + s->next_action = SM_ACTION_INTERNAL_CACHE_NOOP; + } else { + s->cache_info.action = CACHE_DO_SERVE_AND_UPDATE; + s->next_action = SM_ACTION_SERVE_FROM_CACHE; + } - client_response_code = s->cache_info.object_read->response_get()->status_get(); - } + client_response_code = s->cache_info.object_read->response_get()->status_get(); + } - ink_assert(base_response->valid()); + ink_assert(base_response->valid()); - if (client_response_code == HTTP_STATUS_NOT_MODIFIED) { - ink_assert(GET_VIA_STRING(VIA_CLIENT_REQUEST) != VIA_CLIENT_SIMPLE); - SET_VIA_STRING(VIA_CLIENT_REQUEST, VIA_CLIENT_IMS); - SET_VIA_STRING(VIA_PROXY_RESULT, VIA_PROXY_NOT_MODIFIED); - } else { - SET_VIA_STRING(VIA_PROXY_RESULT, VIA_PROXY_SERVED); - } + if (client_response_code == HTTP_STATUS_NOT_MODIFIED) { + ink_assert(GET_VIA_STRING(VIA_CLIENT_REQUEST) != VIA_CLIENT_SIMPLE); + SET_VIA_STRING(VIA_CLIENT_REQUEST, VIA_CLIENT_IMS); + SET_VIA_STRING(VIA_PROXY_RESULT, VIA_PROXY_NOT_MODIFIED); + } else { + SET_VIA_STRING(VIA_PROXY_RESULT, VIA_PROXY_SERVED); + } - ink_assert(client_response_code != HTTP_STATUS_NONE); + ink_assert(client_response_code != HTTP_STATUS_NONE); - if (s->next_action == SM_ACTION_SERVE_FROM_CACHE && s->state_machine->do_transform_open()) { - set_header_for_transform(s, base_response); - } else { - build_response(s, base_response, &s->hdr_info.client_response, s->client_info.http_version, client_response_code); - } + if (s->next_action == SM_ACTION_SERVE_FROM_CACHE && s->state_machine->do_transform_open()) { + set_header_for_transform(s, base_response); + } else { + build_response(s, base_response, &s->hdr_info.client_response, s->client_info.http_version, client_response_code); + } - return; + return; } }