Merge pull request #1235 from nghttp2/backend-conn-timeout

nghttpx: Add read/write-timeout parameters to backend option

src/shrpx.cc

@@ -1736,11 +1736,13 @@ Connections:
               parameters       are:      "proto=<PROTO>",       "tls",
               "sni=<SNI_HOST>",         "fall=<N>",        "rise=<N>",
               "affinity=<METHOD>",    "dns",    "redirect-if-not-tls",
-              "upgrade-scheme",  and  "mruby=<PATH>".   The  parameter
+              "upgrade-scheme",                        "mruby=<PATH>",
-              consists of keyword, and  optionally followed by "=" and
+              "read-timeout=<DURATION>",                           and
-              value.  For  example, the parameter  "proto=h2" consists
+              "write-timeout=<DURATION>".   The parameter  consists of
-              of the  keyword "proto"  and value "h2".   The parameter
+              keyword, and optionally followed  by "=" and value.  For
-              "tls" consists of the keyword "tls" without value.  Each
+              example,  the  parameter   "proto=h2"  consists  of  the
+              keyword  "proto" and  value "h2".   The parameter  "tls"
+              consists  of  the  keyword "tls"  without  value.   Each
               parameter is described as follows.
 
               The backend application protocol  can be specified using
@@ -1839,6 +1841,14 @@ Connections:
               matched.  All backends which share the same pattern must
               have the same mruby path.
 
+              "read-timeout=<DURATION>" and "write-timeout=<DURATION>"
+              parameters  specify the  read and  write timeout  of the
+              backend connection  when this  pattern is  matched.  All
+              backends which share the same pattern must have the same
+              timeouts.  If these timeouts  are entirely omitted for a
+              pattern,            --backend-read-timeout           and
+              --backend-write-timeout are used.
+
               Since ";" and ":" are  used as delimiter, <PATTERN> must
               not  contain these  characters.  Since  ";" has  special
               meaning in shell, the option value must be quoted.

src/shrpx_config.cc

