Re: svn commit: r354779 - /httpd/httpd/branches/2.2.x/STATUS

2005-12-07 Thread Justin Erenkrantz
On Wed, Dec 07, 2005 at 03:48:39PM -, [EMAIL PROTECTED] wrote:
 +* proxy_util: Fix case where a shared keepalive connection results in
 +  different (and incorrect) workers from being accessed.
 +  
 http://svn.apache.org/viewcvs.cgi/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=350250view=diffr1=350250r2=332306
 +  +1: jim

In reviewing this, I can't for the life of me figure out why there's an if
and else check that has the exact same body.  Can't we just do:

if (!conn-hostname || !worker-is_address_reusable ||
(r-connection-keepalives 
 (r-proxyreq == PROXYREQ_PROXY || r-proxyreq == PROXYREQ_REVERSE) 
 (strcasecmp(conn-hostname, uri-hostname) != 0))) {
   
}

With a conditional like this, it'd probably be nice to have the conditional
tests in a comment so that we can understand the check that it's doing.  =)

Or, am I missing something?  -- justin


Re: svn commit: r354779 - /httpd/httpd/branches/2.2.x/STATUS

2005-12-07 Thread Jim Jagielski
Justin Erenkrantz wrote:
 
 On Wed, Dec 07, 2005 at 03:48:39PM -, [EMAIL PROTECTED] wrote:
  +* proxy_util: Fix case where a shared keepalive connection results in
  +  different (and incorrect) workers from being accessed.
  +  
  http://svn.apache.org/viewcvs.cgi/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=350250view=diffr1=350250r2=332306
  +  +1: jim
 
 In reviewing this, I can't for the life of me figure out why there's an if
 and else check that has the exact same body.  Can't we just do:
 
 if (!conn-hostname || !worker-is_address_reusable ||
 (r-connection-keepalives 
  (r-proxyreq == PROXYREQ_PROXY || r-proxyreq == PROXYREQ_REVERSE) 
  (strcasecmp(conn-hostname, uri-hostname) != 0))) {

 }
 

The sole reason was the keep the present setup, so that if
is_address_reusable becomes more accurate we don't loose
information on what was the original intent... I'd also
prefer making it one large conditional as well, but
others were looking at the current logic and I didn't
want to change too much :)

-- 
===
 Jim Jagielski   [|]   [EMAIL PROTECTED]   [|]   http://www.jaguNET.com/
   If you can dodge a wrench, you can dodge a ball.


Re: svn commit: r354779 - /httpd/httpd/branches/2.2.x/STATUS

2005-12-07 Thread Justin Erenkrantz
On Wed, Dec 07, 2005 at 11:29:57AM -0500, Jim Jagielski wrote:
 The sole reason was the keep the present setup, so that if
 is_address_reusable becomes more accurate we don't loose
 information on what was the original intent... I'd also

Can you please elaborate on that?  Thanks.  -- justin


Re: svn commit: r354779 - /httpd/httpd/branches/2.2.x/STATUS

2005-12-07 Thread Jim Jagielski
Jim Jagielski wrote:
 
 Justin Erenkrantz wrote:
  
  On Wed, Dec 07, 2005 at 03:48:39PM -, [EMAIL PROTECTED] wrote:
   +* proxy_util: Fix case where a shared keepalive connection results in
   +  different (and incorrect) workers from being accessed.
   +  
   http://svn.apache.org/viewcvs.cgi/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=350250view=diffr1=350250r2=332306
   +  +1: jim
  
  In reviewing this, I can't for the life of me figure out why there's an if
  and else check that has the exact same body.  Can't we just do:
  
  if (!conn-hostname || !worker-is_address_reusable ||
  (r-connection-keepalives 
   (r-proxyreq == PROXYREQ_PROXY || r-proxyreq == PROXYREQ_REVERSE) 
   (strcasecmp(conn-hostname, uri-hostname) != 0))) {
 
  }
  
 
 The sole reason was the keep the present setup, so that if
 is_address_reusable becomes more accurate we don't loose
 information on what was the original intent... I'd also
 prefer making it one large conditional as well, but
 others were looking at the current logic and I didn't
 want to change too much :)
 

PS: Unless add't work is done on this section within the next
few days, I'll clean up the whole section (in trunk)
to streamline things... 

