nghttpx: Move fall/rise configuration to --backend option

This commit removes --backend-fall and --backend-rise options.  The
these configurations are now set as fall and rise parameters in
--backend option.

gennghttpxfun.py

@@ -129,8 +129,6 @@ OPTIONS = [
     "backend-tls",
     "backend-connections-per-host",
     "error-page",
-    "backend-fall",
-    "backend-rise",
 ]
 
 LOGVARS = [

src/shrpx.cc

@@ -1198,7 +1198,7 @@ Options:
   The options are categorized into several groups.
 
 Connections:
-  -b, --backend=(<HOST>,<PORT>|unix:<PATH>)[;[<PATTERN>[:...]][;proto=<PROTO>][;tls]]
+  -b, --backend=(<HOST>,<PORT>|unix:<PATH>)[;[<PATTERN>[:...]][;proto=<PROTO>][;tls][;fall=<N>][;rise=<N>]]
               Set  backend  host  and   port.   The  multiple  backend
               addresses are  accepted by repeating this  option.  UNIX
               domain socket  can be  specified by prefixing  path name
@@ -1272,6 +1272,23 @@ Connections:
               Optionally,  TLS  can  be enabled  by  specifying  "tls"
               keyword.  TLS is not enabled by default.
 
+              Optionally,  the feature  to detect  whether backend  is
+              online/offline can  be enabled  using "fall"  and "rise"
+              parameters.   Using  "fall=<N>"  parameter,  if  nghttpx
+              cannot connect  to a  this backend <N>  times in  a row,
+              this  backend  is  assumed  to be  offline,  and  it  is
+              excluded from load balancing.  If <N> is 0, this backend
+              never  be excluded  from load  balancing whatever  times
+              nghttpx cannot connect  to it, and this  is the default.
+              There is  also "rise=<N>" parameter.  After  backend was
+              excluded from load balancing group, nghttpx periodically
+              attempts to make a connection to the failed backend, and
+              if the  connection is made  successfully <N> times  in a
+              row, the backend is assumed to  be online, and it is now
+              eligible  for load  balancing target.   If <N>  is 0,  a
+              backend  is permanently  offline, once  it goes  in that
+              state, and this is the default behaviour.
+
               Since ";" and ":" are  used as delimiter, <PATTERN> must
               not  contain these  characters.  Since  ";" has  special
               meaning in shell, the option value must be quoted.
@@ -1314,24 +1331,6 @@ Connections:
               --backend-write-timeout options.
   --accept-proxy-protocol
               Accept PROXY protocol version 1 on frontend connection.
-  --backend-fall=<N>
-              If  nghttpx cannot  connect  to a  specific backend  <N>
-              times in a  row, that backend is assumed  to be offline,
-              and  it  is  excluded  from load  balancing.   See  also
-              --backend-rise option.  If <N> is  0, a backend never be
-              excluded  from  load  balancing whatever  times  nghttpx
-              cannot connect to it.
-              Default: )" << get_config()->conn.downstream.fall << R"(
-  --backend-rise=<N>
-              As described  in --backend-fall,  a backend  is excluded
-              from  load  balancing  if  nghttpx assumes  that  it  is
-              offline.  Then  nghttpx periodically attempts to  make a
-              connection to the failed  backend, and if the connection
-              is made successfully <N> times  in a row, the backend is
-              assumed to  be online, and  it is now eligible  for load
-              balancing target.  If <N> is 0, a backend is permanently
-              offline, once it goes in that state.
-              Default: )" << get_config()->conn.downstream.rise << R"(
 
 Performance:
   -n, --workers=<N>
@@ -2537,8 +2536,6 @@ int main(int argc, char **argv) {
         {SHRPX_OPT_BACKEND_CONNECTIONS_PER_HOST.c_str(), required_argument,
          &flag, 121},
         {SHRPX_OPT_ERROR_PAGE.c_str(), required_argument, &flag, 122},
-        {SHRPX_OPT_BACKEND_FALL.c_str(), required_argument, &flag, 123},
-        {SHRPX_OPT_BACKEND_RISE.c_str(), required_argument, &flag, 124},
         {nullptr, 0, nullptr, 0}};
 
     int option_index = 0;
@@ -3114,14 +3111,6 @@ int main(int argc, char **argv) {
         // --error-page
         cmdcfgs.emplace_back(SHRPX_OPT_ERROR_PAGE, StringRef{optarg});
         break;
-      case 123:
-        // --backend-fall
-        cmdcfgs.emplace_back(SHRPX_OPT_BACKEND_FALL, StringRef{optarg});
-        break;
-      case 124:
-        // --backend-rise
-        cmdcfgs.emplace_back(SHRPX_OPT_BACKEND_RISE, StringRef{optarg});
-        break;
       default:
         break;
       }

src/shrpx_config.cc

@@ -644,6 +644,8 @@ int parse_upstream_params(UpstreamParams &out, const StringRef &src_params) {
 } // namespace
 
 struct DownstreamParams {
+  size_t fall;
+  size_t rise;
   shrpx_proto proto;
   bool tls;
 };
@@ -675,6 +677,34 @@ int parse_downstream_params(DownstreamParams &out,
         LOG(ERROR) << "backend: proto: unknown protocol " << protostr;
         return -1;
       }
+    } else if (util::istarts_with_l(param, "fall=")) {
+      auto valstr = StringRef{first + str_size("fall="), end};
+      if (valstr.empty()) {
+        LOG(ERROR) << "backend: fall: non-negative integer is expected";
+        return -1;
+      }
+
+      auto n = util::parse_uint(valstr);
+      if (n == -1) {
+        LOG(ERROR) << "backend: fall: non-negative integer is expected";
+        return -1;
+      }
+
+      out.fall = n;
+    } else if (util::istarts_with_l(param, "rise=")) {
+      auto valstr = StringRef{first + str_size("rise="), end};
+      if (valstr.empty()) {
+        LOG(ERROR) << "backend: rise: non-negative integer is expected";
+        return -1;
+      }
+
+      auto n = util::parse_uint(valstr);
+      if (n == -1) {
+        LOG(ERROR) << "backend: rise: non-negative integer is expected";
+        return -1;
+      }
+
+      out.rise = n;
     } else if (util::strieq_l("tls", param)) {
       out.tls = true;
     } else if (util::strieq_l("no-tls", param)) {
@@ -703,8 +733,8 @@ namespace {
 // as catch-all.  We also parse protocol specified in |src_proto|.
 //
 // This function returns 0 if it succeeds, or -1.
-int parse_mapping(const DownstreamAddrConfig &addr,
+int parse_mapping(DownstreamAddrConfig addr, const StringRef &src_pattern,
-                  const StringRef &src_pattern, const StringRef &src_params) {
+                  const StringRef &src_params) {
   // This returns at least 1 element (it could be empty string).  We
   // will append '/' to all patterns, so it becomes catch-all pattern.
   auto mapping = util::split_str(src_pattern, ':');
@@ -718,6 +748,9 @@ int parse_mapping(const DownstreamAddrConfig &addr,
     return -1;
   }
 
+  addr.fall = params.fall;
+  addr.rise = params.rise;
+
   for (const auto &raw_pattern : mapping) {
     auto done = false;
     std::string pattern;
@@ -893,7 +926,6 @@ enum {
   SHRPX_OPTID_BACKEND_ADDRESS_FAMILY,
   SHRPX_OPTID_BACKEND_CONNECTIONS_PER_FRONTEND,
   SHRPX_OPTID_BACKEND_CONNECTIONS_PER_HOST,
-  SHRPX_OPTID_BACKEND_FALL,
   SHRPX_OPTID_BACKEND_HTTP_PROXY_URI,
   SHRPX_OPTID_BACKEND_HTTP1_CONNECTIONS_PER_FRONTEND,
   SHRPX_OPTID_BACKEND_HTTP1_CONNECTIONS_PER_HOST,
@@ -909,7 +941,6 @@ enum {
   SHRPX_OPTID_BACKEND_READ_TIMEOUT,
   SHRPX_OPTID_BACKEND_REQUEST_BUFFER,
   SHRPX_OPTID_BACKEND_RESPONSE_BUFFER,
-  SHRPX_OPTID_BACKEND_RISE,
   SHRPX_OPTID_BACKEND_TLS,
   SHRPX_OPTID_BACKEND_TLS_SNI_FIELD,
   SHRPX_OPTID_BACKEND_WRITE_TIMEOUT,
@@ -1189,9 +1220,6 @@ int option_lookup_token(const char *name, size_t namelen) {
       }
       break;
     case 'e':
-      if (util::strieq_l("backend-ris", name, 11)) {
-        return SHRPX_OPTID_BACKEND_RISE;
-      }
       if (util::strieq_l("host-rewrit", name, 11)) {
         return SHRPX_OPTID_HOST_REWRITE;
       }
@@ -1199,11 +1227,6 @@ int option_lookup_token(const char *name, size_t namelen) {
         return SHRPX_OPTID_HTTP2_BRIDGE;
       }
       break;
-    case 'l':
-      if (util::strieq_l("backend-fal", name, 11)) {
-        return SHRPX_OPTID_BACKEND_FALL;
-      }
-      break;
     case 'y':
       if (util::strieq_l("client-prox", name, 11)) {
         return SHRPX_OPTID_CLIENT_PROXY;
@@ -2682,10 +2705,6 @@ int parse_config(const StringRef &opt, const StringRef &optarg,
                       opt, optarg);
   case SHRPX_OPTID_ERROR_PAGE:
     return parse_error_page(mod_config()->http.error_pages, opt, optarg);
-  case SHRPX_OPTID_BACKEND_FALL:
-    return parse_uint(&mod_config()->conn.downstream.fall, opt, optarg);
-  case SHRPX_OPTID_BACKEND_RISE:
-    return parse_uint(&mod_config()->conn.downstream.rise, opt, optarg);
   case SHRPX_OPTID_CONF:
     LOG(WARN) << "conf: ignored";
 

src/shrpx_config.h

@@ -275,8 +275,6 @@ constexpr auto SHRPX_OPT_BACKEND_TLS = StringRef::from_lit("backend-tls");
 constexpr auto SHRPX_OPT_BACKEND_CONNECTIONS_PER_HOST =
     StringRef::from_lit("backend-connections-per-host");
 constexpr auto SHRPX_OPT_ERROR_PAGE = StringRef::from_lit("error-page");
-constexpr auto SHRPX_OPT_BACKEND_FALL = StringRef::from_lit("backend-fall");
-constexpr auto SHRPX_OPT_BACKEND_RISE = StringRef::from_lit("backend-rise");
 
 constexpr size_t SHRPX_OBFUSCATED_NODE_LENGTH = 8;
 
@@ -337,6 +335,8 @@ struct DownstreamAddrConfig {
   // <HOST>:<PORT>.  This does not treat 80 and 443 specially.  If
   // |host_unix| is true, this is "localhost".
   ImmutableString hostport;
+  size_t fall;
+  size_t rise;
   // backend port.  0 if |host_unix| is true.
   uint16_t port;
   // true if |host| contains UNIX domain socket path.
@@ -617,8 +617,6 @@ struct ConnectionConfig {
     size_t connections_per_frontend;
     size_t request_buffer_size;
     size_t response_buffer_size;
-    size_t fall;
-    size_t rise;
     // Address family of backend connection.  One of either AF_INET,
     // AF_INET6 or AF_UNSPEC.  This is ignored if backend connection
     // is made via Unix domain socket.

src/shrpx_live_check.cc

@@ -340,9 +340,7 @@ void LiveCheck::on_success() {
               << " succeeded " << success_count_ << " time(s) in a row";
   }
 
-  auto &downstreamconf = get_config()->conn.downstream;
+  if (success_count_ < addr_->rise) {
-
-  if (success_count_ < downstreamconf.rise) {
     disconnect();
 
     schedule();

src/shrpx_worker.cc

@@ -82,7 +82,8 @@ bool match_shared_downstream_addr(
       }
 
       auto &b = rhs->addrs[i];
-      if (a.host == b.host && a.port == b.port && a.host_unix == b.host_unix) {
+      if (a.host == b.host && a.port == b.port && a.host_unix == b.host_unix &&
+          a.fall == b.fall && a.rise == b.rise) {
         break;
       }
     }
@@ -159,6 +160,8 @@ Worker::Worker(struct ev_loop *loop, SSL_CTX *sv_ssl_ctx, SSL_CTX *cl_ssl_ctx,
       dst_addr.hostport = src_addr.hostport;
       dst_addr.port = src_addr.port;
       dst_addr.host_unix = src_addr.host_unix;
+      dst_addr.fall = src_addr.fall;
+      dst_addr.rise = src_addr.rise;
 
       dst_addr.connect_blocker = make_unique<ConnectBlocker>(randgen_, loop_);
       dst_addr.live_check = make_unique<LiveCheck>(
@@ -473,21 +476,19 @@ void downstream_failure(DownstreamAddr *addr) {
 
   connect_blocker->on_failure();
 
-  auto fail_count = connect_blocker->get_fail_count();
+  if (addr->fall == 0) {
-
-  auto &downstreamconf = get_config()->conn.downstream;
-
-  if (downstreamconf.fall == 0) {
     return;
   }
 
-  if (fail_count >= downstreamconf.fall) {
+  auto fail_count = connect_blocker->get_fail_count();
+
+  if (fail_count >= addr->fall) {
     LOG(WARN) << "Could not connect to " << util::to_numeric_addr(&addr->addr)
               << " " << fail_count << " times in a row; considered as offline";
 
     connect_blocker->offline();
 
-    if (downstreamconf.rise) {
+    if (addr->rise) {
       addr->live_check->schedule();
     }
   }

src/shrpx_worker.h

@@ -81,6 +81,8 @@ struct DownstreamAddr {
 
   std::unique_ptr<ConnectBlocker> connect_blocker;
   std::unique_ptr<LiveCheck> live_check;
+  size_t fall;
+  size_t rise;
   // Client side TLS session cache
   TLSSessionCache tls_session_cache;
   // Http2Session object created for this address.  This list chains