@@ -812,6 +812,8 @@ struct DownstreamParams {
   StringRef sni;
   StringRef mruby;
   AffinityConfig affinity;
+  ev_tstamp read_timeout;
+  ev_tstamp write_timeout;
   size_t fall;
   size_t rise;
   shrpx_proto proto;
@@ -821,6 +823,22 @@ struct DownstreamParams {
   bool upgrade_scheme;
 };
 
+namespace {
+// Parses |value| of parameter named |name| as duration.  This
+// function returns 0 if it succeeds and the parsed value is assigned
+// to |dest|, or -1.
+int parse_downstream_param_duration(ev_tstamp &dest, const StringRef &name,
+                                    const StringRef &value) {
+  auto t = util::parse_duration_with_unit(value);
+  if (t == std::numeric_limits<double>::infinity()) {
+    LOG(ERROR) << "backend: " << name << ": bad value: '" << value << "'";
+    return -1;
+  }
+  dest = t;
+  return 0;
+}
+} // namespace
+
 namespace {
 // Parses downstream configuration parameter |src_params|, and stores
 // parsed results into |out|.  This function returns 0 if it succeeds,
@@ -928,6 +946,18 @@ int parse_downstream_params(DownstreamParams &out,
     } else if (util::istarts_with_l(param, "mruby=")) {
       auto valstr = StringRef{first + str_size("mruby="), end};
       out.mruby = valstr;
+    } else if (util::istarts_with_l(param, "read-timeout=")) {
+      if (parse_downstream_param_duration(
+              out.read_timeout, StringRef::from_lit("read-timeout"),
+              StringRef{first + str_size("read-timeout="), end}) == -1) {
+        return -1;
+      }
+    } else if (util::istarts_with_l(param, "write-timeout=")) {
+      if (parse_downstream_param_duration(
+              out.write_timeout, StringRef::from_lit("write-timeout"),
+              StringRef{first + str_size("write-timeout="), end}) == -1) {
+        return -1;
+      }
     } else if (!param.empty()) {
       LOG(ERROR) << "backend: " << param << ": unknown keyword";
       return -1;
@@ -1065,6 +1095,29 @@ int parse_mapping(Config *config, DownstreamAddrConfig &addr,
           return -1;
         }
       }
+      // All backends in the same group must have the same read/write
+      // timeout.  If some backends do not specify read/write timeout,
+      // and there is at least one backend with read/write timeout, it
+      // is used for all backends in the group.
+      if (params.read_timeout > 1e-9) {
+        if (g.timeout.read < 1e-9) {
+          g.timeout.read = params.read_timeout;
+        } else if (fabs(g.timeout.read - params.read_timeout) > 1e-9) {
+          LOG(ERROR)
+              << "backend: read-timeout: multiple different read-timeout "
+                 "found in a single group";
+          return -1;
+        }
+      }
+      if (params.write_timeout > 1e-9) {
+        if (g.timeout.write < 1e-9) {
+          g.timeout.write = params.write_timeout;
+        } else if (fabs(g.timeout.write - params.write_timeout) > 1e-9) {
+          LOG(ERROR) << "backend: write-timeout: multiple different "
+                        "write-timeout found in a single group";
+          return -1;
+        }
+      }
 
       g.addrs.push_back(addr);
       continue;
@@ -1087,6 +1140,8 @@ int parse_mapping(Config *config, DownstreamAddrConfig &addr,
     }
     g.redirect_if_not_tls = params.redirect_if_not_tls;
     g.mruby_file = make_string_ref(downstreamconf.balloc, params.mruby);
+    g.timeout.read = params.read_timeout;
+    g.timeout.write = params.write_timeout;
 
     if (pattern[0] == '*') {
       // wildcard pattern
@@ -4048,6 +4103,14 @@ int configure_downstream_group(Config *config, bool http2_proxy,
                   return lhs.hash < rhs.hash;
                 });
     }
+
+    auto &timeout = g.timeout;
+    if (timeout.read < 1e-9) {
+      timeout.read = downstreamconf.timeout.read;
+    }
+    if (timeout.write < 1e-9) {
+      timeout.write = downstreamconf.timeout.write;
+    }
   }
 
   return 0;

src/shrpx_config.h

