You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by co...@apache.org on 2014/12/20 16:56:17 UTC

svn commit: r1647009 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_proxy.xml docs/manual/mod/mod_proxy_fcgi.xml include/ap_mmn.h modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h modules/proxy/proxy_util.c

Author: covener
Date: Sat Dec 20 15:56:16 2014
New Revision: 1647009

URL: http://svn.apache.org/r1647009
Log:
Allow SetHandler+UDS+fcgi to take advantage of dedicated workers including
opting in to connection reuse and other proxy options (max=, etc).

adds 'enablereuse' proxyoption and a minor MMN bump to share
proxy_desocketfy outside of mod_proxy.c, which is required to
match workers to URLs.


Modified:
    httpd/httpd/trunk/CHANGES
    httpd/httpd/trunk/docs/manual/mod/mod_proxy.xml
    httpd/httpd/trunk/docs/manual/mod/mod_proxy_fcgi.xml
    httpd/httpd/trunk/include/ap_mmn.h
    httpd/httpd/trunk/modules/proxy/mod_proxy.c
    httpd/httpd/trunk/modules/proxy/mod_proxy.h
    httpd/httpd/trunk/modules/proxy/proxy_util.c

Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1647009&r1=1647008&r2=1647009&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Sat Dec 20 15:56:16 2014
@@ -1,6 +1,14 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.5.0
 
+  *) mod_proxy_fcgi: Enable UDS backends configured with SetHandler/RewriteRule
+     to opt-in to connection reuse and other Proxy options via explicitly
+     declared "proxy workers" (<Proxy unix:... enablereuse=on max=...) 
+     [Eric Covener]
+
+  *) mod_proxy: Add "enablereuse" option as the inverse of "disablereuse".
+     [Eric Covener]
+
   *) mod_proxy_fcgi: Enable opt-in to TCP connection reuse by explicitly
      setting proxy option disablereuse=off. [Eric Covener] PR 57378.
  

Modified: httpd/httpd/trunk/docs/manual/mod/mod_proxy.xml
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/docs/manual/mod/mod_proxy.xml?rev=1647009&r1=1647008&r2=1647009&view=diff
==============================================================================
--- httpd/httpd/trunk/docs/manual/mod/mod_proxy.xml (original)
+++ httpd/httpd/trunk/docs/manual/mod/mod_proxy.xml Sat Dec 20 15:56:16 2014
@@ -1020,6 +1020,12 @@ ProxyPass /mirror/foo http://backend.exa
     robin DNS. To disable connection pooling reuse,
     set this property value to <code>On</code>.
     </td></tr>
+    <tr><td>enablereuse</td>
+        <td>On</td>
+        <td>This is the inverse of 'disablereuse' above, provided as a
+        convenience for scheme handlers that require opt-in for connection
+        reuse (such as <module>mod_proxy_fcgi</module>).
+    </td></tr>
     <tr><td>flushpackets</td>
         <td>off</td>
         <td>Determines whether the proxy module will auto-flush the output

Modified: httpd/httpd/trunk/docs/manual/mod/mod_proxy_fcgi.xml
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/docs/manual/mod/mod_proxy_fcgi.xml?rev=1647009&r1=1647008&r2=1647009&view=diff
==============================================================================
--- httpd/httpd/trunk/docs/manual/mod/mod_proxy_fcgi.xml (original)
+++ httpd/httpd/trunk/docs/manual/mod/mod_proxy_fcgi.xml Sat Dec 20 15:56:16 2014
@@ -77,7 +77,7 @@
 
     <example><title>Single application instance, connection reuse</title>
     <highlight language="config">
-      ProxyPass /myapp/ fcgi://localhost:4000/ disablereuse=off
+      ProxyPass /myapp/ fcgi://localhost:4000/ enablereuse=on
       </highlight>
     </example>
 
@@ -87,7 +87,7 @@
     PHP-FPM is listening.  Connection pooling is enabled.</p>
     <example><title>PHP-FPM</title>
     <highlight language="config">
