From aeb92bbbe29a552d2c68236c64a8176a0dcdbd8b Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 30 Sep 2018 12:23:34 +0900 Subject: [PATCH] nghttpx: Add read/write-timeout parameters to backend option --- src/shrpx.cc | 20 ++++++-- src/shrpx_config.cc | 63 +++++++++++++++++++++++++ src/shrpx_config.h | 5 ++ src/shrpx_http2_session.cc | 10 ++-- src/shrpx_http_downstream_connection.cc | 15 +++--- src/shrpx_worker.cc | 7 ++- src/shrpx_worker.h | 5 ++ 7 files changed, 104 insertions(+), 21 deletions(-) diff --git a/src/shrpx.cc b/src/shrpx.cc index 89d7f893..5809deba 100644 --- a/src/shrpx.cc +++ b/src/shrpx.cc @@ -1736,11 +1736,13 @@ Connections: parameters are: "proto=", "tls", "sni=", "fall=", "rise=", "affinity=", "dns", "redirect-if-not-tls", - "upgrade-scheme", and "mruby=". The parameter - consists of keyword, and optionally followed by "=" and - value. For example, the parameter "proto=h2" consists - of the keyword "proto" and value "h2". The parameter - "tls" consists of the keyword "tls" without value. Each + "upgrade-scheme", "mruby=", + "read-timeout=", and + "write-timeout=". The parameter consists of + keyword, and optionally followed by "=" and value. For + 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=" and "write-timeout=" + 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, must not contain these characters. Since ";" has special meaning in shell, the option value must be quoted. diff --git a/src/shrpx_config.cc b/src/shrpx_config.cc index 27f376a2..b79a7996 100644 --- a/src/shrpx_config.cc +++ b/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::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; diff --git a/src/shrpx_config.h b/src/shrpx_config.h index 7e023451..3b60e77f 100644 --- a/src/shrpx_config.h +++ b/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 { diff --git a/src/shrpx_http2_session.cc b/src/shrpx_http2_session.cc index 8c4ece3c..b48a93c6 100644 --- a/src/shrpx_http2_session.cc +++ b/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, - worker->get_downstream_config()->timeout.read, {}, {}, writecb, - readcb, timeoutcb, this, get_config()->tls.dyn_rec.warmup_threshold, + group->shared_addr->timeout.write, group->shared_addr->timeout.read, + {}, {}, writecb, readcb, timeoutcb, this, + 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(); diff --git a/src/shrpx_http_downstream_connection.cc b/src/shrpx_http_downstream_connection.cc index 9fd3c124..b93c8e80 100644 --- a/src/shrpx_http_downstream_connection.cc +++ b/src/shrpx_http_downstream_connection.cc @@ -190,9 +190,8 @@ HttpDownstreamConnection::HttpDownstreamConnection( const std::shared_ptr &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, - worker->get_downstream_config()->timeout.read, {}, {}, connectcb, - readcb, connect_timeoutcb, this, + group->shared_addr->timeout.write, group->shared_addr->timeout.read, + {}, {}, connectcb, 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) { - conn_.read_timeout = downstreamconf.timeout.read; + if (conn_.read_timeout < group_->shared_addr->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(); diff --git a/src/shrpx_worker.cc b/src/shrpx_worker.cc index 98201e81..05caa198 100644 --- a/src/shrpx_worker.cc +++ b/src/shrpx_worker.cc @@ -77,7 +77,7 @@ DownstreamAddrGroup::~DownstreamAddrGroup() {} using DownstreamKey = std::tuple< std::vector>, - 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; diff --git a/src/shrpx_worker.h b/src/shrpx_worker.h index e6deaa58..cc8c2a03 100644 --- a/src/shrpx_worker.h +++ b/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 {