[prev in list] [next in list] [prev in thread] [next in thread] 

List:       apache-httpd-dev
Subject:    Re: svn commit: r1899390 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy.c modules/proxy/mo
From:       jean-frederic clere <jfclere () gmail ! com>
Date:       2022-04-05 7:42:54
Message-ID: b50a4fd5-063d-c960-bde7-48cd8ac53ac8 () gmail ! com
[Download RAW message or body]

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


[prev in list] [next in list] [prev in thread] [next in thread]