-      ProxyPassMatch ^/myapp/.*\.php(/.*)?$ fcgi://localhost:9000/var/www/ disablereuse=off
+      ProxyPassMatch ^/myapp/.*\.php(/.*)?$ fcgi://localhost:9000/var/www/ enablereuse=on
     </highlight>
     </example>
 

Modified: httpd/httpd/trunk/include/ap_mmn.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/include/ap_mmn.h?rev=1647009&r1=1647008&r2=1647009&view=diff
==============================================================================
--- httpd/httpd/trunk/include/ap_mmn.h (original)
+++ httpd/httpd/trunk/include/ap_mmn.h Sat Dec 20 15:56:16 2014
@@ -475,6 +475,7 @@
  *                         ap_have_so_reuseport to ap_listen.h.
  * 20140627.9 (2.5.0-dev)  Add cgi_pass_auth and AP_CGI_PASS_AUTH_* to 
  *                         core_dir_config
+ * 20140627.10 (2.5.0-dev) Add ap_proxy_de_socketfy to mod_proxy.h
  */
 
 #define MODULE_MAGIC_COOKIE 0x41503235UL /* "AP25" */
@@ -482,7 +483,7 @@
 #ifndef MODULE_MAGIC_NUMBER_MAJOR
 #define MODULE_MAGIC_NUMBER_MAJOR 20140627
 #endif
-#define MODULE_MAGIC_NUMBER_MINOR 9                 /* 0...n */
+#define MODULE_MAGIC_NUMBER_MINOR 10                 /* 0...n */
 
 /**
  * Determine if the server's current MODULE_MAGIC_NUMBER is at least a

Modified: httpd/httpd/trunk/modules/proxy/mod_proxy.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy.c?rev=1647009&r1=1647008&r2=1647009&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/mod_proxy.c (original)
+++ httpd/httpd/trunk/modules/proxy/mod_proxy.c Sat Dec 20 15:56:16 2014
@@ -174,6 +174,15 @@ static const char *set_worker_param(apr_
             return "DisableReuse must be On|Off";
         worker->s->disablereuse_set = 1;
     }
+    else if (!strcasecmp(key, "enablereuse")) {
+        if (!strcasecmp(val, "on"))
+            worker->s->disablereuse = 0;
+        else if (!strcasecmp(val, "off"))
+            worker->s->disablereuse = 1;
+        else
+            return "EnableReuse must be On|Off";
+        worker->s->disablereuse_set = 1;
+    }
     else if (!strcasecmp(key, "route")) {
         /* Worker route.
          */
@@ -1477,15 +1486,16 @@ static const char *
     return add_proxy(cmd, dummy, f1, r1, 1);
 }
 
-static char *de_socketfy(apr_pool_t *p, char *url)
+PROXY_DECLARE(char *) ap_proxy_de_socketfy(apr_pool_t *p, const char *url)
 {
     char *ptr;
     /*
      * We could be passed a URL during the config stage that contains
      * the UDS path... ignore it
      */
+    char *url_copy = apr_pstrdup(p, url);
     if (!strncasecmp(url, "unix:", 5) &&
-        ((ptr = ap_strchr(url, '|')) != NULL)) {
+        ((ptr = ap_strchr(url_copy, '|')) != NULL)) {
         /* move past the 'unix:...|' UDS path info */
         char *ret, *c;
 
@@ -1504,7 +1514,7 @@ static char *de_socketfy(apr_pool_t *p,
             return ret;
         }
     }
-    return url;
+    return url_copy;
 }
 
 static const char *
@@ -1598,7 +1608,7 @@ static const char *
     }
 
     new->fake = apr_pstrdup(cmd->pool, f);
