You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Greg Stein <gs...@gmail.com> on 2018/02/01 08:00:49 UTC
Re: svn commit: r1822849 - /httpd/httpd/trunk/modules/proxy/proxy_util.c
On Thu, Feb 1, 2018 at 1:34 AM, <rp...@apache.org> wrote:
> Author: rpluem
> Date: Thu Feb 1 07:34:02 2018
> New Revision: 1822849
>
> URL: http://svn.apache.org/viewvc?rev=1822849&view=rev
> Log:
> * 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.
>
> Modified:
> httpd/httpd/trunk/modules/proxy/proxy_util.c
>
> Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/
> proxy/proxy_util.c?rev=1822849&r1=1822848&r2=1822849&view=diff
> ============================================================
> ==================
> --- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
> +++ httpd/httpd/trunk/modules/proxy/proxy_util.c Thu Feb 1 07:34:02 2018
> @@ -1841,8 +1841,7 @@ PROXY_DECLARE(apr_status_t) ap_proxy_ini
>
> 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) {
> + if (worker->s->hmax == 0) {
> worker->s->hmax = mpm_threads;
>
Would be nice to leave *some* kind of explanatory comment in there. Without
that comment, the only explanation is "way over there" in commit log
messages.
Cheers,
-g
Re: svn commit: r1822849 - /httpd/httpd/trunk/modules/proxy/proxy_util.c
Posted by Greg Stein <gs...@gmail.com>.
Any comment is a good comment :-)
(I don't understand the logic in there, so can't really comment on the
substance ... just noted the removal of all commentary)
Thanks!
-g
On Thu, Feb 1, 2018 at 2:36 AM, Plüm, Rüdiger, Vodafone Group <
ruediger.pluem@vodafone.com> wrote:
> Thanks for the review. Does r1822858 address your concern?
>
>
>
> Regards
>
>
>
> Rüdiger
>
>
>
> *Von:* Greg Stein [mailto:gstein@gmail.com]
> *Gesendet:* Donnerstag, 1. Februar 2018 09:01
> *An:* dev@httpd.apache.org
> *Betreff:* Re: svn commit: r1822849 - /httpd/httpd/trunk/modules/
> proxy/proxy_util.c
>
>
>
>
>
>
>
> On Thu, Feb 1, 2018 at 1:34 AM, <rp...@apache.org> wrote:
>
> Author: rpluem
> Date: Thu Feb 1 07:34:02 2018
> New Revision: 1822849
>
> URL: http://svn.apache.org/viewvc?rev=1822849&view=rev
> Log:
> * 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.
>
> Modified:
> httpd/httpd/trunk/modules/proxy/proxy_util.c
>
> Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/
> proxy/proxy_util.c?rev=1822849&r1=1822848&r2=1822849&view=diff
> ============================================================
> ==================
> --- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
> +++ httpd/httpd/trunk/modules/proxy/proxy_util.c Thu Feb 1 07:34:02 2018
> @@ -1841,8 +1841,7 @@ PROXY_DECLARE(apr_status_t) ap_proxy_ini
>
> 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) {
> + if (worker->s->hmax == 0) {
> worker->s->hmax = mpm_threads;
>
>
>
> Would be nice to leave *some* kind of explanatory comment in there.
> Without that comment, the only explanation is "way over there" in commit
> log messages.
>
>
>
> Cheers,
>
> -g
>
>
>
Re: svn commit: r1822849 -
/httpd/httpd/trunk/modules/proxy/proxy_util.c
Posted by Stefan Eissing <st...@greenbytes.de>.
You beat me to it. Well done! :-)
-Stefan
> Am 01.02.2018 um 15:43 schrieb Yann Ylavic <yl...@gmail.com>:
>
> On Thu, Feb 1, 2018 at 9:52 AM, Plüm, Rüdiger, Vodafone Group
> <ru...@vodafone.com> wrote:
>>
>>> -----Ursprüngliche Nachricht-----
>>> Von: Yann Ylavic [mailto:ylavic.dev@gmail.com]
>>> Gesendet: Donnerstag, 1. Februar 2018 09:49
>>> An: httpd-dev <de...@httpd.apache.org>
>>> Betreff: Re: svn commit: r1822849 -
>>> /httpd/httpd/trunk/modules/proxy/proxy_util.c
>>>
>>> On Thu, Feb 1, 2018 at 9:36 AM, Plüm, Rüdiger, Vodafone Group
>>> <ru...@vodafone.com> wrote:
>>>> Thanks for the review. Does r1822858 address your concern?
>>>
>>> I have a different one :p
>>>
>>> Couldn't we have a helper (optional fn) from mod_http2 which give the
>>> overhead to mod_proxy?
>>> I quite like the default hmax=0 to just work...
>>
>> This would be 2nd edition :-)
>> At least the admin now has a chance to adjust manually. But totally agree
>> that it would be cool to set the "good" value automatically by default.
>
> I couldn't wait for my use cases, r1822878 :)
> Does this work for you (and Stefan)?
>
>
> Regards,
> Yann.
Re: svn commit: r1822849 - /httpd/httpd/trunk/modules/proxy/proxy_util.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Thu, Feb 1, 2018 at 9:52 AM, Plüm, Rüdiger, Vodafone Group
<ru...@vodafone.com> wrote:
>
>> -----Ursprüngliche Nachricht-----
>> Von: Yann Ylavic [mailto:ylavic.dev@gmail.com]
>> Gesendet: Donnerstag, 1. Februar 2018 09:49
>> An: httpd-dev <de...@httpd.apache.org>
>> Betreff: Re: svn commit: r1822849 -
>> /httpd/httpd/trunk/modules/proxy/proxy_util.c
>>
>> On Thu, Feb 1, 2018 at 9:36 AM, Plüm, Rüdiger, Vodafone Group
>> <ru...@vodafone.com> wrote:
>> > Thanks for the review. Does r1822858 address your concern?
>>
>> I have a different one :p
>>
>> Couldn't we have a helper (optional fn) from mod_http2 which give the
>> overhead to mod_proxy?
>> I quite like the default hmax=0 to just work...
>
> This would be 2nd edition :-)
> At least the admin now has a chance to adjust manually. But totally agree
> that it would be cool to set the "good" value automatically by default.
I couldn't wait for my use cases, r1822878 :)
Does this work for you (and Stefan)?
Regards,
Yann.
AW: svn commit: r1822849 -
/httpd/httpd/trunk/modules/proxy/proxy_util.c
Posted by Plüm,
Rüdiger,
Vodafone Group <ru...@vodafone.com>.
> -----Ursprüngliche Nachricht-----
> Von: Yann Ylavic [mailto:ylavic.dev@gmail.com]
> Gesendet: Donnerstag, 1. Februar 2018 09:49
> An: httpd-dev <de...@httpd.apache.org>
> Betreff: Re: svn commit: r1822849 -
> /httpd/httpd/trunk/modules/proxy/proxy_util.c
>
> On Thu, Feb 1, 2018 at 9:36 AM, Plüm, Rüdiger, Vodafone Group
> <ru...@vodafone.com> wrote:
> > Thanks for the review. Does r1822858 address your concern?
>
> I have a different one :p
>
> Couldn't we have a helper (optional fn) from mod_http2 which give the
> overhead to mod_proxy?
> I quite like the default hmax=0 to just work...
This would be 2nd edition :-)
At least the admin now has a chance to adjust manually. But totally agree
that it would be cool to set the "good" value automatically by default.
Regards
Rüdiger
Re: svn commit: r1822849 - /httpd/httpd/trunk/modules/proxy/proxy_util.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Thu, Feb 1, 2018 at 9:36 AM, Plüm, Rüdiger, Vodafone Group
<ru...@vodafone.com> wrote:
> Thanks for the review. Does r1822858 address your concern?
I have a different one :p
Couldn't we have a helper (optional fn) from mod_http2 which give the
overhead to mod_proxy?
I quite like the default hmax=0 to just work...
Regards,
Yann.
AW: svn commit: r1822849 -
/httpd/httpd/trunk/modules/proxy/proxy_util.c
Posted by Plüm,
Rüdiger,
Vodafone Group <ru...@vodafone.com>.
Thanks for the review. Does r1822858 address your concern?
Regards
Rüdiger
Von: Greg Stein [mailto:gstein@gmail.com]
Gesendet: Donnerstag, 1. Februar 2018 09:01
An: dev@httpd.apache.org
Betreff: Re: svn commit: r1822849 - /httpd/httpd/trunk/modules/proxy/proxy_util.c
On Thu, Feb 1, 2018 at 1:34 AM, <rp...@apache.org>> wrote:
Author: rpluem
Date: Thu Feb 1 07:34:02 2018
New Revision: 1822849
URL: http://svn.apache.org/viewvc?rev=1822849&view=rev
Log:
* 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.
Modified:
httpd/httpd/trunk/modules/proxy/proxy_util.c
Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=1822849&r1=1822848&r2=1822849&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
+++ httpd/httpd/trunk/modules/proxy/proxy_util.c Thu Feb 1 07:34:02 2018
@@ -1841,8 +1841,7 @@ PROXY_DECLARE(apr_status_t) ap_proxy_ini
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) {
+ if (worker->s->hmax == 0) {
worker->s->hmax = mpm_threads;
Would be nice to leave *some* kind of explanatory comment in there. Without that comment, the only explanation is "way over there" in commit log messages.
Cheers,
-g