mod_proxy patches

[Mathias Herberts <ma...@gmail.com>]

Hi,

back in october I submitted several patches to mod_proxy to address
some issues I had. At the time I was asked to resubmit my patched
against trunk instead of branch 2.2. I did not find the time then to
redo the work but at last I've been able to dedicate some time to this
task.

Included in this message are therefore three patches.


* stickysessionpathid.patch

This patch adds a balancer option 'stickysessionpathid' which allows
to set the name to look for in the request path. This is needed for
example when dealing with servlet containers as backend as the servlet
spec defines the name of the session cookie to be 'JSESSIONID' but the
name of the path component to be ';jsessionid'. Without this patch
Apache can only detect one of the two occurences.

The value set with 'stickysessionpathid' is only looked for in the
path, not in the cookie. If the token is not found in the path, the
value of 'stickysession' is looked for (if it was defined).

* forceclose.patch

This patch adds a worker option 'forceclose' which can be set to 'On'
or 'Off' and which specifies if the backend connection should be
closed upon request completion.

Being able to force the closing of a backend connection (AJP or HTTP,
SSL connections are already closed after request completion) is
important as Apache has a multi-process architecture which could lead
to connection exhaustion to a backend if all connections to it are
already held by processes currently serving another backend.

* balancer-manager-status.patch

This patch modifies the balancer-manager page so the status can be
set/unset to Disabled/Stopped/Hot-StandBy/In-Error.

I submit this patch without being really convinced the setting of
parameters through the balancer-manager page is really useful when
Apache is operating in multi processes mode, the parameters
modification being only visible locally in the process having served
the modification request.


Any feedback would be appreciated.

Regards,

Mathias.

Re: mod_proxy patches

[Mladen Turk <mt...@apache.org>]

Mathias Herberts wrote:
> 
> The 'forceclose' patch will ensure that this situation will never
> happen as the Cmax connections will not be kept idle. It will have a
> slight performance penalty but that's better than a total inability to
> serve requests (which should only happen when T0 is really serving
> Cmax concurrent requests).
>

This will drop the protocol to AJP1.2, and there is no advantage
over http protocol in that case if you need to reconnect all the
time. If AJP 1.3 protocol doesn't fit use the http protocol instead
forcing AJP 1.3 to behave like it shouldn't.

Regards,
Mladen.

Re: mod_proxy patches

[Mladen Turk <mt...@apache.org>]

Jim Jagielski wrote:
> 
> On Mar 16, 2007, at 8:52 AM, Mathias Herberts wrote:
> 
>>
>> I agree that reusing the backend connections can be a good thing, but
>> there are times when this is just not a very good idea.
>>
> 
> I agree that there are times when having a single-shot
> connection is "better" than having a pool.

I agree with you, but in that case, using a http instead ajp is
a better alternative. Unlike with mod_jk, with mod_proxy we have
an option to choose the appropriate protocol.

Regards,
Mladen.

Re: mod_proxy patches

[Jim Jagielski <ji...@jaguNET.com>]

On Mar 16, 2007, at 8:52 AM, Mathias Herberts wrote:

>
> I agree that reusing the backend connections can be a good thing, but
> there are times when this is just not a very good idea.
>

I agree that there are times when having a single-shot
connection is "better" than having a pool. It's
certainly not a normal situation but for those people
affected by this, it's a deal-breaker.

Re: mod_proxy patches

[Mathias Herberts <ma...@gmail.com>]

On 3/16/07, Jean-Frederic <jf...@gmail.com> wrote:

[snip]

> >
> > * forceclose.patch
> >
> > This patch adds a worker option 'forceclose' which can be set to 'On'
> > or 'Off' and which specifies if the backend connection should be
> > closed upon request completion.
> >
> > Being able to force the closing of a backend connection (AJP or HTTP,
> > SSL connections are already closed after request completion) is
> > important as Apache has a multi-process architecture which could lead
> > to connection exhaustion to a backend if all connections to it are
> > already held by processes currently serving another backend.
>
> Well the good thing is that we reuse the backend connections. Anyway
> could be goof for testing +1.

I agree that reusing the backend connections can be a good thing, but
there are times when this is just not a very good idea.

Imagine a front-end Apache that connects to several Tomcat using AJP
or HTTP (via mod_proxy_balancer or mod_jk, the problem is the same).
If each Tomcat has a maximum number of connections set to Cmax then
after each of them reaches this number no more connections can be
accepted and thus no more requests taken care of.

