From 63f2b158b5833f087a50565808f05d290690de2b Mon Sep 17 00:00:00 2001 From: Bryan Call Date: Wed, 26 Aug 2020 16:30:27 -0700 Subject: [PATCH 1/4] New option to dead server to not retry during dead period --- proxy/http/HttpSM.cc | 6 ++++++ proxy/http/HttpTransact.cc | 11 +++++++++++ proxy/http/HttpTransact.h | 3 ++- 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc index ae4b5a03adf..7719dc0c4b9 100644 --- a/proxy/http/HttpSM.cc +++ b/proxy/http/HttpSM.cc @@ -4936,6 +4936,12 @@ HttpSM::do_http_server_open(bool raw) call_transact_and_set_next_state(HttpTransact::Forbidden); return; } + + if (HttpTransact::is_server_negative_cached(&t_state) == true && + t_state.txn_conf->connect_attempts_max_retries_dead_server < 0) { + call_transact_and_set_next_state(HttpTransact::OriginDead); + return; + } } // Check for self loop. diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc index ef7781dfd90..c08ee1bd9c7 100644 --- a/proxy/http/HttpTransact.cc +++ b/proxy/http/HttpTransact.cc @@ -865,6 +865,15 @@ HttpTransact::TooEarly(State *s) TRANSACT_RETURN(SM_ACTION_SEND_ERROR_CACHE_NOOP, nullptr); } +void +HttpTransact::OriginDead(State *s) +{ + TxnDebug("http_trans", "origin server is marked down"); + bootstrap_state_variables_from_request(s, &s->hdr_info.client_request); + build_error_response(s, HTTP_STATUS_BAD_GATEWAY, "Origin Server Marked Down", "connect#failed_connect"); + TRANSACT_RETURN(SM_ACTION_SEND_ERROR_CACHE_NOOP, nullptr); +} + void HttpTransact::HandleBlindTunnel(State *s) { @@ -3776,6 +3785,8 @@ HttpTransact::handle_response_from_server(State *s) max_connect_retries = s->txn_conf->connect_attempts_max_retries; } + TxnDebug("http_trans", "max_connect_retries: %d s->current.attempts: %d", max_connect_retries, s->current.attempts); + if (is_request_retryable(s) && s->current.attempts < max_connect_retries) { // If this is a round robin DNS entry & we're tried configured // number of times, we should try another node diff --git a/proxy/http/HttpTransact.h b/proxy/http/HttpTransact.h index df7986a4588..97cb1755cdc 100644 --- a/proxy/http/HttpTransact.h +++ b/proxy/http/HttpTransact.h @@ -937,8 +937,9 @@ class HttpTransact static void HandleRequestAuthorized(State *s); static void BadRequest(State *s); static void Forbidden(State *s); - static void TooEarly(State *s); static void SelfLoop(State *s); + static void TooEarly(State *s); + static void OriginDead(State *s); static void PostActiveTimeoutResponse(State *s); static void PostInactiveTimeoutResponse(State *s); static void DecideCacheLookup(State *s); From d05fb64175f3e2fcbf326f4b88ac70a4409c305e Mon Sep 17 00:00:00 2001 From: Bryan Call Date: Thu, 27 Aug 2020 10:03:52 -0700 Subject: [PATCH 2/4] Update to not inline HttpTransact::is_server_negative_cached --- proxy/http/HttpTransact.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc index c08ee1bd9c7..fb6893f1538 100644 --- a/proxy/http/HttpTransact.cc +++ b/proxy/http/HttpTransact.cc @@ -451,7 +451,7 @@ update_cache_control_information_from_config(HttpTransact::State *s) } } -inline bool +bool HttpTransact::is_server_negative_cached(State *s) { if (s->host_db_info.app.http_data.last_failure != 0 && From 2a4b0431b6c62897954108029233d9710741ac14 Mon Sep 17 00:00:00 2001 From: Bryan Call Date: Tue, 29 Sep 2020 15:36:30 -0700 Subject: [PATCH 3/4] Decided to make the value the actual number of retries --- proxy/http/HttpSM.cc | 2 +- proxy/http/HttpTransact.cc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc index 7719dc0c4b9..7e9ba7d3bee 100644 --- a/proxy/http/HttpSM.cc +++ b/proxy/http/HttpSM.cc @@ -4938,7 +4938,7 @@ HttpSM::do_http_server_open(bool raw) } if (HttpTransact::is_server_negative_cached(&t_state) == true && - t_state.txn_conf->connect_attempts_max_retries_dead_server < 0) { + t_state.txn_conf->connect_attempts_max_retries_dead_server <= 0) { call_transact_and_set_next_state(HttpTransact::OriginDead); return; } diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc index fb6893f1538..66954e300e2 100644 --- a/proxy/http/HttpTransact.cc +++ b/proxy/http/HttpTransact.cc @@ -3779,7 +3779,7 @@ HttpTransact::handle_response_from_server(State *s) } if (is_server_negative_cached(s)) { - max_connect_retries = s->txn_conf->connect_attempts_max_retries_dead_server; + max_connect_retries = s->txn_conf->connect_attempts_max_retries_dead_server - 1; } else { // server not yet negative cached - use default number of retries max_connect_retries = s->txn_conf->connect_attempts_max_retries; From d6045b560af131b305b5860a6e04f467ad1a9ab9 Mon Sep 17 00:00:00 2001 From: Bryan Call Date: Tue, 29 Sep 2020 16:01:12 -0700 Subject: [PATCH 4/4] Docs update --- doc/admin-guide/files/records.config.en.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/admin-guide/files/records.config.en.rst b/doc/admin-guide/files/records.config.en.rst index 78e3551ed2c..ee973029eec 100644 --- a/doc/admin-guide/files/records.config.en.rst +++ b/doc/admin-guide/files/records.config.en.rst @@ -1452,7 +1452,7 @@ Origin Server Connect Attempts :reloadable: :overridable: - Maximum number of connection retries |TS| can make while an origin is marked dead. Typically this value is smaller than + Maximum number of connection attempts |TS| can make while an origin is marked dead per request. Typically this value is smaller than `proxy.config.http.connect_attempts_max_retries`_ so an error is returned to the client faster and also to reduce the load on the dead origin. The timeout interval `proxy.config.http.connect_attempts_timeout`_ in seconds is used with this setting.