trafficserver-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From zw...@apache.org
Subject [trafficserver] branch 7.1.x updated: retry safe requests
Date Fri, 03 Feb 2017 23:53:31 GMT
This is an automated email from the ASF dual-hosted git repository.

zwoop pushed a commit to branch 7.1.x
in repository https://git-dual.apache.org/repos/asf/trafficserver.git

The following commit(s) were added to refs/heads/7.1.x by this push:
       new  9cbac37   retry safe requests
9cbac37 is described below

commit 9cbac37750fe08e8bafbeb4bd3df82f1d4a06daa
Author: vijayabhaskar <vijayabhaskar_mamidi@yahoo.com>
AuthorDate: Wed Feb 1 17:12:36 2017 +0000

    retry safe requests
    
    (cherry picked from commit a57f6609644752b8a2e81bcd4872f64575331c1e)
---
 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                                     |  3 +++
 proxy/http/HttpSM.cc                                        |  7 ++++---
 proxy/http/HttpTransact.cc                                  |  7 +++++--
 proxy/http/HttpTransactHeaders.cc                           | 13 +++++++++++++
 proxy/http/HttpTransactHeaders.h                            |  2 ++
 13 files changed, 55 insertions(+), 5 deletions(-)

diff --git a/doc/admin-guide/files/records.config.en.rst b/doc/admin-guide/files/records.config.en.rst
index 84d42cc..4e996bf 100644
--- a/doc/admin-guide/files/records.config.en.rst
+++ b/doc/admin-guide/files/records.config.en.rst
@@ -1057,6 +1057,19 @@ ip-resolve
    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 3c9c3fe..cd3b674 100644
