You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by jf...@apache.org on 2022/03/30 14:42:14 UTC

svn commit: r1899390 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h

Author: jfclere
Date: Wed Mar 30 14:42:14 2022
New Revision: 1899390

URL: http://svn.apache.org/viewvc?rev=1899390&view=rev
Log:
Add WorkerBalancerGrowth. To allow creation of workers
to dynamically added balancers.

Modified:
    httpd/httpd/trunk/CHANGES
    httpd/httpd/trunk/modules/proxy/mod_proxy.c
    httpd/httpd/trunk/modules/proxy/mod_proxy.h

Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1899390&r1=1899389&r2=1899390&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Wed Mar 30 14:42:14 2022
@@ -1,6 +1,10 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.5.1
 
+  *) mod_proxy: Add WorkerBalancerGrowth to allow adding workers to
+     balancer created dynamically or via "empty" <Proxy balancer://../>
+     [Jean-Frederic Clere]
+
   *) ab: Add an optional ramp delay when starting concurrent connections so
      as to not trigger denial of service protection in the network. Report
      levels of concurrency achieved in cases where the test completes before

Modified: httpd/httpd/trunk/modules/proxy/mod_proxy.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy.c?rev=1899390&r1=1899389&r2=1899390&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/mod_proxy.c (original)
+++ httpd/httpd/trunk/modules/proxy/mod_proxy.c Wed Mar 30 14:42:14 2022
@@ -1633,6 +1633,8 @@ static void * create_proxy_config(apr_po
     ps->ppinherit_set = 0;
     ps->bgrowth = 5;
     ps->bgrowth_set = 0;
+    ps->wbgrowth = 0;
+    ps->wbgrowth_set = 0;
     ps->req_set = 0;
     ps->recv_buffer_size = 0; /* this default was left unset for some reason */
     ps->recv_buffer_size_set = 0;
@@ -1789,6 +1791,8 @@ static void * merge_proxy_config(apr_poo
     ps->req_set = overrides->req_set || base->req_set;
     ps->bgrowth = (overrides->bgrowth_set == 0) ? base->bgrowth : overrides->bgrowth;
     ps->bgrowth_set = overrides->bgrowth_set || base->bgrowth_set;
+    ps->wbgrowth = (overrides->wbgrowth_set == 0) ? base->wbgrowth : overrides->wbgrowth;
+    ps->wbgrowth_set = overrides->wbgrowth_set || base->wbgrowth_set;
     ps->max_balancers = overrides->max_balancers || base->max_balancers;
     ps->bal_persist = overrides->bal_persist;
     ps->recv_buffer_size = (overrides->recv_buffer_size_set == 0) ? base->recv_buffer_size : overrides->recv_buffer_size;
@@ -2664,6 +2668,21 @@ static const char *set_bgrowth(cmd_parms
     return NULL;
 }
 
+static const char *set_wbgrowth(cmd_parms *parms, void *dummy, const char *arg)
+{
+    proxy_server_conf *psf =
+    ap_get_module_config(parms->server->module_config, &proxy_module);
+
+    int growth = atoi(arg);
+    if (growth < 0 || growth > 1000) {
+        return "WorkerBalancerGrowth must be between 0 and 1000";
+    }
+    psf->wbgrowth = growth;
+    psf->wbgrowth_set = 1;
+
+    return NULL;
+}
+
 static const char *set_persist(cmd_parms *parms, void *dummy, int flag)
 {
     proxy_server_conf *psf =
@@ -3047,6 +3066,35 @@ static const char *proxysection(cmd_parm
                 return apr_pstrcat(cmd->temp_pool, thiscmd->name, " ", err, " ",
                                    word, "=", val, "; ", conf->p, NULL);
         }
+    } else {
+        /* we have an empty <Proxy/> */
+        if (!ap_strchr_c(conf->p, ':'))
+            return apr_pstrcat(cmd->pool, thiscmd->name,
+                               "> arguments are not supported for non url.",
+                               NULL);
+        if (ap_proxy_valid_balancer_name((char *)conf->p, 9)) {
+            balancer = ap_proxy_get_balancer(cmd->pool, sconf, conf->p, 0);
+            if (!balancer) {
+                err = ap_proxy_define_balancer(cmd->pool, &balancer,
+                                               sconf, conf->p, "/", 0);
+                if (err)
+                    return apr_pstrcat(cmd->temp_pool, thiscmd->name,
+                                       " ", err, NULL);
+            }
+            if (!balancer->section_config) {
+                balancer->section_config = new_dir_conf;
+            }
+            /* Allow to add members dynamically */
+            if (sconf->wbgrowth_set) {
+                balancer->growth = sconf->wbgrowth;
+                balancer->growth_set = 1;
+            } else {
+                return apr_pstrcat(cmd->temp_pool, thiscmd->name, "/> requires WorkerBalancerGrowth > 0.", NULL);
+            }
+        } else {
+            return apr_pstrcat(cmd->pool, thiscmd->name, " " ,conf->p,
+                               "> only balancer://balancername is supported.", NULL);
+        }
     }
 
     cmd->path = old_path;
@@ -3109,6 +3157,8 @@ static const command_rec proxy_cmds[] =
      "A balancer name and scheme with list of params"),
     AP_INIT_TAKE1("BalancerGrowth", set_bgrowth, NULL, RSRC_CONF,
      "Number of additional Balancers that can be added post-config"),
+    AP_INIT_TAKE1("WorkerBalancerGrowth", set_wbgrowth, NULL, RSRC_CONF,
+     "Number of additional Workers per Balancer that can be added post-config"),
     AP_INIT_FLAG("BalancerPersist", set_persist, NULL, RSRC_CONF,
      "on if the balancer should persist changes on reboot/restart made via the Balancer Manager"),
     AP_INIT_FLAG("BalancerInherit", set_inherit, NULL, RSRC_CONF,

Modified: httpd/httpd/trunk/modules/proxy/mod_proxy.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy.h?rev=1899390&r1=1899389&r2=1899390&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/mod_proxy.h (original)
+++ httpd/httpd/trunk/modules/proxy/mod_proxy.h Wed Mar 30 14:42:14 2022
@@ -163,6 +163,7 @@ typedef struct {
     int req;                /* true if proxy requests are enabled */
     int max_balancers;      /* maximum number of allowed balancers */
     int bgrowth;            /* number of post-config balancers can added */
+    int wbgrowth;            /* number of post-config work per balancer that can added */
     enum {
       via_off,
       via_on,
@@ -198,6 +199,7 @@ typedef struct {
     unsigned int proxy_status_set:1;
     unsigned int source_address_set:1;
     unsigned int bgrowth_set:1;
+    unsigned int wbgrowth_set:1;
     unsigned int bal_persist:1;
     unsigned int inherit:1;
     unsigned int inherit_set:1;



Re: svn commit: r1899390 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h

Posted by jean-frederic clere <jf...@gmail.com>.
On 31/03/2022 12:34, Stefan Eissing wrote:
> 
> 
>> Am 31.03.2022 um 11:55 schrieb Ruediger Pluem <rp...@apache.org>:
>>
>>
>>
>> On 3/31/22 11:11 AM, Ruediger Pluem wrote:
>>>
>>>
>>> On 3/30/22 4:42 PM, jfclere@apache.org wrote:
>>>> Author: jfclere
>>>> Date: Wed Mar 30 14:42:14 2022
>>>> New Revision: 1899390
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=1899390&view=rev
>>>> Log:
>>>> Add WorkerBalancerGrowth. To allow creation of workers
>>>> to dynamically added balancers.
>>>>
>>>> Modified:
>>>> httpd/httpd/trunk/CHANGES
>>>> httpd/httpd/trunk/modules/proxy/mod_proxy.c
>>>> httpd/httpd/trunk/modules/proxy/mod_proxy.h
>>>>
>>>> Modified: httpd/httpd/trunk/CHANGES
>>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1899390&r1=1899389&r2=1899390&view=diff
>>>> ==============================================================================
>>>> --- httpd/httpd/trunk/CHANGES [utf-8] (original)
>>>> +++ httpd/httpd/trunk/CHANGES [utf-8] Wed Mar 30 14:42:14 2022
>>>> @@ -1,6 +1,10 @@
>>>> -*- coding: utf-8 -*-
>>>> Changes with Apache 2.5.1
>>>>
>>>> + *) mod_proxy: Add WorkerBalancerGrowth to allow adding workers to
>>>> + balancer created dynamically or via "empty" <Proxy balancer://../>
>>>> + [Jean-Frederic Clere]
>>>
>>> I am not sure why this is needed. You can already do this via
>>>
>>> <Proxy balancer://somebalancer/ growth=10>
>>> </Proxy>
>>
>> Or
>>
>> <Proxy balancer://somebalancer/>
>> ProxySet growth=10
>> </Proxy>
> 
> FYI: Travis trunk also fails almost completely. Does not seem to accept a proxy configuration.

I have done the PR and I am now waiting on Travis.

> 
>>
>> Regards
>>
>> Rüdiger
> 


-- 
Cheers

Jean-Frederic


Re: svn commit: r1899390 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h

Posted by Stefan Eissing <st...@eissing.org>.

> Am 01.04.2022 um 08:47 schrieb jean-frederic clere <jf...@gmail.com>:
> 
> On 31/03/2022 12:59, Ruediger Pluem wrote:
>> On 3/31/22 12:34 PM, Stefan Eissing wrote:
>>> 
>>> 
>>>> Am 31.03.2022 um 11:55 schrieb Ruediger Pluem <rp...@apache.org>:
>>>> 
>>>> 
>>>> 
>>>> On 3/31/22 11:11 AM, Ruediger Pluem wrote:
>>>>> 
>>>>> 
>>>>> On 3/30/22 4:42 PM, jfclere@apache.org wrote:
>>>>>> Author: jfclere
>>>>>> Date: Wed Mar 30 14:42:14 2022
>>>>>> New Revision: 1899390
>>>>>> 
>>>>>> URL: http://svn.apache.org/viewvc?rev=1899390&view=rev
>>>>>> Log:
>>>>>> Add WorkerBalancerGrowth. To allow creation of workers
>>>>>> to dynamically added balancers.
>>>>>> 
>>>>>> Modified:
>>>>>> httpd/httpd/trunk/CHANGES
>>>>>> httpd/httpd/trunk/modules/proxy/mod_proxy.c
>>>>>> httpd/httpd/trunk/modules/proxy/mod_proxy.h
>>>>>> 
>>>>>> Modified: httpd/httpd/trunk/CHANGES
>>>>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1899390&r1=1899389&r2=1899390&view=diff
>>>>>> ==============================================================================
>>>>>> --- httpd/httpd/trunk/CHANGES [utf-8] (original)
>>>>>> +++ httpd/httpd/trunk/CHANGES [utf-8] Wed Mar 30 14:42:14 2022
>>>>>> @@ -1,6 +1,10 @@
>>>>>> -*- coding: utf-8 -*-
>>>>>> Changes with Apache 2.5.1
>>>>>> 
>>>>>> + *) mod_proxy: Add WorkerBalancerGrowth to allow adding workers to
>>>>>> + balancer created dynamically or via "empty" <Proxy balancer://../>
>>>>>> + [Jean-Frederic Clere]
>>>>> 
>>>>> I am not sure why this is needed. You can already do this via
>>>>> 
>>>>> <Proxy balancer://somebalancer/ growth=10>
>>>>> </Proxy>
>>>> 
>>>> Or
>>>> 
>>>> <Proxy balancer://somebalancer/>
>>>> ProxySet growth=10
>>>> </Proxy>
>>> 
>>> FYI: Travis trunk also fails almost completely. Does not seem to accept a proxy configuration.
>> This is because the if
>> + if (!ap_strchr_c(conf->p, ':'))
>> + return apr_pstrcat(cmd->pool, thiscmd->name,
>> + "> arguments are not supported for non url.",
>> + NULL);
>> should not return with an error, but just encapsulate the remainder of the block. And I think the further
>> return apr_pstrcat are also wrong.
>> But as said I am not sure about the purpose at all as you can already do, what the patch should provide if I understand the patch
>> correctly.
> 
> The purpose was to be able to add a balancer in the balancer-manager handle but that needs to pre-create the mutex and the slots for the workers.
> 
> While looking to that I noted that:
> <Proxy balancer://somebalancer/>
> </Proxy>
> 
> was doing nothing, the balancer is ignored, I should I revert the patch and add an error message if there is an empty entry like this one?

I am much in favour of making CI work again, as I am currently flying blind on my changes and PRs. However that works best for you.

Kind Regards,
Stefan

> 
>> Regards
>> Rüdiger
> 
> 
> -- 
> Cheers
> 
> Jean-Frederic


Re: svn commit: r1899390 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h

Posted by Ruediger Pluem <rp...@apache.org>.

On 4/1/22 10:23 AM, jean-frederic clere wrote:
> On 01/04/2022 10:03, Ruediger Pluem wrote:
>>

> 
> Sure I have a PR to revert, waiting on travis...
> 
>> Feel free to add a detection for such empty proxy blocks. I think a warning
>> should be sufficient. How do you want to detect this? By inspecting new_dir_conf after ap_walk_config was executed?
> 
> putting it in the proxysection() is not the best, correct?

I haven't made detailed thoughts on how to implement this detection. Hence I cannot answer :-)

Regards

Rüdiger


Re: svn commit: r1899390 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h

Posted by jean-frederic clere <jf...@gmail.com>.
On 01/04/2022 10:03, Ruediger Pluem wrote:
> 
> 
> On 4/1/22 8:47 AM, jean-frederic clere wrote:
>> On 31/03/2022 12:59, Ruediger Pluem wrote:
>>>
>>>
>>> On 3/31/22 12:34 PM, Stefan Eissing wrote:
>>>>
>>>>
>>>>> Am 31.03.2022 um 11:55 schrieb Ruediger Pluem <rp...@apache.org>:
>>>>>
>>>>>
>>>>>
>>>>> On 3/31/22 11:11 AM, Ruediger Pluem wrote:
>>>>>>
>>>>>>
>>>>>> On 3/30/22 4:42 PM, jfclere@apache.org wrote:
>>>>>>> Author: jfclere
>>>>>>> Date: Wed Mar 30 14:42:14 2022
>>>>>>> New Revision: 1899390
>>>>>>>
>>>>>>> URL: http://svn.apache.org/viewvc?rev=1899390&view=rev
>>>>>>> Log:
>>>>>>> Add WorkerBalancerGrowth. To allow creation of workers
>>>>>>> to dynamically added balancers.
>>>>>>>
>>>>>>> Modified:
>>>>>>> httpd/httpd/trunk/CHANGES
>>>>>>> httpd/httpd/trunk/modules/proxy/mod_proxy.c
>>>>>>> httpd/httpd/trunk/modules/proxy/mod_proxy.h
>>>>>>>
>>>>>>> Modified: httpd/httpd/trunk/CHANGES
>>>>>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1899390&r1=1899389&r2=1899390&view=diff
>>>>>>> ==============================================================================
>>>>>>> --- httpd/httpd/trunk/CHANGES [utf-8] (original)
>>>>>>> +++ httpd/httpd/trunk/CHANGES [utf-8] Wed Mar 30 14:42:14 2022
>>>>>>> @@ -1,6 +1,10 @@
>>>>>>> -*- coding: utf-8 -*-
>>>>>>> Changes with Apache 2.5.1
>>>>>>>
>>>>>>> + *) mod_proxy: Add WorkerBalancerGrowth to allow adding workers to
>>>>>>> + balancer created dynamically or via "empty" <Proxy balancer://../>
>>>>>>> + [Jean-Frederic Clere]
>>>>>>
>>>>>> I am not sure why this is needed. You can already do this via
>>>>>>
>>>>>> <Proxy balancer://somebalancer/ growth=10>
>>>>>> </Proxy>
>>>>>
>>>>> Or
>>>>>
>>>>> <Proxy balancer://somebalancer/>
>>>>> ProxySet growth=10
>>>>> </Proxy>
>>>>
>>>> FYI: Travis trunk also fails almost completely. Does not seem to accept a proxy configuration.
>>>
>>> This is because the if
>>>
>>> +        if (!ap_strchr_c(conf->p, ':'))
>>> +            return apr_pstrcat(cmd->pool, thiscmd->name,
>>> +                               "> arguments are not supported for non url.",
>>> +                               NULL);
>>>
>>> should not return with an error, but just encapsulate the remainder of the block. And I think the further
>>> return apr_pstrcat are also wrong.
>>>
>>> But as said I am not sure about the purpose at all as you can already do, what the patch should provide if I understand the patch
>>> correctly.
>>
>> The purpose was to be able to add a balancer in the balancer-manager handle but that needs to pre-create the mutex and the slots
>> for the workers.
>>
>> While looking to that I noted that:
>> <Proxy balancer://somebalancer/>
>> </Proxy>
>>
>> was doing nothing, the balancer is ignored, I should I revert the patch and add an error message if there is an empty entry like
>> this one?
> 
> Do
> 
> <Proxy balancer://somebalancer/ growth=10>
> </Proxy>
> 
> or
> 
> <Proxy balancer://somebalancer/>
> ProxySet growth=10
> </Proxy>
> 
> work and do what you want? If yes, please revert.

Sure I have a PR to revert, waiting on travis...

> Feel free to add a detection for such empty proxy blocks. I think a warning
> should be sufficient. How do you want to detect this? By inspecting new_dir_conf after ap_walk_config was executed?

putting it in the proxysection() is not the best, correct?

> 
> Regards
> 
> Rüdiger


-- 
Cheers

Jean-Frederic


Re: svn commit: r1899390 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h

Posted by Ruediger Pluem <rp...@apache.org>.

On 4/1/22 8:47 AM, jean-frederic clere wrote:
> On 31/03/2022 12:59, Ruediger Pluem wrote:
>>
>>
>> On 3/31/22 12:34 PM, Stefan Eissing wrote:
>>>
>>>
>>>> Am 31.03.2022 um 11:55 schrieb Ruediger Pluem <rp...@apache.org>:
>>>>
>>>>
>>>>
>>>> On 3/31/22 11:11 AM, Ruediger Pluem wrote:
>>>>>
>>>>>
>>>>> On 3/30/22 4:42 PM, jfclere@apache.org wrote:
>>>>>> Author: jfclere
>>>>>> Date: Wed Mar 30 14:42:14 2022
>>>>>> New Revision: 1899390
>>>>>>
>>>>>> URL: http://svn.apache.org/viewvc?rev=1899390&view=rev
>>>>>> Log:
>>>>>> Add WorkerBalancerGrowth. To allow creation of workers
>>>>>> to dynamically added balancers.
>>>>>>
>>>>>> Modified:
>>>>>> httpd/httpd/trunk/CHANGES
>>>>>> httpd/httpd/trunk/modules/proxy/mod_proxy.c
>>>>>> httpd/httpd/trunk/modules/proxy/mod_proxy.h
>>>>>>
>>>>>> Modified: httpd/httpd/trunk/CHANGES
>>>>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1899390&r1=1899389&r2=1899390&view=diff
>>>>>> ==============================================================================
>>>>>> --- httpd/httpd/trunk/CHANGES [utf-8] (original)
>>>>>> +++ httpd/httpd/trunk/CHANGES [utf-8] Wed Mar 30 14:42:14 2022
>>>>>> @@ -1,6 +1,10 @@
>>>>>> -*- coding: utf-8 -*-
>>>>>> Changes with Apache 2.5.1
>>>>>>
>>>>>> + *) mod_proxy: Add WorkerBalancerGrowth to allow adding workers to
>>>>>> + balancer created dynamically or via "empty" <Proxy balancer://../>
>>>>>> + [Jean-Frederic Clere]
>>>>>
>>>>> I am not sure why this is needed. You can already do this via
>>>>>
>>>>> <Proxy balancer://somebalancer/ growth=10>
>>>>> </Proxy>
>>>>
>>>> Or
>>>>
>>>> <Proxy balancer://somebalancer/>
>>>> ProxySet growth=10
>>>> </Proxy>
>>>
>>> FYI: Travis trunk also fails almost completely. Does not seem to accept a proxy configuration.
>>
>> This is because the if
>>
>> +        if (!ap_strchr_c(conf->p, ':'))
>> +            return apr_pstrcat(cmd->pool, thiscmd->name,
>> +                               "> arguments are not supported for non url.",
>> +                               NULL);
>>
>> should not return with an error, but just encapsulate the remainder of the block. And I think the further
>> return apr_pstrcat are also wrong.
>>
>> But as said I am not sure about the purpose at all as you can already do, what the patch should provide if I understand the patch
>> correctly.
> 
> The purpose was to be able to add a balancer in the balancer-manager handle but that needs to pre-create the mutex and the slots
> for the workers.
> 
> While looking to that I noted that:
> <Proxy balancer://somebalancer/>
> </Proxy>
> 
> was doing nothing, the balancer is ignored, I should I revert the patch and add an error message if there is an empty entry like
> this one?

Do

<Proxy balancer://somebalancer/ growth=10>
</Proxy>

or

<Proxy balancer://somebalancer/>
ProxySet growth=10
</Proxy>

work and do what you want? If yes, please revert. Feel free to add a detection for such empty proxy blocks. I think a warning
should be sufficient. How do you want to detect this? By inspecting new_dir_conf after ap_walk_config was executed?

Regards

Rüdiger

Re: svn commit: r1899390 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h

Posted by jean-frederic clere <jf...@gmail.com>.
On 01/04/2022 13:41, Jim Jagielski wrote:
> It was added in anticipation of the capability to be folded in, and done 
> so "now" so that it would;t require any API changes.
> 
> Unless it's actually breaking something, I'd vote to simply keep it

OK I will try to propose some code to create the balancers I am still 
stuck how to create the memory slots for the workers of the those 
dynamic balancers.

> 
>> On Apr 1, 2022, at 3:42 AM, jean-frederic clere <jfclere@gmail.com 
>> <ma...@gmail.com>> wrote:
>>
>> On 01/04/2022 08:47, jean-frederic clere wrote:
>>> On 31/03/2022 12:59, Ruediger Pluem wrote:
>>>>
>>>>
>>>> On 3/31/22 12:34 PM, Stefan Eissing wrote:
>>>>>
>>>>>
>>>>>> Am 31.03.2022 um 11:55 schrieb Ruediger Pluem <rpluem@apache.org 
>>>>>> <ma...@apache.org>>:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 3/31/22 11:11 AM, Ruediger Pluem wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 3/30/22 4:42 PM, jfclere@apache.org 
>>>>>>> <ma...@apache.org> wrote:
>>>>>>>> Author: jfclere
>>>>>>>> Date: Wed Mar 30 14:42:14 2022
>>>>>>>> New Revision: 1899390
>>>>>>>>
>>>>>>>> URL: http://svn.apache.org/viewvc?rev=1899390&view=rev 
>>>>>>>> <http://svn.apache.org/viewvc?rev=1899390&view=rev>
>>>>>>>> Log:
>>>>>>>> Add WorkerBalancerGrowth. To allow creation of workers
>>>>>>>> to dynamically added balancers.
>>>>>>>>
>>>>>>>> Modified:
>>>>>>>> httpd/httpd/trunk/CHANGES
>>>>>>>> httpd/httpd/trunk/modules/proxy/mod_proxy.c
>>>>>>>> httpd/httpd/trunk/modules/proxy/mod_proxy.h
>>>>>>>>
>>>>>>>> Modified: httpd/httpd/trunk/CHANGES
>>>>>>>> URL: 
>>>>>>>> http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1899390&r1=1899389&r2=1899390&view=diff 
>>>>>>>> <http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1899390&r1=1899389&r2=1899390&view=diff>
>>>>>>>> ==============================================================================
>>>>>>>> --- httpd/httpd/trunk/CHANGES [utf-8] (original)
>>>>>>>> +++ httpd/httpd/trunk/CHANGES [utf-8] Wed Mar 30 14:42:14 2022
>>>>>>>> @@ -1,6 +1,10 @@
>>>>>>>> -*- coding: utf-8 -*-
>>>>>>>> Changes with Apache 2.5.1
>>>>>>>>
>>>>>>>> + *) mod_proxy: Add WorkerBalancerGrowth to allow adding workers to
>>>>>>>> + balancer created dynamically or via "empty" <Proxy 
>>>>>>>> balancer://../ <balancer://../>>
>>>>>>>> + [Jean-Frederic Clere]
>>>>>>>
>>>>>>> I am not sure why this is needed. You can already do this via
>>>>>>>
>>>>>>> <Proxy balancer://somebalancer/ <balancer://somebalancer/> growth=10>
>>>>>>> </Proxy>
>>>>>>
>>>>>> Or
>>>>>>
>>>>>> <Proxy balancer://somebalancer/ <balancer://somebalancer/>>
>>>>>> ProxySet growth=10
>>>>>> </Proxy>
>>>>>
>>>>> FYI: Travis trunk also fails almost completely. Does not seem to 
>>>>> accept a proxy configuration.
>>>>
>>>> This is because the if
>>>>
>>>> +  if (!ap_strchr_c(conf->p, ':'))
>>>> +  return apr_pstrcat(cmd->pool, thiscmd->name,
>>>> +  "> arguments are not supported for non url.",
>>>> +  NULL);
>>>>
>>>> should not return with an error, but just encapsulate the remainder 
>>>> of the block. And I think the further
>>>> return apr_pstrcat are also wrong.
>>>>
>>>> But as said I am not sure about the purpose at all as you can 
>>>> already do, what the patch should provide if I understand the patch
>>>> correctly.
>>> The purpose was to be able to add a balancer in the balancer-manager 
>>> handle but that needs to pre-create the mutex and the slots for the 
>>> workers.
>>> While looking to that I noted that:
>>> <Proxy balancer://somebalancer/ <balancer://somebalancer/>>
>>> </Proxy>
>>> was doing nothing, the balancer is ignored, I should I revert the 
>>> patch and add an error message if there is an empty entry like this one?
>>
>> There is also the BalancerGrowth directive that does nothing else than 
>> creating a memory slot for balancers we never add/create in the 
>> balancer-manager, I am tempted to remove it...
>>
>> Would it be better to add the missing logic? I have some pieces in 
>> mod_proxy_cluster (https://github.com/modcluster/mod_proxy_cluster 
>> <https://github.com/modcluster/mod_proxy_cluster>that could use the logic.
>>>>
>>>> Regards
>>>>
>>>> Rüdiger
>>
>>
>> --
>> Cheers
>>
>> Jean-Frederic
> 


-- 
Cheers

Jean-Frederic


Re: svn commit: r1899390 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h

Posted by Jim Jagielski <ji...@jaguNET.com>.
It was added in anticipation of the capability to be folded in, and done so "now" so that it would;t require any API changes.

Unless it's actually breaking something, I'd vote to simply keep it

> On Apr 1, 2022, at 3:42 AM, jean-frederic clere <jf...@gmail.com> wrote:
> 
> On 01/04/2022 08:47, jean-frederic clere wrote:
>> On 31/03/2022 12:59, Ruediger Pluem wrote:
>>> 
>>> 
>>> On 3/31/22 12:34 PM, Stefan Eissing wrote:
>>>> 
>>>> 
>>>>> Am 31.03.2022 um 11:55 schrieb Ruediger Pluem <rp...@apache.org>:
>>>>> 
>>>>> 
>>>>> 
>>>>> On 3/31/22 11:11 AM, Ruediger Pluem wrote:
>>>>>> 
>>>>>> 
>>>>>> On 3/30/22 4:42 PM, jfclere@apache.org wrote:
>>>>>>> Author: jfclere
>>>>>>> Date: Wed Mar 30 14:42:14 2022
>>>>>>> New Revision: 1899390
>>>>>>> 
>>>>>>> URL: http://svn.apache.org/viewvc?rev=1899390&view=rev
>>>>>>> Log:
>>>>>>> Add WorkerBalancerGrowth. To allow creation of workers
>>>>>>> to dynamically added balancers.
>>>>>>> 
>>>>>>> Modified:
>>>>>>> httpd/httpd/trunk/CHANGES
>>>>>>> httpd/httpd/trunk/modules/proxy/mod_proxy.c
>>>>>>> httpd/httpd/trunk/modules/proxy/mod_proxy.h
>>>>>>> 
>>>>>>> Modified: httpd/httpd/trunk/CHANGES
>>>>>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1899390&r1=1899389&r2=1899390&view=diff 
>>>>>>> ============================================================================== 
>>>>>>> --- httpd/httpd/trunk/CHANGES [utf-8] (original)
>>>>>>> +++ httpd/httpd/trunk/CHANGES [utf-8] Wed Mar 30 14:42:14 2022
>>>>>>> @@ -1,6 +1,10 @@
>>>>>>> -*- coding: utf-8 -*-
>>>>>>> Changes with Apache 2.5.1
>>>>>>> 
>>>>>>> + *) mod_proxy: Add WorkerBalancerGrowth to allow adding workers to
>>>>>>> + balancer created dynamically or via "empty" <Proxy balancer://../>
>>>>>>> + [Jean-Frederic Clere]
>>>>>> 
>>>>>> I am not sure why this is needed. You can already do this via
>>>>>> 
>>>>>> <Proxy balancer://somebalancer/ growth=10>
>>>>>> </Proxy>
>>>>> 
>>>>> Or
>>>>> 
>>>>> <Proxy balancer://somebalancer/>
>>>>> ProxySet growth=10
>>>>> </Proxy>
>>>> 
>>>> FYI: Travis trunk also fails almost completely. Does not seem to accept a proxy configuration.
>>> 
>>> This is because the if
>>> 
>>> +  if (!ap_strchr_c(conf->p, ':'))
>>> +  return apr_pstrcat(cmd->pool, thiscmd->name,
>>> +  "> arguments are not supported for non url.",
>>> +  NULL);
>>> 
>>> should not return with an error, but just encapsulate the remainder of the block. And I think the further
>>> return apr_pstrcat are also wrong.
>>> 
>>> But as said I am not sure about the purpose at all as you can already do, what the patch should provide if I understand the patch
>>> correctly.
>> The purpose was to be able to add a balancer in the balancer-manager handle but that needs to pre-create the mutex and the slots for the workers.
>> While looking to that I noted that:
>> <Proxy balancer://somebalancer/>
>> </Proxy>
>> was doing nothing, the balancer is ignored, I should I revert the patch and add an error message if there is an empty entry like this one?
> 
> There is also the BalancerGrowth directive that does nothing else than creating a memory slot for balancers we never add/create in the balancer-manager, I am tempted to remove it...
> 
> Would it be better to add the missing logic? I have some pieces in mod_proxy_cluster (https://github.com/modcluster/mod_proxy_cluster <https://github.com/modcluster/mod_proxy_cluster> that could use the logic.
>>> 
>>> Regards
>>> 
>>> Rüdiger
> 
> 
> -- 
> Cheers
> 
> Jean-Frederic


Re: svn commit: r1899390 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h

Posted by jean-frederic clere <jf...@gmail.com>.
On 01/04/2022 08:47, jean-frederic clere wrote:
> On 31/03/2022 12:59, Ruediger Pluem wrote:
>>
>>
>> On 3/31/22 12:34 PM, Stefan Eissing wrote:
>>>
>>>
>>>> Am 31.03.2022 um 11:55 schrieb Ruediger Pluem <rp...@apache.org>:
>>>>
>>>>
>>>>
>>>> On 3/31/22 11:11 AM, Ruediger Pluem wrote:
>>>>>
>>>>>
>>>>> On 3/30/22 4:42 PM, jfclere@apache.org wrote:
>>>>>> Author: jfclere
>>>>>> Date: Wed Mar 30 14:42:14 2022
>>>>>> New Revision: 1899390
>>>>>>
>>>>>> URL: http://svn.apache.org/viewvc?rev=1899390&view=rev
>>>>>> Log:
>>>>>> Add WorkerBalancerGrowth. To allow creation of workers
>>>>>> to dynamically added balancers.
>>>>>>
>>>>>> Modified:
>>>>>> httpd/httpd/trunk/CHANGES
>>>>>> httpd/httpd/trunk/modules/proxy/mod_proxy.c
>>>>>> httpd/httpd/trunk/modules/proxy/mod_proxy.h
>>>>>>
>>>>>> Modified: httpd/httpd/trunk/CHANGES
>>>>>> URL: 
>>>>>> http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1899390&r1=1899389&r2=1899390&view=diff 
>>>>>>
>>>>>> ============================================================================== 
>>>>>>
>>>>>> --- httpd/httpd/trunk/CHANGES [utf-8] (original)
>>>>>> +++ httpd/httpd/trunk/CHANGES [utf-8] Wed Mar 30 14:42:14 2022
>>>>>> @@ -1,6 +1,10 @@
>>>>>> -*- coding: utf-8 -*-
>>>>>> Changes with Apache 2.5.1
>>>>>>
>>>>>> + *) mod_proxy: Add WorkerBalancerGrowth to allow adding workers to
>>>>>> + balancer created dynamically or via "empty" <Proxy balancer://../>
>>>>>> + [Jean-Frederic Clere]
>>>>>
>>>>> I am not sure why this is needed. You can already do this via
>>>>>
>>>>> <Proxy balancer://somebalancer/ growth=10>
>>>>> </Proxy>
>>>>
>>>> Or
>>>>
>>>> <Proxy balancer://somebalancer/>
>>>> ProxySet growth=10
>>>> </Proxy>
>>>
>>> FYI: Travis trunk also fails almost completely. Does not seem to 
>>> accept a proxy configuration.
>>
>> This is because the if
>>
>> +        if (!ap_strchr_c(conf->p, ':'))
>> +            return apr_pstrcat(cmd->pool, thiscmd->name,
>> +                               "> arguments are not supported for non 
>> url.",
>> +                               NULL);
>>
>> should not return with an error, but just encapsulate the remainder of 
>> the block. And I think the further
>> return apr_pstrcat are also wrong.
>>
>> But as said I am not sure about the purpose at all as you can already 
>> do, what the patch should provide if I understand the patch
>> correctly.
> 
> The purpose was to be able to add a balancer in the balancer-manager 
> handle but that needs to pre-create the mutex and the slots for the 
> workers.
> 
> While looking to that I noted that:
> <Proxy balancer://somebalancer/>
> </Proxy>
> 
> was doing nothing, the balancer is ignored, I should I revert the patch 
> and add an error message if there is an empty entry like this one?

