You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Daniel Ruggeri <DR...@primary.net> on 2013/08/09 22:51:20 UTC

Fixing UDS in trunk/2.4 proposal

So I'm tasked with making httpd hold its own weight better against nginx
as a reverse proxy to a local service. Unfortunately, nginx supports UDS
and we don't quite yet. I've come across a bug that seems easy enough to
fix, but I am wondering if there's a better way. Thoughts are welcome.

With the currently proposed UDS support in mod_proxy, first requests
always seem fine but subsequent requests for the worker fail (attempted
DNS lookup when none should be done). I wouldn't have +1'ed had I
realized this at the time, but I have a proposed fix...

--- httpd-2.4.6-UDS/modules/proxy/proxy_util.c  2013-08-09
15:12:23.000000000 -0500
+++ httpd-2.4.6/modules/proxy/proxy_util.c      2013-08-09
15:15:33.000000000 -0500
@@ -2127,7 +2127,7 @@
      */

     if (!conn->hostname || !worker->s->is_address_reusable ||
-        worker->s->disablereuse || strncmp(conn->hostname, "socket=",
7) == 0) {
+        worker->s->disablereuse) {
         if (proxyname) {
             conn->hostname = apr_pstrdup(conn->pool, proxyname);
             conn->port = proxyport;

I haven't spent a ton of time on this so I'm wondering... This seems
simple enough, but isn't there a place we could do this once to avoid
having to execute the same logic (substring and path decode) on all
subsequent requests? I'd also much rather not have to do a string
comparison on all subsequent hits...

If I don't hear anything otherwise, I'll just commit this and add it to
the backport proposal next week or so


P.S.
My simple tests with 100 concurrent users for a total of 1,000,000
requests yielded the following numbers (with the above patch applied).
The backend supports about 20k requests/sec. This seems to be a mighty
compelling case for UDS...

nginx UDS: 16001.28
nginx UDS: 18138.94
nginx UDS: 15499.64

Apache UDS: 16348.70
Apache UDS: 14580.92
Apache UDS: 15211.97

Apache TCP: 11859.46
(only got one in)

-- 
Daniel Ruggeri


Re: Fixing UDS in trunk/2.4 proposal

Posted by Jim Jagielski <ji...@jaguNET.com>.
On Aug 11, 2013, at 8:30 AM, Jeff Trawick <tr...@gmail.com> wrote:

> 
> Another thought...  Socket paths should be passed through  ap_runtime_dir_relative(), right?
> 

Hmmm... I never considered that, but it makes sense.

Re: Fixing UDS in trunk/2.4 proposal

Posted by Jeff Trawick <tr...@gmail.com>.
On Sat, Aug 10, 2013 at 12:30 PM, Jeff Trawick <tr...@gmail.com> wrote:

> On Sat, Aug 10, 2013 at 11:32 AM, Jim Jagielski <ji...@jagunet.com> wrote:
>
>> +1... By the way, I'm working on a minor patch that works around
>> that "stupid" encoding of '/' requirement...
>>
>
> Did you give any thought to bypassing the normal proxy parsing altogether?
>
> For mod_authnz_fcgi I started by using a copy of ap_proxy_canon_netloc()
> to parse the backend address, but I think I'll just use a few regexes
> (literal IPv6 address+port, any other address+port, and eventually socket=
> when AF_UNIX is implemented).  I don't have access to proxy APIs and
> there's just too much baggage with what ap_proxy_canon_netloc() has to do
> for proxy to bring it along only for fcgi addresses.
>

Another thought...  Socket paths should be passed through
 ap_runtime_dir_relative(), right?

>
>
>
>> On Fri, Aug 09, 2013 at 03:51:20PM -0500, Daniel Ruggeri wrote:
>> > So I'm tasked with making httpd hold its own weight better against nginx
>> > as a reverse proxy to a local service. Unfortunately, nginx supports UDS
>> > and we don't quite yet. I've come across a bug that seems easy enough to
>> > fix, but I am wondering if there's a better way. Thoughts are welcome.
>> >
>> > With the currently proposed UDS support in mod_proxy, first requests
>> > always seem fine but subsequent requests for the worker fail (attempted
>> > DNS lookup when none should be done). I wouldn't have +1'ed had I
>> > realized this at the time, but I have a proposed fix...
>> >
>> > --- httpd-2.4.6-UDS/modules/proxy/proxy_util.c  2013-08-09
>> > 15:12:23.000000000 -0500
>> > +++ httpd-2.4.6/modules/proxy/proxy_util.c      2013-08-09
>> > 15:15:33.000000000 -0500
>> > @@ -2127,7 +2127,7 @@
>> >       */
>> >
>> >      if (!conn->hostname || !worker->s->is_address_reusable ||
>> > -        worker->s->disablereuse || strncmp(conn->hostname, "socket=",
>> > 7) == 0) {
>> > +        worker->s->disablereuse) {
>> >          if (proxyname) {
>> >              conn->hostname = apr_pstrdup(conn->pool, proxyname);
>> >              conn->port = proxyport;
>> >
>> > I haven't spent a ton of time on this so I'm wondering... This seems
>> > simple enough, but isn't there a place we could do this once to avoid
>> > having to execute the same logic (substring and path decode) on all
>> > subsequent requests? I'd also much rather not have to do a string
>> > comparison on all subsequent hits...
>> >
>> > If I don't hear anything otherwise, I'll just commit this and add it to
>> > the backport proposal next week or so
>> >
>> >
>> > P.S.
>> > My simple tests with 100 concurrent users for a total of 1,000,000
>> > requests yielded the following numbers (with the above patch applied).
>> > The backend supports about 20k requests/sec. This seems to be a mighty
>> > compelling case for UDS...
>> >
>> > nginx UDS: 16001.28
>> > nginx UDS: 18138.94
>> > nginx UDS: 15499.64
>> >
>> > Apache UDS: 16348.70
>> > Apache UDS: 14580.92
>> > Apache UDS: 15211.97
>> >
>> > Apache TCP: 11859.46
>> > (only got one in)
>> >
>> > --
>> > Daniel Ruggeri
>>
>> --
>>
>> ===========================================================================
>>    Jim Jagielski   [|]   jim@jaguNET.com   [|]   http://www.jaguNET.com/
>>         "Great is the guilt of an unnecessary war"  ~ John Adams
>>
>
>
>
> --
> Born in Roswell... married an alien...
> http://emptyhammock.com/
>



-- 
Born in Roswell... married an alien...
http://emptyhammock.com/

Re: Fixing UDS in trunk/2.4 proposal

Posted by Jim Jagielski <ji...@jaguNET.com>.
On Aug 10, 2013, at 12:30 PM, Jeff Trawick <tr...@gmail.com> wrote:

> On Sat, Aug 10, 2013 at 11:32 AM, Jim Jagielski <ji...@jagunet.com> wrote:
> +1... By the way, I'm working on a minor patch that works around
> that "stupid" encoding of '/' requirement...
> 
> Did you give any thought to bypassing the normal proxy parsing altogether?

Yeah... 
> 
> For mod_authnz_fcgi I started by using a copy of ap_proxy_canon_netloc() to parse the backend address, but I think I'll just use a few regexes (literal IPv6 address+port, any other address+port, and eventually socket= when AF_UNIX is implemented).  I don't have access to proxy APIs and there's just too much baggage with what ap_proxy_canon_netloc() has to do for proxy to bring it along only for fcgi addresses.
> 
> 
> 
> On Fri, Aug 09, 2013 at 03:51:20PM -0500, Daniel Ruggeri wrote:
> > So I'm tasked with making httpd hold its own weight better against nginx
> > as a reverse proxy to a local service. Unfortunately, nginx supports UDS
> > and we don't quite yet. I've come across a bug that seems easy enough to
> > fix, but I am wondering if there's a better way. Thoughts are welcome.
> >
> > With the currently proposed UDS support in mod_proxy, first requests
> > always seem fine but subsequent requests for the worker fail (attempted
> > DNS lookup when none should be done). I wouldn't have +1'ed had I
> > realized this at the time, but I have a proposed fix...
> >
> > --- httpd-2.4.6-UDS/modules/proxy/proxy_util.c  2013-08-09
> > 15:12:23.000000000 -0500
> > +++ httpd-2.4.6/modules/proxy/proxy_util.c      2013-08-09
> > 15:15:33.000000000 -0500
> > @@ -2127,7 +2127,7 @@
> >       */
> >
> >      if (!conn->hostname || !worker->s->is_address_reusable ||
> > -        worker->s->disablereuse || strncmp(conn->hostname, "socket=",
> > 7) == 0) {
> > +        worker->s->disablereuse) {
> >          if (proxyname) {
> >              conn->hostname = apr_pstrdup(conn->pool, proxyname);
> >              conn->port = proxyport;
> >
> > I haven't spent a ton of time on this so I'm wondering... This seems
> > simple enough, but isn't there a place we could do this once to avoid
> > having to execute the same logic (substring and path decode) on all
> > subsequent requests? I'd also much rather not have to do a string
> > comparison on all subsequent hits...
> >
> > If I don't hear anything otherwise, I'll just commit this and add it to
> > the backport proposal next week or so
> >
> >
> > P.S.
> > My simple tests with 100 concurrent users for a total of 1,000,000
> > requests yielded the following numbers (with the above patch applied).
> > The backend supports about 20k requests/sec. This seems to be a mighty
> > compelling case for UDS...
> >
> > nginx UDS: 16001.28
> > nginx UDS: 18138.94
> > nginx UDS: 15499.64
> >
> > Apache UDS: 16348.70
> > Apache UDS: 14580.92
> > Apache UDS: 15211.97
> >
> > Apache TCP: 11859.46
> > (only got one in)
> >
> > --
> > Daniel Ruggeri
> 
> --
> ===========================================================================
>    Jim Jagielski   [|]   jim@jaguNET.com   [|]   http://www.jaguNET.com/
>         "Great is the guilt of an unnecessary war"  ~ John Adams
> 
> 
> 
> -- 
> Born in Roswell... married an alien...
> http://emptyhammock.com/


Re: Fixing UDS in trunk/2.4 proposal

Posted by Jeff Trawick <tr...@gmail.com>.
On Sat, Aug 10, 2013 at 11:32 AM, Jim Jagielski <ji...@jagunet.com> wrote:

> +1... By the way, I'm working on a minor patch that works around
> that "stupid" encoding of '/' requirement...
>

Did you give any thought to bypassing the normal proxy parsing altogether?

For mod_authnz_fcgi I started by using a copy of ap_proxy_canon_netloc() to
parse the backend address, but I think I'll just use a few regexes (literal
IPv6 address+port, any other address+port, and eventually socket= when
AF_UNIX is implemented).  I don't have access to proxy APIs and there's
just too much baggage with what ap_proxy_canon_netloc() has to do for proxy
to bring it along only for fcgi addresses.



> On Fri, Aug 09, 2013 at 03:51:20PM -0500, Daniel Ruggeri wrote:
> > So I'm tasked with making httpd hold its own weight better against nginx
> > as a reverse proxy to a local service. Unfortunately, nginx supports UDS
> > and we don't quite yet. I've come across a bug that seems easy enough to
> > fix, but I am wondering if there's a better way. Thoughts are welcome.
> >
> > With the currently proposed UDS support in mod_proxy, first requests
> > always seem fine but subsequent requests for the worker fail (attempted
> > DNS lookup when none should be done). I wouldn't have +1'ed had I
> > realized this at the time, but I have a proposed fix...
> >
> > --- httpd-2.4.6-UDS/modules/proxy/proxy_util.c  2013-08-09
> > 15:12:23.000000000 -0500
> > +++ httpd-2.4.6/modules/proxy/proxy_util.c      2013-08-09
> > 15:15:33.000000000 -0500
> > @@ -2127,7 +2127,7 @@
> >       */
> >
> >      if (!conn->hostname || !worker->s->is_address_reusable ||
> > -        worker->s->disablereuse || strncmp(conn->hostname, "socket=",
> > 7) == 0) {
> > +        worker->s->disablereuse) {
> >          if (proxyname) {
> >              conn->hostname = apr_pstrdup(conn->pool, proxyname);
> >              conn->port = proxyport;
> >
> > I haven't spent a ton of time on this so I'm wondering... This seems
> > simple enough, but isn't there a place we could do this once to avoid
> > having to execute the same logic (substring and path decode) on all
> > subsequent requests? I'd also much rather not have to do a string
> > comparison on all subsequent hits...
> >
> > If I don't hear anything otherwise, I'll just commit this and add it to
> > the backport proposal next week or so
> >
> >
> > P.S.
> > My simple tests with 100 concurrent users for a total of 1,000,000
> > requests yielded the following numbers (with the above patch applied).
> > The backend supports about 20k requests/sec. This seems to be a mighty
> > compelling case for UDS...
> >
> > nginx UDS: 16001.28
> > nginx UDS: 18138.94
> > nginx UDS: 15499.64
> >
> > Apache UDS: 16348.70
> > Apache UDS: 14580.92
> > Apache UDS: 15211.97
> >
> > Apache TCP: 11859.46
> > (only got one in)
> >
> > --
> > Daniel Ruggeri
>
> --
> ===========================================================================
>    Jim Jagielski   [|]   jim@jaguNET.com   [|]   http://www.jaguNET.com/
>         "Great is the guilt of an unnecessary war"  ~ John Adams
>



-- 
Born in Roswell... married an alien...
http://emptyhammock.com/

Re: Fixing UDS in trunk/2.4 proposal

Posted by Jim Jagielski <ji...@jaguNET.com>.
+1... By the way, I'm working on a minor patch that works around
that "stupid" encoding of '/' requirement...

On Fri, Aug 09, 2013 at 03:51:20PM -0500, Daniel Ruggeri wrote:
> So I'm tasked with making httpd hold its own weight better against nginx
> as a reverse proxy to a local service. Unfortunately, nginx supports UDS
> and we don't quite yet. I've come across a bug that seems easy enough to
> fix, but I am wondering if there's a better way. Thoughts are welcome.
> 
> With the currently proposed UDS support in mod_proxy, first requests
> always seem fine but subsequent requests for the worker fail (attempted
> DNS lookup when none should be done). I wouldn't have +1'ed had I
> realized this at the time, but I have a proposed fix...
> 
> --- httpd-2.4.6-UDS/modules/proxy/proxy_util.c  2013-08-09
> 15:12:23.000000000 -0500
> +++ httpd-2.4.6/modules/proxy/proxy_util.c      2013-08-09
> 15:15:33.000000000 -0500
> @@ -2127,7 +2127,7 @@
>       */
> 
>      if (!conn->hostname || !worker->s->is_address_reusable ||
> -        worker->s->disablereuse || strncmp(conn->hostname, "socket=",
> 7) == 0) {
> +        worker->s->disablereuse) {
>          if (proxyname) {
>              conn->hostname = apr_pstrdup(conn->pool, proxyname);
>              conn->port = proxyport;
> 
> I haven't spent a ton of time on this so I'm wondering... This seems
> simple enough, but isn't there a place we could do this once to avoid
> having to execute the same logic (substring and path decode) on all
> subsequent requests? I'd also much rather not have to do a string
> comparison on all subsequent hits...
> 
> If I don't hear anything otherwise, I'll just commit this and add it to
> the backport proposal next week or so
> 
> 
> P.S.
> My simple tests with 100 concurrent users for a total of 1,000,000
> requests yielded the following numbers (with the above patch applied).
> The backend supports about 20k requests/sec. This seems to be a mighty
> compelling case for UDS...
> 
> nginx UDS: 16001.28
> nginx UDS: 18138.94
> nginx UDS: 15499.64
> 
> Apache UDS: 16348.70
> Apache UDS: 14580.92
> Apache UDS: 15211.97
> 
> Apache TCP: 11859.46
> (only got one in)
> 
> -- 
> Daniel Ruggeri

-- 
===========================================================================
   Jim Jagielski   [|]   jim@jaguNET.com   [|]   http://www.jaguNET.com/
        "Great is the guilt of an unnecessary war"  ~ John Adams