Merge r1822849, r1822858, r1822878, r1822879, r1822883, r1828485, r1828493 from trunk:

* When mod_http2 is loaded more then ThreadsPerChild backend connections can
  be useful as mod_http2 has an additional thread pool on top of
  ThreadsPerChild.
  But leave the default with ThreadsPerChild.

* Add some some comment why we do not limit hmax any longer

mod_proxy: follow up to r1822849.

Get the help(er) of mod_http2 to determine how much connections should
be handled in the reslist by default (i.e. max_threads).

mod_proxy: follow up to r1822849 and r1822878.

Does r1822878's "static" APR_RETRIEVE_OPTIONAL_FN work if, say, mod_proxy is
builtin but mod_http2 isn't?

Not worth taking the risk here since it's not a fast path...

Note: if this is an issue, I'm afraid it applies elsewhere too.

mod_proxy: follow up to r1822849 and r1822879.

Fix my maths, thanks Stefan and Rüdiger!

needs mod_http2.h

* Add missing CHANGES entry for revisions 1822849,1822858,1822878,1822879,1822883,1828485

Submitted by: rpluem, ylavic, ylavic, ylavic, gsmith, rpluem
Reviewed by: rpluem, jim, ylavic

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1832485 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/CHANGES b/CHANGES
index c55dccc09811d461081e3df235564e1369f6be8d..9468c3ded7f280f7df4c8accc977172657236b43 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,11 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.4.34
 
+  *) mod_proxy: Do not restrict the maximum pool size for backend connections
+     any longer by the maximum number of threads per process and use a better
+     default if mod_http2 is loaded.
+     [Yann Ylavic, Ruediger Pluem, Stefan Eissing, Gregg Smith]
+
   *) core: Preserve the original HTTP request method in the '%<m' LogFormat
      when an path-based ErrorDocument is used.  PR 62186. 
      [Micha Lenk <micha lenk.info>]

diff --git a/modules/http2/h2_config.c b/modules/http2/h2_config.c
index 0f495c75002698634e7e0546618345ff2440a459..876635509c556de3a784670fbaf1ef02a503f611 100644 (file)
--- a/modules/http2/h2_config.c
+++ b/modules/http2/h2_config.c
@@ -604,6 +604,30 @@ static const char *h2_conf_set_early_hints(cmd_parms *parms,
     return "value must be On or Off";
 }
 