There is also the BalancerGrowth directive that does nothing else than 
creating a memory slot for balancers we never add/create in the 
balancer-manager, I am tempted to remove it...

Would it be better to add the missing logic? I have some pieces in 
mod_proxy_cluster (https://github.com/modcluster/mod_proxy_cluster that 
could use the logic.
> 
>>
>> Regards
>>
>> Rüdiger
> 
> 


-- 
Cheers

Jean-Frederic


Re: svn commit: r1899390 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h

Posted by jean-frederic clere <jf...@gmail.com>.
On 31/03/2022 12:59, Ruediger Pluem wrote:
> 
> 
> On 3/31/22 12:34 PM, Stefan Eissing wrote:
>>
>>
>>> Am 31.03.2022 um 11:55 schrieb Ruediger Pluem <rp...@apache.org>:
>>>
>>>
>>>
>>> On 3/31/22 11:11 AM, Ruediger Pluem wrote:
>>>>
>>>>
>>>> On 3/30/22 4:42 PM, jfclere@apache.org wrote:
>>>>> Author: jfclere
>>>>> Date: Wed Mar 30 14:42:14 2022
>>>>> New Revision: 1899390
>>>>>
>>>>> URL: http://svn.apache.org/viewvc?rev=1899390&view=rev
>>>>> Log:
>>>>> Add WorkerBalancerGrowth. To allow creation of workers
>>>>> to dynamically added balancers.
>>>>>
>>>>> Modified:
>>>>> httpd/httpd/trunk/CHANGES
>>>>> httpd/httpd/trunk/modules/proxy/mod_proxy.c
>>>>> httpd/httpd/trunk/modules/proxy/mod_proxy.h
>>>>>
>>>>> Modified: httpd/httpd/trunk/CHANGES
>>>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1899390&r1=1899389&r2=1899390&view=diff
>>>>> ==============================================================================
>>>>> --- httpd/httpd/trunk/CHANGES [utf-8] (original)
>>>>> +++ httpd/httpd/trunk/CHANGES [utf-8] Wed Mar 30 14:42:14 2022
>>>>> @@ -1,6 +1,10 @@
>>>>> -*- coding: utf-8 -*-
>>>>> Changes with Apache 2.5.1
>>>>>
>>>>> + *) mod_proxy: Add WorkerBalancerGrowth to allow adding workers to
>>>>> + balancer created dynamically or via "empty" <Proxy balancer://../>
>>>>> + [Jean-Frederic Clere]
>>>>
>>>> I am not sure why this is needed. You can already do this via
>>>>
>>>> <Proxy balancer://somebalancer/ growth=10>
>>>> </Proxy>
>>>
>>> Or
>>>
>>> <Proxy balancer://somebalancer/>
>>> ProxySet growth=10
>>> </Proxy>
>>
>> FYI: Travis trunk also fails almost completely. Does not seem to accept a proxy configuration.
> 
> This is because the if
> 
> +        if (!ap_strchr_c(conf->p, ':'))
> +            return apr_pstrcat(cmd->pool, thiscmd->name,
> +                               "> arguments are not supported for non url.",
> +                               NULL);
> 
> should not return with an error, but just encapsulate the remainder of the block. And I think the further
> return apr_pstrcat are also wrong.
> 
> But as said I am not sure about the purpose at all as you can already do, what the patch should provide if I understand the patch
> correctly.

