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

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

Reply via email to