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