The purpose was to be able to add a balancer in the balancer-manager 
handle but that needs to pre-create the mutex and the slots for the workers.

While looking to that I noted that:
<Proxy balancer://somebalancer/>
</Proxy>

was doing nothing, the balancer is ignored, I should I revert the patch 
and add an error message if there is an empty entry like this one?

> 
> Regards
> 
> Rüdiger


-- 
Cheers

Jean-Frederic


Re: svn commit: r1899390 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h

Posted by Ruediger Pluem <rp...@apache.org>.

On 3/31/22 12:34 PM, Stefan Eissing wrote:
> 
> 
>> Am 31.03.2022 um 11:55 schrieb Ruediger Pluem <rp...@apache.org>:
>>
>>
>>
>> On 3/31/22 11:11 AM, Ruediger Pluem wrote:
>>>
>>>
>>> On 3/30/22 4:42 PM, jfclere@apache.org wrote:
>>>> Author: jfclere
>>>> Date: Wed Mar 30 14:42:14 2022
>>>> New Revision: 1899390
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=1899390&view=rev
>>>> Log:
>>>> Add WorkerBalancerGrowth. To allow creation of workers
>>>> to dynamically added balancers.
>>>>
>>>> Modified:
>>>> httpd/httpd/trunk/CHANGES
>>>> httpd/httpd/trunk/modules/proxy/mod_proxy.c
>>>> httpd/httpd/trunk/modules/proxy/mod_proxy.h
>>>>
>>>> Modified: httpd/httpd/trunk/CHANGES
>>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1899390&r1=1899389&r2=1899390&view=diff
>>>> ==============================================================================
>>>> --- httpd/httpd/trunk/CHANGES [utf-8] (original)
>>>> +++ httpd/httpd/trunk/CHANGES [utf-8] Wed Mar 30 14:42:14 2022
>>>> @@ -1,6 +1,10 @@
>>>> -*- coding: utf-8 -*-
>>>> Changes with Apache 2.5.1
>>>>
>>>> + *) mod_proxy: Add WorkerBalancerGrowth to allow adding workers to
>>>> + balancer created dynamically or via "empty" <Proxy balancer://../>
>>>> + [Jean-Frederic Clere]
>>>
>>> I am not sure why this is needed. You can already do this via
>>>
>>> <Proxy balancer://somebalancer/ growth=10>
>>> </Proxy>
>>
>> Or
>>
>> <Proxy balancer://somebalancer/>
>> ProxySet growth=10
>> </Proxy>
> 
> FYI: Travis trunk also fails almost completely. Does not seem to accept a proxy configuration.

