Mailing List Archive

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

> Am 31.03.2022 um 11:55 schrieb Ruediger Pluem <rpluem@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

On 3/31/22 12:34 PM, Stefan Eissing wrote:
>
>
>> Am 31.03.2022 um 11:55 schrieb Ruediger Pluem <rpluem@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

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>:
>>>
>>>
>>>
>>> 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

> Am 01.04.2022 um 08:47 schrieb jean-frederic clere <jfclere@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 <rpluem@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

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>:
>>>>
>>>>
>>>>
>>>> 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

On 31/03/2022 12:34, Stefan Eissing wrote:
>
>
>> Am 31.03.2022 um 11:55 schrieb Ruediger Pluem <rpluem@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

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 <rpluem@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

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 <rpluem@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

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

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 <jfclere@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>:
>>>>>
>>>>>
>>>>>
>>>>> 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

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
>> <mailto:jfclere@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
>>>>>> <mailto:rpluem@apache.org>>:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 3/31/22 11:11 AM, Ruediger Pluem wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 3/30/22 4:42 PM, jfclere@apache.org
>>>>>>> <mailto: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
>>>>>>>> <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