mod_proxy: Add a per-worker maxbusy limit on concurrent requests, glo…

changes-entries/proxy-worker-maxbusy.txt

@@ -0,0 +1,4 @@
+  *) mod_proxy: Add a "maxbusy" BalancerMember parameter, a hard limit on the
+     number of requests concurrently active against a worker across all
+     child processes, distinct from the per-child connection pool size
+     controlled by "max".  [Arturo Bernal]

docs/log-message-tags/next-number

@@ -1 +1 @@
-10636
+10642

docs/manual/mod/mod_proxy.xml

@@ -1147,6 +1147,34 @@ ProxyPass "/example" "http://backend.example.com" max=20 ttl=120 retry=300
     per process in the active MPM. In the Prefork MPM, this is always 1;
     while with other MPMs, it is controlled by the
     <directive>ThreadsPerChild</directive> directive.</td></tr>
+    <tr><td>maxbusy</td>
+        <td>0</td>
+        <td>Maximum number of requests that may be concurrently active
+    against this <directive module="mod_proxy_balancer">BalancerMember</directive>,
+    counted across all child processes of this <code>httpd</code>
+    instance. Unlike <code>max</code>, which bounds the size of the
+    per-child connection pool, <code>maxbusy</code> is a single global
+    limit on in-flight requests, and only applies to a balancer member.
+    While a member is at its <code>maxbusy</code> limit, the balancer
+    treats it as temporarily unavailable and selects another member with
+    spare capacity instead, the same way it already does for a member
+    that is disabled or in an error state; the balancer's own
+    <code>timeout</code> parameter (set on the
+    <directive module="mod_proxy_balancer">Proxy balancer://...</directive>
+    block, not on an individual member, and unrelated to a member's own
+    backend connection timeout) applies if no member currently has
+    capacity. A request only fails with
+    <code>HTTP_SERVICE_UNAVAILABLE</code> if no member becomes available
+    before that balancer timeout elapses, or immediately if none is
+    configured. The default, 0, means no limit. With
+    <directive module="mod_proxy">BalancerPersist</directive> on, a worker's
+    persisted shared state survives a restart as-is; changing only
+    <code>maxbusy</code> in the configuration does not replace that
+    persisted value, the same as for other worker parameters such as
+    <code>loadfactor</code> or <code>smax</code>. A full stop and start is
+    required for a configuration-only <code>maxbusy</code> change to take
+    effect in that case.
+    </td></tr>
     <tr><td>smax</td>
         <td>max</td>
         <td>Retained connection pool entries above this limit are freed

include/ap_mmn.h

@@ -739,14 +739,22 @@
  * 20211221.31 (2.5.1-dev) Add ap_*_timingsafe() to httpd.h
  * 20211221.32 (2.5.1-dev) Add ap_cstr_stripws() to httpd.h
  * 20211221.33 (2.5.1-dev) Add ap_size_parameter_parse() to httpd.h
+ * 20211221.34 (2.5.1-dev) Add maxbusy to proxy_worker_shared, and
+ *                          ap_proxy_admission_table_init(),
+ *                          ap_proxy_admission_table_check(),
+ *                          ap_proxy_get_reserved_count(),
+ *                          ap_proxy_worker_has_capacity(),
+ *                          ap_proxy_try_reserve_worker() and
+ *                          ap_proxy_release_worker_reservation() to
+ *                          proxy_util.h
  */
 
 #define MODULE_MAGIC_COOKIE 0x41503235UL /* "AP25" */
 
 #ifndef MODULE_MAGIC_NUMBER_MAJOR
 #define MODULE_MAGIC_NUMBER_MAJOR 20211221
 #endif
-#define MODULE_MAGIC_NUMBER_MINOR 33             /* 0...n */
+#define MODULE_MAGIC_NUMBER_MINOR 34             /* 0...n */
 
 /**
  * Determine if the server's current MODULE_MAGIC_NUMBER is at least a

modules/proxy/balancers/mod_lbmethod_heartbeat.c

@@ -15,6 +15,7 @@
  */
 
 #include "mod_proxy.h"
+#include "proxy_util.h"
 #include "scoreboard.h"
 #include "ap_mpm.h"
 #include "apr_version.h"
@@ -343,7 +344,7 @@ static proxy_worker *find_best_hb(proxy_balancer *balancer,
             ap_proxy_retry_worker_fn("BALANCER", *worker, r->server);
         }
 