This is because the if

+        if (!ap_strchr_c(conf->p, ':'))
+            return apr_pstrcat(cmd->pool, thiscmd->name,
+                               "> arguments are not supported for non url.",
+                               NULL);

should not return with an error, but just encapsulate the remainder of the block. And I think the further
return apr_pstrcat are also wrong.

But as said I am not sure about the purpose at all as you can already do, what the patch should provide if I understand the patch
correctly.

Regards

Rüdiger

Re: svn commit: r1899390 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h

Posted by Stefan Eissing <st...@eissing.org>.

> Am 31.03.2022 um 11:55 schrieb Ruediger Pluem <rp...@apache.org>:
> 
> 
> 
> On 3/31/22 11:11 AM, Ruediger Pluem wrote:
>> 
>> 
>> On 3/30/22 4:42 PM, jfclere@apache.org wrote:
>>> Author: jfclere
>>> Date: Wed Mar 30 14:42:14 2022
>>> New Revision: 1899390
>>> 
>>> URL: http://svn.apache.org/viewvc?rev=1899390&view=rev
>>> Log:
>>> Add WorkerBalancerGrowth. To allow creation of workers
>>> to dynamically added balancers.
>>> 
>>> Modified:
>>> httpd/httpd/trunk/CHANGES
>>> httpd/httpd/trunk/modules/proxy/mod_proxy.c
>>> httpd/httpd/trunk/modules/proxy/mod_proxy.h
>>> 
>>> Modified: httpd/httpd/trunk/CHANGES
>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1899390&r1=1899389&r2=1899390&view=diff
>>> ==============================================================================
>>> --- httpd/httpd/trunk/CHANGES [utf-8] (original)
>>> +++ httpd/httpd/trunk/CHANGES [utf-8] Wed Mar 30 14:42:14 2022
>>> @@ -1,6 +1,10 @@
>>> -*- coding: utf-8 -*-
>>> Changes with Apache 2.5.1
>>> 
>>> + *) mod_proxy: Add WorkerBalancerGrowth to allow adding workers to
>>> + balancer created dynamically or via "empty" <Proxy balancer://../>
>>> + [Jean-Frederic Clere]
>> 
>> I am not sure why this is needed. You can already do this via
>> 
>> <Proxy balancer://somebalancer/ growth=10>
>> </Proxy>
> 
> Or
> 
> <Proxy balancer://somebalancer/>
> ProxySet growth=10
> </Proxy>

