Re: svn commit: r354779 - /httpd/httpd/branches/2.2.x/STATUS
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
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
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
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
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
-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
=?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.