-        if (PROXY_WORKER_IS_USABLE(*worker)) {
+        if (PROXY_WORKER_IS_USABLE(*worker) && ap_proxy_worker_has_capacity(*worker)) {
             server->worker = *worker;
             if (server->seen < LBM_HEARTBEAT_MAX_LASTSEEN) {
                 apr_uint32_t ready = (apr_uint32_t)server->ready;

modules/proxy/mod_proxy.c

@@ -161,6 +161,29 @@ static const char *set_worker_param(apr_pool_t *p,
             return "Smax must be a positive number";
         worker->s->smax = ival;
     }
+    else if (!strcasecmp(key, "maxbusy")) {
+        /* Hard limit on the number of requests concurrently active
+         * against this worker, across all children. 0 (the default)
+         * means no limit. Only meaningful for a BalancerMember: the
+         * admission table it is checked against is only consulted from
+         * mod_proxy_balancer, see ap_proxy_try_reserve_worker().
+         */
+        apr_off_t off;
+
+        if (!worker->balancer) {
+            return "MaxBusy is only valid for a BalancerMember";
+        }
+        if (ap_size_parameter_parse(val, &off) != APR_SUCCESS || off < 0) {
+            return "MaxBusy must be a non-negative integer";
+        }
+        /* The live admission counter is a 32-bit atomic (see
+         * ap_proxy_try_reserve_worker()); reject what it cannot hold
+         * rather than silently truncate it. */
+        if (off > APR_UINT32_MAX) {
+            return "MaxBusy must not exceed 4294967295";
+        }
+        worker->s->maxbusy = (apr_size_t)off;
+    }
     else if (!strcasecmp(key, "acquire")) {
         /* Acquire timeout in given unit (default is milliseconds).
          * If set this will be the maximum time to
@@ -1384,6 +1407,26 @@ static int proxy_handler(request_rec *r)
 
     do {
         char *url = uri;
+
+        if (worker) {
+            /* Entering this iteration at all means the previous one left
+             * the worker unusable and attempts remain (see the loop
+             * condition below): a failover to a different worker is
+             * about to be attempted via the next ap_proxy_pre_request()
+             * call. The worker being abandoned is no longer handling
+             * this proxied request, so release its maxbusy admission
+             * reservation now rather than leaving it counted as busy for
+             * as long as the eventual successful attempt takes to
+             * complete. A no-op if it held no reservation (maxbusy == 0,
+             * or none could be obtained). This does not touch
+             * worker->s->busy (the bybusyness load-balancing metric),
+             * which keeps its original, unconditional request-pool
+             * lifetime.
+             */
+            apr_pool_cleanup_run(r->pool, worker,
+                                 ap_proxy_release_worker_reservation);
+        }
+
         /* Try to obtain the most suitable worker */
         access_status = ap_proxy_pre_request(&worker, &balancer, r, conf, &url);
         if (access_status != OK) {
@@ -3172,6 +3215,12 @@ static int proxy_post_config(apr_pool_t *pconf, apr_pool_t *plog,
     ap_proxy_strmatch_path = apr_strmatch_precompile(pconf, "path=", 0);
     ap_proxy_strmatch_domain = apr_strmatch_precompile(pconf, "domain=", 0);
 
+    /* Create (or, after the first restart in this process's lifetime,
+     * reuse) the maxbusy admission table before any child is forked;
+     * see the function's own comment for why this must outlive a
+     * graceful restart. */
+    ap_proxy_admission_table_init(main_s);
+
     for (; s; s = s->next) {
         int rc, i;
         proxy_server_conf *sconf =
@@ -3188,6 +3237,13 @@ static int proxy_post_config(apr_pool_t *pconf, apr_pool_t *plog,
         }
     }
 
+    /* Pre-registering every maxbusy BalancerMember's admission slot (so
+     * the request path only ever looks one up, never creates one) needs
+     * balancer->s->sname, which is not final until mod_proxy_balancer's
+     * own post_config runs -- see ap_proxy_admission_table_check()'s own
+     * comment. It is called from there, not here.
+     */
+
     return OK;
 }
 

modules/proxy/mod_proxy.h

@@ -472,7 +472,15 @@ typedef struct {
     apr_size_t      recv_buffer_size;
     apr_size_t      io_buffer_size;
     apr_size_t      elected;    /* Number of times the worker was elected */
-    apr_size_t      busy;       /* busyness factor */
+    apr_size_t      busy;       /* busyness factor; a load-balancing metric
+                                 * only, reset by some lbmethods' reset() */
+    apr_size_t      maxbusy;    /* Hard limit on concurrently active requests
+                                 * to this worker, across all children; 0
+                                 * (the default) means no limit. This is
+                                 * configuration, not live state: the live
+                                 * admission count it is checked against
+                                 * lives outside this per-generation shared
+                                 * memory, see ap_proxy_try_reserve_worker(). */
     apr_size_t      response_field_size; /* Size of proxy response buffer in bytes. */
     apr_port_t      port;
     apr_off_t       transferred;/* Number of bytes transferred to remote */