FYI: Travis trunk also fails almost completely. Does not seem to accept a proxy configuration.

> 
> Regards
> 
> Rüdiger


Re: svn commit: r1899390 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h

Posted by Ruediger Pluem <rp...@apache.org>.

On 3/31/22 11:11 AM, Ruediger Pluem wrote:
> 
> 
> On 3/30/22 4:42 PM, jfclere@apache.org wrote:
>> Author: jfclere
>> Date: Wed Mar 30 14:42:14 2022
>> New Revision: 1899390
>>
>> URL: http://svn.apache.org/viewvc?rev=1899390&view=rev
>> Log:
>> Add WorkerBalancerGrowth. To allow creation of workers
>> to dynamically added balancers.
>>
>> Modified:
>>     httpd/httpd/trunk/CHANGES
>>     httpd/httpd/trunk/modules/proxy/mod_proxy.c
>>     httpd/httpd/trunk/modules/proxy/mod_proxy.h
>>
>> Modified: httpd/httpd/trunk/CHANGES
>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1899390&r1=1899389&r2=1899390&view=diff
>> ==============================================================================
>> --- httpd/httpd/trunk/CHANGES [utf-8] (original)
>> +++ httpd/httpd/trunk/CHANGES [utf-8] Wed Mar 30 14:42:14 2022
>> @@ -1,6 +1,10 @@
>>                                                           -*- coding: utf-8 -*-
>>  Changes with Apache 2.5.1
>>  
>> +  *) mod_proxy: Add WorkerBalancerGrowth to allow adding workers to
>> +     balancer created dynamically or via "empty" <Proxy balancer://../>
>> +     [Jean-Frederic Clere]
> 
> I am not sure why this is needed. You can already do this via
> 
> <Proxy balancer://somebalancer/ growth=10>
> </Proxy>