@@ -504,6 +504,11 @@ struct DownstreamAddrGroupConfig {
   // true if this group requires that client connection must be TLS,
   // and the request must be redirected to https URI.
   bool redirect_if_not_tls;
+  // Timeouts for backend connection.
+  struct {
+    ev_tstamp read;
+    ev_tstamp write;
+  } timeout;
 };
 
 struct TicketKey {

src/shrpx_http2_session.cc

@@ -186,9 +186,9 @@ Http2Session::Http2Session(struct ev_loop *loop, SSL_CTX *ssl_ctx,
     : dlnext(nullptr),
       dlprev(nullptr),
       conn_(loop, -1, nullptr, worker->get_mcpool(),
-            worker->get_downstream_config()->timeout.write,
+            group->shared_addr->timeout.write, group->shared_addr->timeout.read,
-            worker->get_downstream_config()->timeout.read, {}, {}, writecb,
+            {}, {}, writecb, readcb, timeoutcb, this,
-            readcb, timeoutcb, this, get_config()->tls.dyn_rec.warmup_threshold,
+            get_config()->tls.dyn_rec.warmup_threshold,
             get_config()->tls.dyn_rec.idle_timeout, PROTO_HTTP2),
       wb_(worker->get_mcpool()),
       worker_(worker),
@@ -1972,10 +1972,8 @@ int Http2Session::connected() {
     SSLOG(INFO, this) << "Connection established";
   }
 
-  auto &downstreamconf = *get_config()->conn.downstream;
-
   // Reset timeout for write.  Previously, we set timeout for connect.
-  conn_.wt.repeat = downstreamconf.timeout.write;
+  conn_.wt.repeat = group_->shared_addr->timeout.write;
   ev_timer_again(conn_.loop, &conn_.wt);
 
   conn_.rlimit.startw();

src/shrpx_http_downstream_connection.cc

@@ -190,9 +190,8 @@ HttpDownstreamConnection::HttpDownstreamConnection(
     const std::shared_ptr<DownstreamAddrGroup> &group, size_t initial_addr_idx,
     struct ev_loop *loop, Worker *worker)
     : conn_(loop, -1, nullptr, worker->get_mcpool(),
-            worker->get_downstream_config()->timeout.write,
+            group->shared_addr->timeout.write, group->shared_addr->timeout.read,
-            worker->get_downstream_config()->timeout.read, {}, {}, connectcb,
+            {}, {}, connectcb, readcb, connect_timeoutcb, this,
-            readcb, connect_timeoutcb, this,
             get_config()->tls.dyn_rec.warmup_threshold,
             get_config()->tls.dyn_rec.idle_timeout, PROTO_HTTP1),
       on_read_(&HttpDownstreamConnection::noop),
@@ -459,11 +458,11 @@ int HttpDownstreamConnection::initiate_connection() {
   } else {
     // we may set read timer cb to idle_timeoutcb.  Reset again.
     ev_set_cb(&conn_.rt, timeoutcb);
-    if (conn_.read_timeout < downstreamconf.timeout.read) {
+    if (conn_.read_timeout < group_->shared_addr->timeout.read) {
-      conn_.read_timeout = downstreamconf.timeout.read;
+      conn_.read_timeout = group_->shared_addr->timeout.read;
       conn_.last_read = ev_now(conn_.loop);
     } else {
-      conn_.again_rt(downstreamconf.timeout.read);
+      conn_.again_rt(group_->shared_addr->timeout.read);
     }
 
     ev_set_cb(&conn_.rev, readcb);
@@ -1489,10 +1488,8 @@ int HttpDownstreamConnection::connected() {
     DCLOG(INFO, this) << "Connected to downstream host";
   }
 
-  auto &downstreamconf = *get_config()->conn.downstream;
-
   // Reset timeout for write.  Previously, we set timeout for connect.
-  conn_.wt.repeat = downstreamconf.timeout.write;
+  conn_.wt.repeat = group_->shared_addr->timeout.write;
   ev_timer_again(conn_.loop, &conn_.wt);
 
   conn_.rlimit.startw();

src/shrpx_worker.cc

@@ -77,7 +77,7 @@ DownstreamAddrGroup::~DownstreamAddrGroup() {}
 using DownstreamKey = std::tuple<
     std::vector<std::tuple<StringRef, StringRef, size_t, size_t, shrpx_proto,
                            uint16_t, bool, bool, bool, bool>>,
-    bool, int, StringRef, StringRef, int>;
+    bool, int, StringRef, StringRef, int, int64_t, int64_t>;
 
 namespace {
 DownstreamKey create_downstream_key(
@@ -109,6 +109,9 @@ DownstreamKey create_downstream_key(
   std::get<3>(dkey) = affinity.cookie.name;
   std::get<4>(dkey) = affinity.cookie.path;
   std::get<5>(dkey) = affinity.cookie.secure;
+  auto &timeout = shared_addr->timeout;
+  std::get<6>(dkey) = timeout.read;
+  std::get<7>(dkey) = timeout.write;
 
   return dkey;
 }
@@ -221,6 +224,8 @@ void Worker::replace_downstream_config(
     }
     shared_addr->affinity_hash = src.affinity_hash;
     shared_addr->redirect_if_not_tls = src.redirect_if_not_tls;
+    shared_addr->timeout.read = src.timeout.read;
+    shared_addr->timeout.write = src.timeout.write;
 
     size_t num_http1 = 0;
     size_t num_http2 = 0;

src/shrpx_worker.h

@@ -180,6 +180,11 @@ struct SharedDownstreamAddr {
   // true if this group requires that client connection must be TLS,
   // and the request must be redirected to https URI.
   bool redirect_if_not_tls;
+  // Timeouts for backend connection.
+  struct {
+    ev_tstamp read;
+    ev_tstamp write;
+  } timeout;
 };
 
 struct DownstreamAddrGroup {