From 0c63fd307afa4267394f107b49e444b280c878ea Mon Sep 17 00:00:00 2001 From: vijayabhaskar Date: Wed, 1 Feb 2017 17:12:36 +0000 Subject: [PATCH] retry safe requests (cherry picked from commit a57f6609644752b8a2e81bcd4872f64575331c1e) Conflicts: doc/developer-guide/api/functions/TSHttpOverridableConfig.en.rst proxy/InkAPI.cc proxy/http/HttpConfig.cc proxy/http/HttpConfig.h proxy/http/HttpSM.cc proxy/http/HttpTransact.cc --- doc/admin-guide/files/records.config.en.rst | 13 +++++++++++++ .../api/functions/TSHttpOverridableConfig.en.rst | 1 + lib/ts/apidefs.h.in | 1 + mgmt/RecordsConfig.cc | 2 ++ plugins/experimental/ts_lua/ts_lua_http_config.c | 2 ++ proxy/InkAPI.cc | 6 ++++++ proxy/InkAPITest.cc | 1 + proxy/http/HttpConfig.cc | 2 ++ proxy/http/HttpConfig.h | 2 ++ proxy/http/HttpTransact.cc | 5 ++++- proxy/http/HttpTransactHeaders.cc | 13 +++++++++++++ proxy/http/HttpTransactHeaders.h | 2 ++ 12 files changed, 49 insertions(+), 1 deletion(-) diff --git a/doc/admin-guide/files/records.config.en.rst b/doc/admin-guide/files/records.config.en.rst index 84b01e10662..8ccbc863de2 100644 --- a/doc/admin-guide/files/records.config.en.rst +++ b/doc/admin-guide/files/records.config.en.rst @@ -910,6 +910,19 @@ Value Effect according to this setting then it will be used, otherwise it will be released to the pool and a different session selected or created. +.. ts:cv:: CONFIG proxy.config.http.safe_requests_retryable INT 1 + :overridable: + + This setting, on by default, allows requests which are considered safe to be retried on an error. + See https://tools.ietf.org/html/rfc7231#section-4.2.1 to RFC for details on which request methods are considered safe. + + If this setting is ``0`` then ATS retries a failed origin server request only if the bytes sent by ATS + are not acknowledged by the origin server. + + If this setting is ``1`` then ATS retries all the safe methods to a failed origin server irrespective of + previous connection failure status. + + .. ts:cv:: CONFIG proxy.config.http.record_heartbeat INT 0 :reloadable: diff --git a/doc/developer-guide/api/functions/TSHttpOverridableConfig.en.rst b/doc/developer-guide/api/functions/TSHttpOverridableConfig.en.rst index cbf840027eb..1f4575f8d54 100644 --- a/doc/developer-guide/api/functions/TSHttpOverridableConfig.en.rst +++ b/doc/developer-guide/api/functions/TSHttpOverridableConfig.en.rst @@ -81,6 +81,7 @@ The following configurations (from ``records.config``) are overridable. | :ts:cv:`proxy.config.http.anonymize_remove_cookie` | :ts:cv:`proxy.config.http.anonymize_remove_client_ip` | :ts:cv:`proxy.config.http.anonymize_insert_client_ip` +| :ts:cv:`proxy.config.http.safe_requests_retryable` | :ts:cv:`proxy.config.http.response_server_enabled` | :ts:cv:`proxy.config.http.insert_squid_x_forwarded_for` | :ts:cv:`proxy.config.http.server_tcp_init_cwnd` diff --git a/lib/ts/apidefs.h.in b/lib/ts/apidefs.h.in index 6ab3d98abde..7da895a5cc7 100644 --- a/lib/ts/apidefs.h.in +++ b/lib/ts/apidefs.h.in @@ -675,6 +675,7 @@ typedef enum { TS_CONFIG_HTTP_CACHE_MAX_OPEN_WRITE_RETRIES, TS_CONFIG_HTTP_REDIRECT_USE_ORIG_CACHE_KEY, TS_CONFIG_HTTP_ATTACH_SERVER_SESSION_TO_CLIENT, + TS_CONFIG_HTTP_SAFE_REQUESTS_RETRYABLE, TS_CONFIG_HTTP_ORIGIN_MAX_CONNECTIONS_QUEUE, TS_CONFIG_WEBSOCKET_NO_ACTIVITY_TIMEOUT, TS_CONFIG_WEBSOCKET_ACTIVE_TIMEOUT, diff --git a/mgmt/RecordsConfig.cc b/mgmt/RecordsConfig.cc index 6d852089b5d..77f2ccac3c6 100644 --- a/mgmt/RecordsConfig.cc +++ b/mgmt/RecordsConfig.cc @@ -478,6 +478,8 @@ static const RecordElement RecordsConfig[] = , {RECT_CONFIG, "proxy.config.http.attach_server_session_to_client", RECD_INT, "0", RECU_DYNAMIC, RR_NULL, RECC_INT, "[0-1]", RECA_NULL} , + {RECT_CONFIG, "proxy.config.http.safe_requests_retryable", RECD_INT, "1", RECU_DYNAMIC, RR_NULL, RECC_INT, "[0-1]", RECA_NULL} + , {RECT_CONFIG, "proxy.config.net.max_connections_in", RECD_INT, "30000", RECU_DYNAMIC, RR_NULL, RECC_STR, "^[0-9]+$", RECA_NULL} , {RECT_CONFIG, "proxy.config.net.max_connections_active_in", RECD_INT, "10000", RECU_DYNAMIC, RR_NULL, RECC_STR, "^[0-9]+$", RECA_NULL} diff --git a/plugins/experimental/ts_lua/ts_lua_http_config.c b/plugins/experimental/ts_lua/ts_lua_http_config.c index 6aed5382413..70d692e8944 100644 --- a/plugins/experimental/ts_lua/ts_lua_http_config.c +++ b/plugins/experimental/ts_lua/ts_lua_http_config.c @@ -113,6 +113,7 @@ typedef enum { TS_LUA_CONFIG_HTTP_CACHE_MAX_OPEN_WRITE_RETRIES = TS_CONFIG_HTTP_CACHE_MAX_OPEN_WRITE_RETRIES, TS_LUA_CONFIG_HTTP_REDIRECT_USE_ORIG_CACHE_KEY = TS_CONFIG_HTTP_REDIRECT_USE_ORIG_CACHE_KEY, TS_LUA_CONFIG_HTTP_ATTACH_SERVER_SESSION_TO_CLIENT = TS_CONFIG_HTTP_ATTACH_SERVER_SESSION_TO_CLIENT, + TS_LUA_CONFIG_HTTP_SAFE_REQUESTS_RETRYABLE = TS_CONFIG_HTTP_SAFE_REQUESTS_RETRYABLE, TS_LUA_CONFIG_HTTP_ORIGIN_MAX_CONNECTIONS_QUEUE = TS_CONFIG_HTTP_ORIGIN_MAX_CONNECTIONS_QUEUE, TS_LUA_CONFIG_WEBSOCKET_NO_ACTIVITY_TIMEOUT = TS_CONFIG_WEBSOCKET_NO_ACTIVITY_TIMEOUT, TS_LUA_CONFIG_WEBSOCKET_ACTIVE_TIMEOUT = TS_CONFIG_WEBSOCKET_ACTIVE_TIMEOUT, @@ -225,6 +226,7 @@ ts_lua_var_item ts_lua_http_config_vars[] = { TS_LUA_MAKE_VAR_ITEM(TS_LUA_CONFIG_HTTP_CACHE_MAX_OPEN_WRITE_RETRIES), TS_LUA_MAKE_VAR_ITEM(TS_LUA_CONFIG_HTTP_REDIRECT_USE_ORIG_CACHE_KEY), TS_LUA_MAKE_VAR_ITEM(TS_LUA_CONFIG_HTTP_ATTACH_SERVER_SESSION_TO_CLIENT), + TS_LUA_MAKE_VAR_ITEM(TS_LUA_CONFIG_HTTP_SAFE_REQUESTS_RETRYABLE), TS_LUA_MAKE_VAR_ITEM(TS_LUA_CONFIG_HTTP_ORIGIN_MAX_CONNECTIONS_QUEUE), TS_LUA_MAKE_VAR_ITEM(TS_LUA_CONFIG_WEBSOCKET_NO_ACTIVITY_TIMEOUT), TS_LUA_MAKE_VAR_ITEM(TS_LUA_CONFIG_WEBSOCKET_ACTIVE_TIMEOUT), diff --git a/proxy/InkAPI.cc b/proxy/InkAPI.cc index 43ec6f184bf..26326a47217 100644 --- a/proxy/InkAPI.cc +++ b/proxy/InkAPI.cc @@ -7956,6 +7956,10 @@ _conf_to_memberp(TSOverridableConfigKey conf, OverridableHttpConfigParams *overr typ = OVERRIDABLE_TYPE_INT; ret = &overridableHttpConfig->attach_server_session_to_client; break; + case TS_CONFIG_HTTP_SAFE_REQUESTS_RETRYABLE: + typ = OVERRIDABLE_TYPE_INT; + ret = &overridableHttpConfig->safe_requests_retryable; + break; case TS_CONFIG_HTTP_ORIGIN_MAX_CONNECTIONS_QUEUE: typ = OVERRIDABLE_TYPE_INT; ret = &overridableHttpConfig->origin_max_connections_queue; @@ -8369,6 +8373,8 @@ TSHttpTxnConfigFind(const char *name, int length, TSOverridableConfigKey *conf, cnf = TS_CONFIG_HTTP_ANONYMIZE_REMOVE_COOKIE; else if (!strncmp(name, "proxy.config.http.request_header_max_size", length)) cnf = TS_CONFIG_HTTP_REQUEST_HEADER_MAX_SIZE; + else if (!strncmp(name, "proxy.config.http.safe_requests_retryable", length)) + cnf = TS_CONFIG_HTTP_SAFE_REQUESTS_RETRYABLE; break; case 'r': if (!strncmp(name, "proxy.config.http.insert_response_via_str", length)) diff --git a/proxy/InkAPITest.cc b/proxy/InkAPITest.cc index 45f0f4ba590..edbe44eeb8a 100644 --- a/proxy/InkAPITest.cc +++ b/proxy/InkAPITest.cc @@ -7481,6 +7481,7 @@ const char *SDK_Overridable_Configs[TS_CONFIG_LAST_ENTRY] = { "proxy.config.http.cache.max_open_write_retries", "proxy.config.http.redirect_use_orig_cache_key", "proxy.config.http.attach_server_session_to_client", + "proxy.config.http.safe_requests_retryable", "proxy.config.http.origin_max_connections_queue", "proxy.config.websocket.no_activity_timeout", "proxy.config.websocket.active_timeout", diff --git a/proxy/http/HttpConfig.cc b/proxy/http/HttpConfig.cc index 22c3b4e4e3d..ec5b0b960ea 100644 --- a/proxy/http/HttpConfig.cc +++ b/proxy/http/HttpConfig.cc @@ -887,6 +887,7 @@ HttpConfig::startup() HttpEstablishStaticConfigLongLong(c.oride.origin_max_connections_queue, "proxy.config.http.origin_max_connections_queue"); HttpEstablishStaticConfigLongLong(c.origin_min_keep_alive_connections, "proxy.config.http.origin_min_keep_alive_connections"); HttpEstablishStaticConfigLongLong(c.oride.attach_server_session_to_client, "proxy.config.http.attach_server_session_to_client"); + HttpEstablishStaticConfigByte(c.oride.safe_requests_retryable, "proxy.config.http.safe_requests_retryable"); // Wank me. HttpEstablishStaticConfigByte(c.disable_ssl_parenting, "proxy.local.http.parent_proxy.disable_connect_tunneling"); @@ -1165,6 +1166,7 @@ HttpConfig::reconfigure() } params->origin_min_keep_alive_connections = m_master.origin_min_keep_alive_connections; params->oride.attach_server_session_to_client = m_master.oride.attach_server_session_to_client; + params->oride.safe_requests_retryable = m_master.oride.safe_requests_retryable; if (params->oride.origin_max_connections && params->oride.origin_max_connections < params->origin_min_keep_alive_connections) { Warning("origin_max_connections < origin_min_keep_alive_connections, setting min=max , please correct your records.config"); diff --git a/proxy/http/HttpConfig.h b/proxy/http/HttpConfig.h index 56f2f4aebc7..0766013ebc5 100644 --- a/proxy/http/HttpConfig.h +++ b/proxy/http/HttpConfig.h @@ -372,6 +372,7 @@ struct OverridableHttpConfigParams { auth_server_session_private(1), fwd_proxy_auth_to_parent(0), uncacheable_requests_bypass_parent(1), + safe_requests_retryable(1), insert_age_in_response(1), anonymize_remove_from(0), anonymize_remove_referer(0), @@ -494,6 +495,7 @@ struct OverridableHttpConfigParams { MgmtByte auth_server_session_private; MgmtByte fwd_proxy_auth_to_parent; MgmtByte uncacheable_requests_bypass_parent; + MgmtByte safe_requests_retryable; MgmtByte insert_age_in_response; diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc index 3d00a457873..3d919eaad52 100644 --- a/proxy/http/HttpTransact.cc +++ b/proxy/http/HttpTransact.cc @@ -6481,8 +6481,11 @@ HttpTransact::is_request_valid(State *s, HTTPHdr *incoming_request) bool HttpTransact::is_request_retryable(State *s) { + // If safe requests are retryable, it should be safe to retry safe requests irrespective of bytes sent or connection state + // according to RFC the following methods are safe (https://tools.ietf.org/html/rfc7231#section-4.2.1) // If there was no error establishing the connection (and we sent bytes)-- we cannot retry - if (s->current.state != CONNECTION_ERROR && s->state_machine->server_request_hdr_bytes > 0) { + if (!(s->txn_conf->safe_requests_retryable && HttpTransactHeaders::is_method_safe(s->method)) && + (s->current.state != CONNECTION_ERROR && s->state_machine->server_request_hdr_bytes > 0)) { return false; } diff --git a/proxy/http/HttpTransactHeaders.cc b/proxy/http/HttpTransactHeaders.cc index 9d4e2bd6466..1fdc1aff234 100644 --- a/proxy/http/HttpTransactHeaders.cc +++ b/proxy/http/HttpTransactHeaders.cc @@ -71,6 +71,19 @@ HttpTransactHeaders::is_this_method_supported(int the_scheme, int the_method) return false; } +bool +HttpTransactHeaders::is_method_safe(int method) +{ + return (method == HTTP_WKSIDX_GET || method == HTTP_WKSIDX_OPTIONS || method == HTTP_WKSIDX_HEAD || method == HTTP_WKSIDX_TRACE); +} + +bool +HttpTransactHeaders::is_method_idempotent(int method) +{ + return (method == HTTP_WKSIDX_CONNECT || method == HTTP_WKSIDX_DELETE || method == HTTP_WKSIDX_GET || + method == HTTP_WKSIDX_HEAD || method == HTTP_WKSIDX_PUT || method == HTTP_WKSIDX_OPTIONS || method == HTTP_WKSIDX_TRACE); +} + void HttpTransactHeaders::insert_supported_methods_in_response(HTTPHdr *response, int scheme) { diff --git a/proxy/http/HttpTransactHeaders.h b/proxy/http/HttpTransactHeaders.h index f6719ab224c..c27a6a36968 100644 --- a/proxy/http/HttpTransactHeaders.h +++ b/proxy/http/HttpTransactHeaders.h @@ -57,6 +57,8 @@ class HttpTransactHeaders ink_time_t base_response_date, ink_time_t now); static bool does_server_allow_response_to_be_stored(HTTPHdr *resp); static bool downgrade_request(bool *origin_server_keep_alive, HTTPHdr *outgoing_request); + static bool is_method_safe(int method); + static bool is_method_idempotent(int method); static void generate_and_set_squid_codes(HTTPHdr *header, char *via_string, HttpTransact::SquidLogInfo *squid_codes);