Imagine that right after startup Apache receives Cmax concurrent
requests that should be served by the first Tomcat (T0). Cmax child
processes will be spawned and each of them will connect to T0 thus
creating Cmax connections.

After those requests have been taken care of, Apache sits idle for a
while, a while too short for any child process to be killed.

After this idle period, Cmax concurrent requests arrive for the second
Tomcat (T1). The Cmax existing child processes will be used to serve
those requests and Cmax connections to T1 will be created. So far so
good.

But what happens now if a single request supposed to be handled by T0
arrives. Apache will create a new child process and will attempt to
connect it to T0. But as the Cmax first child processes each hold a
connection to Cmax, that connection attempt will fail and that single
request for T0 will not be served.

The 'forceclose' patch will ensure that this situation will never
happen as the Cmax connections will not be kept idle. It will have a
slight performance penalty but that's better than a total inability to
serve requests (which should only happen when T0 is really serving
Cmax concurrent requests).

Mathias.


>
> >
> > * balancer-manager-status.patch
> >
> > This patch modifies the balancer-manager page so the status can be
> > set/unset to Disabled/Stopped/Hot-StandBy/In-Error.
> >
> > I submit this patch without being really convinced the setting of
> > parameters through the balancer-manager page is really useful when
> > Apache is operating in multi processes mode, the parameters
> > modification being only visible locally in the process having served
> > the modification request.
>
> worker->s is in scoreboard so it will be visible to all child processes.
> But it is not protected by a lock. I also not sure it a good idea to
> PROXY_WORKER_IN_ERROR or !PROXY_WORKER_IN_ERROR  as logic of mod_proxy
> handles this already.
>
> Cheers
>
> Jean-Frederic
>
> >
> >
> > Any feedback would be appreciated.
> >
> > Regards,
> >
> > Mathias.
>
>

Re: mod_proxy patches

[Jean-Frederic <jf...@gmail.com>]

On Fri, 2007-03-16 at 12:21 +0100, Mathias Herberts wrote:
> Hi,
> 
> back in october I submitted several patches to mod_proxy to address
> some issues I had. At the time I was asked to resubmit my patched
> against trunk instead of branch 2.2. I did not find the time then to
> redo the work but at last I've been able to dedicate some time to this
> task.
> 
> Included in this message are therefore three patches.
> 
> 
> * stickysessionpathid.patch
> 
> This patch adds a balancer option 'stickysessionpathid' which allows
> to set the name to look for in the request path. This is needed for
> example when dealing with servlet containers as backend as the servlet
> spec defines the name of the session cookie to be 'JSESSIONID' but the
> name of the path component to be ';jsessionid'. Without this patch
> Apache can only detect one of the two occurences.
> 
> The value set with 'stickysessionpathid' is only looked for in the
> path, not in the cookie. If the token is not found in the path, the
> value of 'stickysession' is looked for (if it was defined).

Would it make sense to search for both upper and lower case in
get_path_param()?

> 
> * forceclose.patch
> 
> This patch adds a worker option 'forceclose' which can be set to 'On'
> or 'Off' and which specifies if the backend connection should be
> closed upon request completion.
> 
> Being able to force the closing of a backend connection (AJP or HTTP,
> SSL connections are already closed after request completion) is
> important as Apache has a multi-process architecture which could lead
> to connection exhaustion to a backend if all connections to it are
> already held by processes currently serving another backend.

Well the good thing is that we reuse the backend connections. Anyway
could be goof for testing +1.

> 
> * balancer-manager-status.patch
> 
> This patch modifies the balancer-manager page so the status can be
> set/unset to Disabled/Stopped/Hot-StandBy/In-Error.
> 
> I submit this patch without being really convinced the setting of
> parameters through the balancer-manager page is really useful when
> Apache is operating in multi processes mode, the parameters
> modification being only visible locally in the process having served
> the modification request.

worker->s is in scoreboard so it will be visible to all child processes.
But it is not protected by a lock. I also not sure it a good idea to
PROXY_WORKER_IN_ERROR or !PROXY_WORKER_IN_ERROR  as logic of mod_proxy
handles this already.

Cheers

Jean-Frederic

> 
> 
> Any feedback would be appreciated.
> 
> Regards,
> 
> Mathias.