+void h2_get_num_workers(server_rec *s, int *minw, int *maxw)
+{
+    int threads_per_child = 0;
+    const h2_config *config = h2_config_sget(s);
+
+    *minw = h2_config_geti(config, H2_CONF_MIN_WORKERS);
+    *maxw = h2_config_geti(config, H2_CONF_MAX_WORKERS);    
+    ap_mpm_query(AP_MPMQ_MAX_THREADS, &threads_per_child);
+
+    if (*minw <= 0) {
+        *minw = threads_per_child;
+    }
+    if (*maxw <= 0) {
+        /* As a default, this seems to work quite well under mpm_event. 
+         * For people enabling http2 under mpm_prefork, start 4 threads unless 
+         * configured otherwise. People get unhappy if their http2 requests are 
+         * blocking each other. */
+        *maxw = 3 * (*minw) / 2;
+        if (*maxw < 4) {
+            *maxw = 4;
+        }
+    }
+}
+
 #define AP_END_CMD     AP_INIT_TAKE1(NULL, NULL, NULL, RSRC_CONF, NULL)
 
 const command_rec h2_cmds[] = {

diff --git a/modules/http2/h2_config.h b/modules/http2/h2_config.h
index 54f74dd40ad08fa226db65c5ef6cb9af7bbb9f7c..17d75d6035a135c5989d527c4f705c9949bdae76 100644 (file)
--- a/modules/http2/h2_config.h
+++ b/modules/http2/h2_config.h
@@ -95,6 +95,8 @@ const h2_config *h2_config_rget(request_rec *r);
 int h2_config_geti(const h2_config *conf, h2_config_var_t var);
 apr_int64_t h2_config_geti64(const h2_config *conf, h2_config_var_t var);
 
+void h2_get_num_workers(server_rec *s, int *minw, int *maxw);
+
 void h2_config_init(apr_pool_t *pool);
 
 const struct h2_priority *h2_config_get_priority(const h2_config *conf, 

diff --git a/modules/http2/h2_conn.c b/modules/http2/h2_conn.c
index ed07c7d2bc0ac28ac42fd90ffb22473491673f12..11f54da2582d07a7a139d4d6c039864a84a80233 100644 (file)
--- a/modules/http2/h2_conn.c
+++ b/modules/http2/h2_conn.c
@@ -127,18 +127,7 @@ apr_status_t h2_conn_child_init(apr_pool_t *pool, server_rec *s)
 
     h2_config_init(pool);
     
-    minw = h2_config_geti(config, H2_CONF_MIN_WORKERS);
-    maxw = h2_config_geti(config, H2_CONF_MAX_WORKERS);    
-    if (minw <= 0) {
-        minw = max_threads_per_child;
-    }
-    if (maxw <= 0) {
-        /* As a default, this seems to work quite well under mpm_event. 
-         * For people enabling http2 under mpm_prefork, start 4 threads unless 
-         * configured otherwise. People get unhappy if their http2 requests are 
-         * blocking each other. */
-        maxw = H2MAX(3 * minw / 2, 4);
-    }
+    h2_get_num_workers(s, &minw, &maxw);
     
     idle_secs = h2_config_geti(config, H2_CONF_MAX_WORKER_IDLE_SECS);
     ap_log_error(APLOG_MARK, APLOG_TRACE3, 0, s,

diff --git a/modules/http2/mod_http2.c b/modules/http2/mod_http2.c
index 7881718ad081fbb0d1764def1e66dcc967a4f2ae..3d278e9f2f90d5560bd00efb4a285b7d2a3682d8 100644 (file)
--- a/modules/http2/mod_http2.c
+++ b/modules/http2/mod_http2.c
@@ -193,6 +193,11 @@ static void http2_req_engine_done(h2_req_engine *ngn, conn_rec *r_conn,
     h2_mplx_req_engine_done(ngn, r_conn, status);
 }
 
+static void http2_get_num_workers(server_rec *s, int *minw, int *maxw)
+{
+    h2_get_num_workers(s, minw, maxw);
+}
+
 /* Runs once per created child process. Perform any process 
  * related initionalization here.
  */
@@ -218,6 +223,7 @@ static void h2_hooks(apr_pool_t *pool)
     APR_REGISTER_OPTIONAL_FN(http2_req_engine_push);
     APR_REGISTER_OPTIONAL_FN(http2_req_engine_pull);
     APR_REGISTER_OPTIONAL_FN(http2_req_engine_done);
+    APR_REGISTER_OPTIONAL_FN(http2_get_num_workers);
 
     ap_log_perror(APLOG_MARK, APLOG_TRACE1, 0, pool, "installing hooks");
     

diff --git a/modules/http2/mod_http2.h b/modules/http2/mod_http2.h
index 35e141d8a98e282f851289f3a05b90c05d37f547..7a1b49a4553e0ba517ab4fa595c4dc743fafa0ac 100644 (file)
--- a/modules/http2/mod_http2.h
+++ b/modules/http2/mod_http2.h
@@ -93,4 +93,9 @@ APR_DECLARE_OPTIONAL_FN(void,
                         http2_req_engine_done, (h2_req_engine *engine, 
                                                 conn_rec *rconn,
                                                 apr_status_t status));
+
+APR_DECLARE_OPTIONAL_FN(void,
+                        http2_get_num_workers, (server_rec *s,
+                                                int *minw, int *max));
+
 #endif

diff --git a/modules/proxy/config.m4 b/modules/proxy/config.m4
index 741a9e39d9eb1538351f5be64fde9bd507483cfa..01f09fab1d46aa22c1a841600df4e96a07e2a2af 100644 (file)
--- a/modules/proxy/config.m4
+++ b/modules/proxy/config.m4
@@ -70,7 +70,7 @@ APACHE_MODULE(proxy_balancer, Apache proxy BALANCER module.  Requires --enable-p
 APACHE_MODULE(proxy_express, mass reverse-proxy module. Requires --enable-proxy., , , most, , proxy)
 APACHE_MODULE(proxy_hcheck, [reverse-proxy health-check module. Requires --enable-proxy and --enable-watchdog.], , , most, , [proxy,watchdog])
 
-APR_ADDTO(INCLUDES, [-I\$(top_srcdir)/$modpath_current])
+APR_ADDTO(INCLUDES, [-I\$(top_srcdir)/$modpath_current -I\$(top_srcdir)/modules/http2])
 
 module_selection=$save_module_selection
 module_default=$save_module_default

diff --git a/modules/proxy/mod_proxy.dsp b/modules/proxy/mod_proxy.dsp
index 853406f43bc5ba264ed713a2d90807fcded4f04a..30e2a852f564f53667100f8a8b34363a3b301f81 100644 (file)
--- a/modules/proxy/mod_proxy.dsp
+++ b/modules/proxy/mod_proxy.dsp
@@ -43,7 +43,7 @@ RSC=rc.exe
 # PROP Ignore_Export_Lib 0
 # PROP Target_Dir ""
 # ADD BASE CPP /nologo /MD /W3 /O2 /D "WIN32" /D "NDEBUG" /D "_WINDOWS" /FD /c
-# ADD CPP /nologo /MD /W3 /O2 /Oy- /Zi /I "../ssl" /I "../../include" /I "../../srclib/apr/include" /I "../../srclib/apr-util/include" /I "../generators" /D "NDEBUG" /D "WIN32" /D "_WINDOWS" /D "PROXY_DECLARE_EXPORT" /Fd"Release\mod_proxy_src" /FD /c
+# ADD CPP /nologo /MD /W3 /O2 /Oy- /Zi /I "../ssl" /I "../http2" /I "../../include" /I "../../srclib/apr/include" /I "../../srclib/apr-util/include" /I "../generators" /D "NDEBUG" /D "WIN32" /D "_WINDOWS" /D "PROXY_DECLARE_EXPORT" /Fd"Release\mod_proxy_src" /FD /c
 # ADD BASE MTL /nologo /D "NDEBUG" /win32
 # ADD MTL /nologo /D "NDEBUG" /mktyplib203 /win32
 # ADD BASE RSC /l 0x809 /d "NDEBUG"
@@ -75,7 +75,7 @@ PostBuild_Cmds=if exist $(TargetPath).manifest mt.exe -manifest $(TargetPath).ma
 # PROP Ignore_Export_Lib 0
 # PROP Target_Dir ""
 # ADD BASE CPP /nologo /MDd /W3 /EHsc /Zi /Od /D "WIN32" /D "_DEBUG" /D "_WINDOWS" /FD /c
-# ADD CPP /nologo /MDd /W3 /EHsc /Zi /Od /I "../ssl" /I "../../include" /I "../../srclib/apr/include" /I "../../srclib/apr-util/include" /I "../generators" /D "_DEBUG" /D "WIN32" /D "_WINDOWS" /D "PROXY_DECLARE_EXPORT" /Fd"Debug\mod_proxy_src" /FD /c
+# ADD CPP /nologo /MDd /W3 /EHsc /Zi /Od /I "../ssl" /I "../http2" /I "../../include" /I "../../srclib/apr/include" /I "../../srclib/apr-util/include" /I "../generators" /D "_DEBUG" /D "WIN32" /D "_WINDOWS" /D "PROXY_DECLARE_EXPORT" /Fd"Debug\mod_proxy_src" /FD /c
 # ADD BASE MTL /nologo /D "_DEBUG" /win32
 # ADD MTL /nologo /D "_DEBUG" /mktyplib203 /win32
 # ADD BASE RSC /l 0x809 /d "_DEBUG"

diff --git a/modules/proxy/proxy_util.c b/modules/proxy/proxy_util.c
index 59f5e3084dacc90e43e926d00c03b081326b147e..e848d313986eef434773f7cd5f6b46c242d97574 100644 (file)
--- a/modules/proxy/proxy_util.c
+++ b/modules/proxy/proxy_util.c
@@ -24,6 +24,8 @@
 #include "ajp.h"
 #include "scgi.h"
 
+#include "mod_http2.h" /* for http2_get_num_workers() */
+
 #if APR_HAVE_UNISTD_H
 #include <unistd.h>         /* for getpid() */
 #endif
@@ -1770,8 +1772,9 @@ PROXY_DECLARE(apr_status_t) ap_proxy_share_worker(proxy_worker *worker, proxy_wo
 
 PROXY_DECLARE(apr_status_t) ap_proxy_initialize_worker(proxy_worker *worker, server_rec *s, apr_pool_t *p)
 {
+    APR_OPTIONAL_FN_TYPE(http2_get_num_workers) *get_h2_num_workers;
     apr_status_t rv = APR_SUCCESS;
-    int mpm_threads;
+    int max_threads, minw, maxw;
 
     if (worker->s->status & PROXY_WORKER_INITIALIZED) {
         /* The worker is already initialized */
@@ -1795,11 +1798,26 @@ PROXY_DECLARE(apr_status_t) ap_proxy_initialize_worker(proxy_worker *worker, ser
             worker->s->is_address_reusable = 1;
         }
 
-        ap_mpm_query(AP_MPMQ_MAX_THREADS, &mpm_threads);
-        if (mpm_threads > 1) {
-            /* Set hard max to no more then mpm_threads */
-            if (worker->s->hmax == 0 || worker->s->hmax > mpm_threads) {
-                worker->s->hmax = mpm_threads;
+        /*
+         * When mod_http2 is loaded we might have more threads since it has
+         * its own pool of processing threads.
+         */
+        ap_mpm_query(AP_MPMQ_MAX_THREADS, &max_threads);
+        get_h2_num_workers = APR_RETRIEVE_OPTIONAL_FN(http2_get_num_workers);
+        if (get_h2_num_workers) {
+            get_h2_num_workers(s, &minw, &maxw);
+            /* So now the max is:
+             *   max_threads-1 threads for HTTP/1 each requiring one connection
+             *   + one thread for HTTP/2 requiring maxw connections
+             */
+            max_threads = max_threads - 1 + maxw;
+        }
+        if (max_threads > 1) {
+            /* Default hmax is max_threads to scale with the load and never
+             * wait for an idle connection to proceed.
+             */
+            if (worker->s->hmax == 0) {
+                worker->s->hmax = max_threads;
             }
             if (worker->s->smax == -1 || worker->s->smax > worker->s->hmax) {
                 worker->s->smax = worker->s->hmax;