-    new->real = apr_pstrdup(cmd->pool, de_socketfy(cmd->pool, r));
+    new->real = apr_pstrdup(cmd->pool, ap_proxy_de_socketfy(cmd->pool, r));
     new->flags = flags;
     if (use_regex) {
         new->regex = ap_pregcomp(cmd->pool, f, AP_REG_EXTENDED);
@@ -2141,7 +2151,7 @@ static const char *add_member(cmd_parms
     }
 
     /* Try to find existing worker */
-    worker = ap_proxy_get_worker(cmd->temp_pool, balancer, conf, de_socketfy(cmd->temp_pool, name));
+    worker = ap_proxy_get_worker(cmd->temp_pool, balancer, conf, ap_proxy_de_socketfy(cmd->temp_pool, name));
     if (!worker) {
         ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, cmd->server, APLOGNO(01147)
                      "Defining worker '%s' for balancer '%s'",
@@ -2227,7 +2237,7 @@ static const char *
         }
     }
     else {
-        worker = ap_proxy_get_worker(cmd->temp_pool, NULL, conf, de_socketfy(cmd->temp_pool, name));
+        worker = ap_proxy_get_worker(cmd->temp_pool, NULL, conf, ap_proxy_de_socketfy(cmd->temp_pool, name));
         if (!worker) {
             if (in_proxy_section) {
                 err = ap_proxy_define_worker(cmd->pool, &worker, NULL,
@@ -2369,7 +2379,7 @@ static const char *proxysection(cmd_parm
         }
         else {
             worker = ap_proxy_get_worker(cmd->temp_pool, NULL, sconf,
-                                         de_socketfy(cmd->temp_pool, (char*)conf->p));
+                                         ap_proxy_de_socketfy(cmd->temp_pool, (char*)conf->p));
             if (!worker) {
                 if (use_regex) {
                     err = ap_proxy_define_match_worker(cmd->pool, &worker, NULL,

Modified: httpd/httpd/trunk/modules/proxy/mod_proxy.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy.h?rev=1647009&r1=1647008&r2=1647009&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/mod_proxy.h (original)
+++ httpd/httpd/trunk/modules/proxy/mod_proxy.h Sat Dec 20 15:56:16 2014
@@ -1070,6 +1070,14 @@ int ap_proxy_lb_workers(void);
  */
 PROXY_DECLARE(apr_port_t) ap_proxy_port_of_scheme(const char *scheme);
 
+/**
+ * Strip a unix domain socket (UDS) prefix from the input URL
+ * @param p             pool to allocate result from
+ * @param url           a URL potentially prefixed with a UDS path
+ * @return              URL with the UDS prefix removed
+ */
+PROXY_DECLARE(char *) ap_proxy_de_socketfy(apr_pool_t *p, const char *url);
+
 extern module PROXY_DECLARE_DATA proxy_module;
 
 #endif /*MOD_PROXY_H*/

Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=1647009&r1=1647008&r2=1647009&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
+++ httpd/httpd/trunk/modules/proxy/proxy_util.c Sat Dec 20 15:56:16 2014
@@ -1567,6 +1567,8 @@ PROXY_DECLARE(proxy_worker *) ap_proxy_g
         return NULL;
     }
 
+    url = ap_proxy_de_socketfy(p, url);
+
     c = ap_strchr_c(url, ':');
     if (c == NULL || c[1] != '/' || c[2] != '/' || c[3] == '\0') {
         return NULL;
@@ -1969,6 +1971,40 @@ static int ap_proxy_retry_worker(const c
     }
 }
 
+/*
+ * In the case of the reverse proxy, we need to see if we
+ * were passed a UDS url (eg: from mod_proxy) and adjust uds_path
+ * as required.  
+ */
+static void fix_uds_filename(request_rec *r, char **url) 
+{
+    char *ptr, *ptr2;
+    if (!r || !r->filename) return;
+
+    if (!strncmp(r->filename, "proxy:", 6) &&
+            (ptr2 = ap_strcasestr(r->filename, "unix:")) &&
+            (ptr = ap_strchr(ptr2, '|'))) {
+        apr_uri_t urisock;
+        apr_status_t rv;
+        *ptr = '\0';
+        rv = apr_uri_parse(r->pool, ptr2, &urisock);
+        if (rv == APR_SUCCESS) {
+            char *rurl = ptr+1;
+            char *sockpath = ap_runtime_dir_relative(r->pool, urisock.path);
+            apr_table_setn(r->notes, "uds_path", sockpath);
+            *url = apr_pstrdup(r->pool, rurl); /* so we get the scheme for the uds */
+            /* r->filename starts w/ "proxy:", so add after that */
+            memmove(r->filename+6, rurl, strlen(rurl)+1);
+            ap_log_rerror(APLOG_MARK, APLOG_TRACE2, 0, r,
+                    "*: rewrite of url due to UDS(%s): %s (%s)",
+                    sockpath, *url, r->filename);
+        }
+        else {
+            *ptr = '|';
+        }
+    }
+}
+
 PROXY_DECLARE(int) ap_proxy_pre_request(proxy_worker **worker,
                                         proxy_balancer **balancer,
                                         request_rec *r,
@@ -1983,8 +2019,8 @@ PROXY_DECLARE(int) ap_proxy_pre_request(
             ap_log_rerror(APLOG_MARK, APLOG_TRACE2, 0, r,
                           "%s: found worker %s for %s",
                           (*worker)->s->scheme, (*worker)->s->name, *url);
-
             *balancer = NULL;
+            fix_uds_filename(r, url);
             access_status = OK;
         }
         else if (r->proxyreq == PROXYREQ_PROXY) {
@@ -2004,8 +2040,6 @@ PROXY_DECLARE(int) ap_proxy_pre_request(
         }
         else if (r->proxyreq == PROXYREQ_REVERSE) {
             if (conf->reverse) {
-                char *ptr;
-                char *ptr2;
                 ap_log_rerror(APLOG_MARK, APLOG_TRACE2, 0, r,
                               "*: using default reverse proxy worker for %s (no keepalive)", *url);
                 *balancer = NULL;
@@ -2017,33 +2051,7 @@ PROXY_DECLARE(int) ap_proxy_pre_request(
                  * regarding the Connection header in the request.
                  */
                 apr_table_setn(r->subprocess_env, "proxy-nokeepalive", "1");
-                /*
-                 * In the case of the generic reverse proxy, we need to see if we
-                 * were passed a UDS url (eg: from mod_proxy) and adjust uds_path
-                 * as required.
-                 */
-                if (!strncmp(r->filename, "proxy:", 6) &&
-                    (ptr2 = ap_strcasestr(r->filename, "unix:")) &&
-                    (ptr = ap_strchr(ptr2, '|'))) {
-                    apr_uri_t urisock;
-                    apr_status_t rv;
-                    *ptr = '\0';
-                    rv = apr_uri_parse(r->pool, ptr2, &urisock);
-                    if (rv == APR_SUCCESS) {
-                        char *rurl = ptr+1;
-                        char *sockpath = ap_runtime_dir_relative(r->pool, urisock.path);
-                        apr_table_setn(r->notes, "uds_path", sockpath);
-                        *url = apr_pstrdup(r->pool, rurl); /* so we get the scheme for the uds */
-                        /* r->filename starts w/ "proxy:", so add after that */
-                        memmove(r->filename+6, rurl, strlen(rurl)+1);
-                        ap_log_rerror(APLOG_MARK, APLOG_TRACE2, 0, r,
-                                      "*: rewrite of url due to UDS(%s): %s (%s)",
-                                      sockpath, *url, r->filename);
-                    }
-                    else {
-                        *ptr = '|';
-                    }
-                }
+                fix_uds_filename(r, url);
             }
         }
     }



Re: svn commit: r1647009 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_proxy.xml docs/manual/mod/mod_proxy_fcgi.xml include/ap_mmn.h modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h modules/proxy/proxy_util.c

Posted by Eric Covener <co...@gmail.com>.
On Mon, Dec 22, 2014 at 4:56 AM, Ruediger Pluem <rp...@apache.org> wrote:
>> Modified: httpd/httpd/trunk/modules/proxy/mod_proxy.c
>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy.c?rev=1647009&r1=1647008&r2=1647009&view=diff
>> ==============================================================================
>> --- httpd/httpd/trunk/modules/proxy/mod_proxy.c (original)
>> +++ httpd/httpd/trunk/modules/proxy/mod_proxy.c Sat Dec 20 15:56:16 2014
>            */
>> @@ -1477,15 +1486,16 @@ static const char *
>>      return add_proxy(cmd, dummy, f1, r1, 1);
>>  }
>>
>> -static char *de_socketfy(apr_pool_t *p, char *url)
>> +PROXY_DECLARE(char *) ap_proxy_de_socketfy(apr_pool_t *p, const char *url)
>>  {
>>      char *ptr;
>>      /*
>>       * We could be passed a URL during the config stage that contains
>>       * the UDS path... ignore it
>>       */
>> +    char *url_copy = apr_pstrdup(p, url);
>
> Why do we need to make a copy?
>
>>      if (!strncasecmp(url, "unix:", 5) &&
>> -        ((ptr = ap_strchr(url, '|')) != NULL)) {
>> +        ((ptr = ap_strchr(url_copy, '|')) != NULL)) {
>>          /* move past the 'unix:...|' UDS path info */
>>          char *ret, *c;
>>

Thanks -- I was trying to keep the parameter const and the return
value non-const when I moved it from static to API, but it was not
well done and it seems none of the callers actually need that.  I
removed the copy and made the return value const r1647334.

Re: svn commit: r1647009 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_proxy.xml docs/manual/mod/mod_proxy_fcgi.xml include/ap_mmn.h modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h modules/proxy/proxy_util.c

Posted by Ruediger Pluem <rp...@apache.org>.

On 12/20/2014 04:56 PM, covener@apache.org wrote:
> Author: covener
> Date: Sat Dec 20 15:56:16 2014
> New Revision: 1647009
> 
> URL: http://svn.apache.org/r1647009
> Log:
> Allow SetHandler+UDS+fcgi to take advantage of dedicated workers including
> opting in to connection reuse and other proxy options (max=, etc).
> 
> adds 'enablereuse' proxyoption and a minor MMN bump to share
> proxy_desocketfy outside of mod_proxy.c, which is required to
> match workers to URLs.
> 
> 
> Modified:
>     httpd/httpd/trunk/CHANGES
>     httpd/httpd/trunk/docs/manual/mod/mod_proxy.xml
>     httpd/httpd/trunk/docs/manual/mod/mod_proxy_fcgi.xml
>     httpd/httpd/trunk/include/ap_mmn.h
>     httpd/httpd/trunk/modules/proxy/mod_proxy.c
>     httpd/httpd/trunk/modules/proxy/mod_proxy.h
>     httpd/httpd/trunk/modules/proxy/proxy_util.c
> 

> Modified: httpd/httpd/trunk/modules/proxy/mod_proxy.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy.c?rev=1647009&r1=1647008&r2=1647009&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/proxy/mod_proxy.c (original)
> +++ httpd/httpd/trunk/modules/proxy/mod_proxy.c Sat Dec 20 15:56:16 2014
           */
> @@ -1477,15 +1486,16 @@ static const char *
>      return add_proxy(cmd, dummy, f1, r1, 1);
>  }
>  
> -static char *de_socketfy(apr_pool_t *p, char *url)
> +PROXY_DECLARE(char *) ap_proxy_de_socketfy(apr_pool_t *p, const char *url)
>  {
>      char *ptr;
>      /*
>       * We could be passed a URL during the config stage that contains
>       * the UDS path... ignore it
>       */
> +    char *url_copy = apr_pstrdup(p, url);

Why do we need to make a copy?

>      if (!strncasecmp(url, "unix:", 5) &&
> -        ((ptr = ap_strchr(url, '|')) != NULL)) {
> +        ((ptr = ap_strchr(url_copy, '|')) != NULL)) {
>          /* move past the 'unix:...|' UDS path info */
>          char *ret, *c;
>  

Regards

RĂ¼diger