-- 
===
 Jim Jagielski   [|]   [EMAIL PROTECTED]   [|]   http://www.jaguNET.com/
   If you can dodge a wrench, you can dodge a ball.


Re: svn commit: r354779 - /httpd/httpd/branches/2.2.x/STATUS

2005-12-07 Thread Jim Jagielski
Justin Erenkrantz wrote:
 
 On Wed, Dec 07, 2005 at 11:29:57AM -0500, Jim Jagielski wrote:
  The sole reason was the keep the present setup, so that if
  is_address_reusable becomes more accurate we don't loose
  information on what was the original intent... I'd also
 
 Can you please elaborate on that?  Thanks.  -- justin
 

Sure... Right now, there appears to be some questions on
why we are treating some requests differently and how
that affects the pool. Ruediger was looking into this,
with the end result that some areas of this, such as
what warrants a reusable connection, may be changed.
I did not want to rework what was the original logic
layout too much, just simply fix a quick problem.
The old code has the same if-else-if structure, and
I didn't want to disturb too much since Ruediger stated
that this was the exact section he was playing
around with.

With the entry in STATUS I just didn't want the current
misbehavior to continue in 2.2...

-- 
===
 Jim Jagielski   [|]   [EMAIL PROTECTED]   [|]   http://www.jaguNET.com/
   If you can dodge a wrench, you can dodge a ball.


AW: svn commit: r354779 - /httpd/httpd/branches/2.2.x/STATUS

2005-12-07 Thread Plüm , Rüdiger , VIS


 -Ursprüngliche Nachricht-
 Von: Jim Jagielski 
 Gesendet: Mittwoch, 7. Dezember 2005 17:43
 An: Justin Erenkrantz
 Cc: dev@httpd.apache.org; [EMAIL PROTECTED]
 Betreff: Re: svn commit: r354779 - /httpd/httpd/branches/2.2.x/STATUS

 
 Sure... Right now, there appears to be some questions on
 why we are treating some requests differently and how
 that affects the pool. Ruediger was looking into this,
 with the end result that some areas of this, such as
 what warrants a reusable connection, may be changed.
 I did not want to rework what was the original logic
 layout too much, just simply fix a quick problem.
 The old code has the same if-else-if structure, and
 I didn't want to disturb too much since Ruediger stated
 that this was the exact section he was playing
 around with.

Yes, that was the area I wanted to play, but I had not found time so far :-(.
So if you already have ideas on this feel free to move forward
as I do not want to block progress due to this temporary lack of time
on my side. I am sure it is possible for both of us to work on
this as we share our ideas on the list anyway.

Regards

Rüdiger

[..cut..]


Re: AW: svn commit: r354779 - /httpd/httpd/branches/2.2.x/STATUS

2005-12-07 Thread Jim Jagielski
=?iso-8859-1?Q?Pl=FCm=2C_R=FCdiger=2C_VIS?= wrote:
 
 
 
  -Urspr=FCngliche Nachricht-
  Von: Jim Jagielski=20
  Gesendet: Mittwoch, 7. Dezember 2005 17:43
  An: Justin Erenkrantz
  Cc: dev@httpd.apache.org; [EMAIL PROTECTED]
  Betreff: Re: svn commit: r354779 - /httpd/httpd/branches/2.2.x/STATUS
 
 =20
  Sure... Right now, there appears to be some questions on
  why we are treating some requests differently and how
  that affects the pool. Ruediger was looking into this,
  with the end result that some areas of this, such as
  what warrants a reusable connection, may be changed.
  I did not want to rework what was the original logic
  layout too much, just simply fix a quick problem.
  The old code has the same if-else-if structure, and
  I didn't want to disturb too much since Ruediger stated
  that this was the exact section he was playing
  around with.
 
 Yes, that was the area I wanted to play, but I had not found time so far =
 :-(.
 So if you already have ideas on this feel free to move forward
 as I do not want to block progress due to this temporary lack of time
 on my side. I am sure it is possible for both of us to work on
 this as we share our ideas on the list anyway.
 

OK, great... what I'll do is clean up what we *have* and
then we can work on what we *want* :)

-- 
===
 Jim Jagielski   [|]   [EMAIL PROTECTED]   [|]   http://www.jaguNET.com/
   If you can dodge a wrench, you can dodge a ball.