--- a/doc/developer-guide/api/functions/TSHttpOverridableConfig.en.rst
+++ b/doc/developer-guide/api/functions/TSHttpOverridableConfig.en.rst
@@ -73,6 +73,7 @@ c:member:`TS_CONFIG_HTTP_ANONYMIZE_REMOVE_FROM`                     :ts:cv:`prox
 c:member:`TS_CONFIG_HTTP_ANONYMIZE_REMOVE_REFERER`                  :ts:cv:`proxy.config.http.anonymize_remove_referer`
 c:member:`TS_CONFIG_HTTP_ANONYMIZE_REMOVE_USER_AGENT`               :ts:cv:`proxy.config.http.anonymize_remove_user_agent`
 c:member:`TS_CONFIG_HTTP_ATTACH_SERVER_SESSION_TO_CLIENT`           :ts:cv:`proxy.config.http.attach_server_session_to_client`
+c:member:`TS_CONFIG_HTTP_SAFE_REQUESTS_RETRYABLE`                             :ts:cv:`proxy.config.http.safe_requests_retryable`
 c:member:`TS_CONFIG_HTTP_AUTH_SERVER_SESSION_PRIVATE`               :ts:cv:`proxy.config.http.auth_server_session_private`
 c:member:`TS_CONFIG_HTTP_BACKGROUND_FILL_ACTIVE_TIMEOUT`            :ts:cv:`proxy.config.http.background_fill_active_timeout`
 c:member:`TS_CONFIG_HTTP_BACKGROUND_FILL_COMPLETED_THRESHOLD`       :ts:cv:`proxy.config.http.background_fill_completed_threshold`
diff --git a/lib/ts/apidefs.h.in b/lib/ts/apidefs.h.in
index 3b6f58b..dcf6d0e 100644
--- a/lib/ts/apidefs.h.in
+++ b/lib/ts/apidefs.h.in
@@ -734,6 +734,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 3176f13..84875a0 100644
--- a/mgmt/RecordsConfig.cc
+++ b/mgmt/RecordsConfig.cc
@@ -473,6 +473,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 de0ca0e..b3eedd3 100644
--- a/plugins/experimental/ts_lua/ts_lua_http_config.c
+++ b/plugins/experimental/ts_lua/ts_lua_http_config.c
@@ -112,6 +112,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,
@@ -226,6 +227,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 10a3495..c670298 100644
--- a/proxy/InkAPI.cc
+++ b/proxy/InkAPI.cc
@@ -8121,6 +8121,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;
@@ -8602,6 +8606,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':
diff --git a/proxy/InkAPITest.cc b/proxy/InkAPITest.cc
index 5f0120f..6978c05 100644
--- a/proxy/InkAPITest.cc
+++ b/proxy/InkAPITest.cc
@@ -7610,6 +7610,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 9c69e0a..0ff5423 100644
--- a/proxy/http/HttpConfig.cc
+++ b/proxy/http/HttpConfig.cc
@@ -898,6 +898,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");
   HttpEstablishStaticConfigByte(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");
 
   HttpEstablishStaticConfigByte(c.disable_ssl_parenting, "proxy.local.http.parent_proxy.disable_connect_tunneling");
   HttpEstablishStaticConfigByte(c.oride.forward_connect_method, "proxy.config.http.forward_connect_method");
@@ -1181,6 +1182,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 cc75710..479e9f4 100644
--- a/proxy/http/HttpConfig.h
+++ b/proxy/http/HttpConfig.h
@@ -379,6 +379,7 @@ struct OverridableHttpConfigParams {
       fwd_proxy_auth_to_parent(0),
       uncacheable_requests_bypass_parent(1),
       attach_server_session_to_client(0),
+      safe_requests_retryable(1),
       forward_connect_method(0),
       insert_age_in_response(1),
       anonymize_remove_from(0),
@@ -499,6 +500,8 @@ struct OverridableHttpConfigParams {
   MgmtByte uncacheable_requests_bypass_parent;
   MgmtByte attach_server_session_to_client;
 
+  MgmtByte safe_requests_retryable;
+
   MgmtByte forward_connect_method;
 
   MgmtByte insert_age_in_response;
diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc
index b5489d3..236dfd4 100644
--- a/proxy/http/HttpSM.cc
+++ b/proxy/http/HttpSM.cc
@@ -24,6 +24,7 @@
 
 #include "../ProxyClientTransaction.h"
 #include "HttpSM.h"
+#include "HttpTransactHeaders.h"
 #include "ProxyConfig.h"
 #include "HttpServerSession.h"
 #include "HttpDebugNames.h"
@@ -5033,10 +5034,10 @@ HttpSM::do_http_server_open(bool raw)
     }
   }
 
-  // draft-stenberg-httpbis-tcp recommends only enabling TFO on safe, indempotent methods
or
+  // draft-stenberg-httpbis-tcp recommends only enabling TFO on indempotent methods or
   // those with intervening protocol layers (eg. TLS).
-  if (scheme_to_use == URL_WKSIDX_HTTPS || t_state.method == HTTP_WKSIDX_CONNECT || t_state.method
== HTTP_WKSIDX_DELETE ||
-      t_state.method == HTTP_WKSIDX_GET || t_state.method == HTTP_WKSIDX_HEAD || t_state.method
== HTTP_WKSIDX_PUT) {
+
+  if (scheme_to_use == URL_WKSIDX_HTTPS || HttpTransactHeaders::is_method_idempotent(t_state.method))
{
     opt.f_tcp_fastopen = (t_state.txn_conf->sock_option_flag_out & NetVCOptions::SOCK_OPT_TCP_FAST_OPEN);
   }
 
diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc
index 48273c9..8309b07 100644
--- a/proxy/http/HttpTransact.cc
+++ b/proxy/http/HttpTransact.cc
@@ -6487,9 +6487,12 @@ 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 &&
-      s->state_machine->get_server_session()->get_netvc()->outstanding() != s->state_machine->server_request_hdr_bytes)
{
+  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 &&
+       s->state_machine->get_server_session()->get_netvc()->outstanding() !=
s->state_machine->server_request_hdr_bytes)) {
     return false;
   }
 
diff --git a/proxy/http/HttpTransactHeaders.cc b/proxy/http/HttpTransactHeaders.cc
index 7046f95..d16516f 100644
--- a/proxy/http/HttpTransactHeaders.cc
+++ b/proxy/http/HttpTransactHeaders.cc
@@ -74,6 +74,19 @@ HttpTransactHeaders::is_this_method_supported(int the_scheme, int the_method)
   }
 }
 
+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 f6719ab..c27a6a3 100644
--- a/proxy/http/HttpTransactHeaders.h
+++ b/proxy/http/HttpTransactHeaders.h
@@ -57,6 +57,8 @@ public:
                                            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);
 

-- 
To stop receiving notification emails like this one, please contact
['"commits@trafficserver.apache.org" <commits@trafficserver.apache.org>'].

Mime
View raw message