Or

<Proxy balancer://somebalancer/>
ProxySet growth=10
</Proxy>

Regards

Rüdiger

Re: svn commit: r1899390 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h

Posted by Ruediger Pluem <rp...@apache.org>.

On 3/30/22 4:42 PM, jfclere@apache.org wrote:
> Author: jfclere
> Date: Wed Mar 30 14:42:14 2022
> New Revision: 1899390
> 
> URL: http://svn.apache.org/viewvc?rev=1899390&view=rev
> Log:
> Add WorkerBalancerGrowth. To allow creation of workers
> to dynamically added balancers.
> 
> Modified:
>     httpd/httpd/trunk/CHANGES
>     httpd/httpd/trunk/modules/proxy/mod_proxy.c
>     httpd/httpd/trunk/modules/proxy/mod_proxy.h
> 
> Modified: httpd/httpd/trunk/CHANGES
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1899390&r1=1899389&r2=1899390&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/CHANGES [utf-8] (original)
> +++ httpd/httpd/trunk/CHANGES [utf-8] Wed Mar 30 14:42:14 2022
> @@ -1,6 +1,10 @@
>                                                           -*- coding: utf-8 -*-
>  Changes with Apache 2.5.1
>  
> +  *) mod_proxy: Add WorkerBalancerGrowth to allow adding workers to
> +     balancer created dynamically or via "empty" <Proxy balancer://../>
> +     [Jean-Frederic Clere]

I am not sure why this is needed. You can already do this via

<Proxy balancer://somebalancer/ growth=10>
</Proxy>

Furthermore WorkerBalancerGrowth is inconsistent as the growth parameter I mention above only allows 100 as maximum and
WorkerBalancerGrowth allows 1000.

Regards

Rüdiger