This is uglier than creating a subpool... :)
On Oct 17, 2013, at 12:10 PM, Yann Ylavic <[email protected]> wrote:
> Maybe using the following macros could help to log worker's (full) name when
> no pool is available for ap_proxy_worker_name().
>
> Regards,
>
> Index: modules/proxy/mod_proxy.h
> ===================================================================
> --- modules/proxy/mod_proxy.h (revision 1533127)
> +++ modules/proxy/mod_proxy.h (working copy)
> @@ -328,6 +328,11 @@ do { \
> (w)->s->io_buffer_size_set = (c)->io_buffer_size_set; \
> } while (0)
>
> +#define PROXY_WORKER_FMT "%s%s%s%s"
> +#define PROXY_WORKER_ARG(w) \
> + *(w)->s->uds_path ? "unix" : "", (w)->s->uds_path, \
> + *(w)->s->uds_path ? "|" : "", (w)->s->name
> +
> /* use 2 hashes */
> typedef struct {
> unsigned int def;
> Index: modules/proxy/mod_proxy.c
> ===================================================================
> --- modules/proxy/mod_proxy.c (revision 1533127)
> +++ modules/proxy/mod_proxy.c (working copy)
> @@ -1548,15 +1548,15 @@ static const char *
> } else {
> reuse = 1;
> ap_log_error(APLOG_MARK, APLOG_INFO, 0, cmd->server,
> APLOGNO(01145)
> - "Sharing worker '%s' instead of creating new worker
> '%s'",
> - ap_proxy_worker_name(cmd->pool, worker), new->real);
> + "Sharing worker '"PROXY_WORKER_FMT"' instead of
> creating new worker '%s'",
> + PROXY_WORKER_ARG(worker), new->real);
> }
>
> for (i = 0; i < arr->nelts; i++) {
> if (reuse) {
> ap_log_error(APLOG_MARK, APLOG_WARNING, 0, cmd->server,
> APLOGNO(01146)
> - "Ignoring parameter '%s=%s' for worker '%s'
> because of worker sharing",
> - elts[i].key, elts[i].val,
> ap_proxy_worker_name(cmd->pool, worker));
> + "Ignoring parameter '%s=%s' for worker
> '"PROXY_WORKER_FMT"' because of worker sharing",
> + elts[i].key, elts[i].val,
> PROXY_WORKER_ARG(worker));
> } else {
> const char *err = set_worker_param(cmd->pool, worker,
> elts[i].key,
> elts[i].val);
> @@ -2021,14 +2021,14 @@ static const char *add_member(cmd_parms *cmd, void
> if ((err = ap_proxy_define_worker(cmd->pool, &worker, balancer,
> conf, name, 0)) != NULL)
> return apr_pstrcat(cmd->temp_pool, "BalancerMember ", err, NULL);
> ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, cmd->server, APLOGNO(01148)
> - "Defined worker '%s' for balancer '%s'",
> - ap_proxy_worker_name(cmd->pool, worker),
> balancer->s->name);
> + "Defined worker '"PROXY_WORKER_FMT"' for balancer '%s'",
> + PROXY_WORKER_ARG(worker), balancer->s->name);
> PROXY_COPY_CONF_PARAMS(worker, conf);
> } else {
> reuse = 1;
> ap_log_error(APLOG_MARK, APLOG_INFO, 0, cmd->server, APLOGNO(01149)
> - "Sharing worker '%s' instead of creating new worker
> '%s'",
> - ap_proxy_worker_name(cmd->pool, worker), name);
> + "Sharing worker '"PROXY_WORKER_FMT"' instead of
> creating new worker '%s'",
> + PROXY_WORKER_ARG(worker), name);
> }
>
> arr = apr_table_elts(params);
> @@ -2036,8 +2036,8 @@ static const char *add_member(cmd_parms *cmd, void
> for (i = 0; i < arr->nelts; i++) {
> if (reuse) {
> ap_log_error(APLOG_MARK, APLOG_WARNING, 0, cmd->server,
> APLOGNO(01150)
> - "Ignoring parameter '%s=%s' for worker '%s' because
> of worker sharing",
> - elts[i].key, elts[i].val,
> ap_proxy_worker_name(cmd->pool, worker));
> + "Ignoring parameter '%s=%s' for worker
> '"PROXY_WORKER_FMT"' because of worker sharing",
> + elts[i].key, elts[i].val, PROXY_WORKER_ARG(worker));
> } else {
> err = set_worker_param(cmd->pool, worker, elts[i].key,
> elts[i].val);
> Index: modules/proxy/mod_proxy_balancer.c
> ===================================================================
> --- modules/proxy/mod_proxy_balancer.c (revision 1533127)
> +++ modules/proxy/mod_proxy_balancer.c (working copy)
> @@ -118,8 +118,8 @@ static void init_balancer_members(apr_pool_t *p, s
> int worker_is_initialized;
> proxy_worker *worker = *workers;
> ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(01158)
> - "Looking at %s -> %s initialized?", balancer->s->name,
> - ap_proxy_worker_name(p, worker));
> + "Looking at "PROXY_WORKER_FMT" -> %s initialized?",
> balancer->s->name,
> + PROXY_WORKER_ARG(worker));
> worker_is_initialized = PROXY_WORKER_IS_INITIALIZED(worker);
> if (!worker_is_initialized) {
> ap_proxy_initialize_worker(worker, s, p);
> @@ -639,10 +639,10 @@ static int proxy_balancer_post_request(proxy_worke
> int val = ((int *)balancer->errstatuses->elts)[i];
> if (r->status == val) {
> ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(01174)
> - "%s: Forcing worker (%s) into error state "
> + "%s: Forcing worker ("PROXY_WORKER_FMT") into
> error state "
> "due to status code %d matching 'failonstatus'
> "
> "balancer parameter",
> - balancer->s->name,
> ap_proxy_worker_name(r->pool, worker),
> + balancer->s->name, PROXY_WORKER_ARG(worker),
> val);
> worker->s->status |= PROXY_WORKER_IN_ERROR;
> worker->s->error_time = apr_time_now();
> @@ -654,9 +654,9 @@ static int proxy_balancer_post_request(proxy_worke
> if (balancer->failontimeout
> && (apr_table_get(r->notes, "proxy_timedout")) != NULL) {
> ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(02460)
> - "%s: Forcing worker (%s) into error state "
> + "%s: Forcing worker ("PROXY_WORKER_FMT") into error
> state "
> "due to timeout and 'failonstatus' parameter being
> set",
> - balancer->s->name, ap_proxy_worker_name(r->pool,
> worker));
> + balancer->s->name, PROXY_WORKER_ARG(worker));
> worker->s->status |= PROXY_WORKER_IN_ERROR;
> worker->s->error_time = apr_time_now();
>
> Index: modules/proxy/proxy_util.c
> ===================================================================
> --- modules/proxy/proxy_util.c (revision 1533127)
> +++ modules/proxy/proxy_util.c (working copy)
> @@ -1366,9 +1366,9 @@ static apr_status_t connection_cleanup(void *theco
> /* Sanity check: Did we already return the pooled connection? */
> if (conn->inreslist) {
> ap_log_perror(APLOG_MARK, APLOG_ERR, 0, conn->pool, APLOGNO(00923)
> - "Pooled connection 0x%pp for worker %s has been"
> + "Pooled connection 0x%pp for worker "PROXY_WORKER_FMT"
> has been"
> " already returned to the connection pool.", conn,
> - ap_proxy_worker_name(conn->pool, worker));
> + PROXY_WORKER_ARG(worker));
> return APR_SUCCESS;
> }
>
> @@ -1501,14 +1501,6 @@ PROXY_DECLARE(char *) ap_proxy_worker_name(apr_poo
> if (!(*worker->s->uds_path)) {
> return worker->s->name;
> }
> - if (!pool) {
> - /* ugly */
> - apr_pool_create(&pool, ap_server_conf->process->pool);
> - if (!pool) {
> - /* something is better than nothing :) */
> - return worker->s->name;
> - }
> - }
> return apr_pstrcat(pool, "unix:", worker->s->uds_path, "|",
> worker->s->name, NULL);
> }
>
> @@ -1742,8 +1734,8 @@ PROXY_DECLARE(apr_status_t) ap_proxy_share_worker(
> worker->s = shm;
> worker->s->index = i;
> ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, ap_server_conf, APLOGNO(02338)
> - "%s shm[%d] (0x%pp) for worker: %s", action, i, (void *)shm,
> - ap_proxy_worker_name(NULL, worker));
> + "%s shm[%d] (0x%pp) for worker: "PROXY_WORKER_FMT, action,
> + i, (void *)shm, PROXY_WORKER_ARG(worker));
> return APR_SUCCESS;
> }
>
> @@ -1755,13 +1747,13 @@ PROXY_DECLARE(apr_status_t) ap_proxy_initialize_wo
> if (worker->s->status & PROXY_WORKER_INITIALIZED) {
> /* The worker is already initialized */
> ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(00924)
> - "worker %s shared already initialized",
> - ap_proxy_worker_name(p, worker));
> + "worker "PROXY_WORKER_FMT" shared already initialized",
> + PROXY_WORKER_ARG(worker));
> }
> else {
> ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(00925)
> - "initializing worker %s shared",
> - ap_proxy_worker_name(p, worker));
> + "initializing worker "PROXY_WORKER_FMT" shared",
> + PROXY_WORKER_ARG(worker));
> /* Set default parameters */
> if (!worker->s->retry_set) {
> worker->s->retry = apr_time_from_sec(PROXY_WORKER_DEFAULT_RETRY);
> @@ -1797,13 +1789,13 @@ PROXY_DECLARE(apr_status_t) ap_proxy_initialize_wo
> /* What if local is init'ed and shm isn't?? Even possible? */
> if (worker->local_status & PROXY_WORKER_INITIALIZED) {
> ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(00926)
> - "worker %s local already initialized",
> - ap_proxy_worker_name(p, worker));
> + "worker "PROXY_WORKER_FMT" local already initialized",
> + PROXY_WORKER_ARG(worker));
> }
> else {
> ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(00927)
> - "initializing worker %s local",
> - ap_proxy_worker_name(p, worker));
> + "initializing worker "PROXY_WORKER_FMT" local",
> + PROXY_WORKER_ARG(worker));
> apr_global_mutex_lock(proxy_mutex);
> /* Now init local worker data */
> if (worker->tmutex == NULL) {
> @@ -1902,8 +1894,8 @@ PROXY_DECLARE(int) ap_proxy_pre_request(proxy_work
> *worker = ap_proxy_get_worker(r->pool, NULL, conf, *url);
> if (*worker) {
> ap_log_rerror(APLOG_MARK, APLOG_TRACE2, 0, r,
> - "%s: found worker %s for %s",
> - (*worker)->s->scheme, (*worker)->s->name, *url);
> + "%s: found worker "PROXY_WORKER_FMT" for %s",
> + (*worker)->s->scheme, PROXY_WORKER_ARG(*worker),
> *url);
>
> *balancer = NULL;
> access_status = OK;
> @@ -2957,8 +2949,8 @@ PROXY_DECLARE(apr_status_t) ap_proxy_sync_balancer
> if (worker->hash.def == shm->hash.def && worker->hash.fnv ==
> shm->hash.fnv) {
> found = 1;
> ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(02402)
> - "re-grabbing shm[%d] (0x%pp) for worker: %s",
> i, (void *)shm,
> - ap_proxy_worker_name(conf->pool, worker));
> + "re-grabbing shm[%d] (0x%pp) for worker:
> "PROXY_WORKER_FMT,
> + i, (void *)shm, PROXY_WORKER_ARG(worker));
> break;
> }
> }
> [EOS]
>
>
>
> On Thu, Oct 17, 2013 at 4:10 PM, <[email protected]> wrote:
> Author: jim
> Date: Thu Oct 17 14:10:43 2013
> New Revision: 1533087
>
> URL: http://svn.apache.org/r1533087
> Log:
> Put the uds path in its own field, and adjust the logic
> to look for an empty string rather than a flag.
>
> Modified:
> httpd/httpd/trunk/modules/proxy/mod_proxy.h
> httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c
> httpd/httpd/trunk/modules/proxy/proxy_util.c
>
> Modified: httpd/httpd/trunk/modules/proxy/mod_proxy.h
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy.h?rev=1533087&r1=1533086&r2=1533087&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/proxy/mod_proxy.h (original)
> +++ httpd/httpd/trunk/modules/proxy/mod_proxy.h Thu Oct 17 14:10:43 2013
> @@ -342,6 +342,7 @@ typedef struct {
> char route[PROXY_WORKER_MAX_ROUTE_SIZE]; /* balancing route */
> char redirect[PROXY_WORKER_MAX_ROUTE_SIZE]; /* temporary balancing
> redirection route */
> char flusher[PROXY_WORKER_MAX_SCHEME_SIZE]; /* flush provider used
> by mod_proxy_fdpass */
> + char uds_path[PROXY_WORKER_MAX_NAME_SIZE]; /* path to worker's
> unix domain socket if applicable */
> int lbset; /* load balancer cluster set */
> int retries; /* number of retries on this worker */
> int lbstatus; /* Current lbstatus */
> @@ -388,7 +389,6 @@ typedef struct {
> unsigned int keepalive_set:1;
> unsigned int disablereuse_set:1;
> unsigned int was_malloced:1;
> - unsigned int uds:1;
> } proxy_worker_shared;
>
> #define ALIGNED_PROXY_WORKER_SHARED_SIZE
> (APR_ALIGN_DEFAULT(sizeof(proxy_worker_shared)))
>
> Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c?rev=1533087&r1=1533086&r2=1533087&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c (original)
> +++ httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c Thu Oct 17 14:10:43
> 2013
> @@ -1533,8 +1533,8 @@ static int balancer_handler(request_rec
> ap_escape_uri(r->pool, worker->s->name),
> "&nonce=", balancer->s->nonce,
> "\">", NULL);
> - ap_rvputs(r, (worker->s->uds ? "<i>" : ""),
> ap_proxy_worker_name(r->pool, worker),
> - (worker->s->uds ? "</i>" : ""), "</a></td>", NULL);
> + ap_rvputs(r, (*worker->s->uds_path ? "<i>" : ""),
> ap_proxy_worker_name(r->pool, worker),
> + (*worker->s->uds_path ? "</i>" : ""), "</a></td>",
> NULL);
> ap_rvputs(r, "<td>", ap_escape_html(r->pool,
> worker->s->route),
> NULL);
> ap_rvputs(r, "</td><td>",
> @@ -1559,7 +1559,7 @@ static int balancer_handler(request_rec
> ap_rputs("<hr />\n", r);
> if (wsel && bsel) {
> ap_rputs("<h3>Edit worker settings for ", r);
> - ap_rvputs(r, (wsel->s->uds?"<i>":""),
> ap_proxy_worker_name(r->pool, wsel), (wsel->s->uds?"</i>":""), "</h3>\n",
> NULL);
> + ap_rvputs(r, (*wsel->s->uds_path?"<i>":""),
> ap_proxy_worker_name(r->pool, wsel), (*wsel->s->uds_path?"</i>":""),
> "</h3>\n", NULL);
> ap_rputs("<form method=\"POST\"
> enctype=\"application/x-www-form-urlencoded\" action=\"", r);
> ap_rvputs(r, ap_escape_uri(r->pool, action), "\">\n", NULL);
> ap_rputs("<dl>\n<table><tr><td>Load factor:</td><td><input
> name='w_lf' id='w_lf' type=text ", r);
>
> Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=1533087&r1=1533086&r2=1533087&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
> +++ httpd/httpd/trunk/modules/proxy/proxy_util.c Thu Oct 17 14:10:43 2013
> @@ -1493,7 +1493,7 @@ PROXY_DECLARE(char *) ap_proxy_worker_na
> int rv;
> apr_uri_t uri;
> apr_pool_t *pool = p;
> - if (!worker->s->uds) {
> + if (!(*worker->s->uds_path)) {
> return worker->s->name;
> }
> if (!pool) {
> @@ -1504,11 +1504,7 @@ PROXY_DECLARE(char *) ap_proxy_worker_na
> return worker->s->name;
> }
> }
> - rv = apr_uri_parse(pool, worker->s->name, &uri);
> - if (rv != APR_SUCCESS) {
> - return apr_pstrcat(pool, worker->s->name, "|", NULL);
> - }
> - return apr_pstrcat(pool, "unix:", uri.path, "|", uri.scheme, ":", NULL);
> + return apr_pstrcat(pool, "unix:", worker->s->uds_path, "|",
> worker->s->name, NULL);
> }
>
> PROXY_DECLARE(proxy_worker *) ap_proxy_get_worker(apr_pool_t *p,
> @@ -1609,16 +1605,17 @@ PROXY_DECLARE(char *) ap_proxy_define_wo
> proxy_worker_shared *wshared;
> char *ptr, *sockpath = NULL;
>
> - /* Look to see if we are using UDS:
> - require format: unix:/path/foo/bar.sock|http:
> - This results in talking http to the socket at /path/foo/bar.sock
> - */
> + /*
> + * Look to see if we are using UDS:
> + * require format: unix:/path/foo/bar.sock|http://ignored/path2/
> + * This results in talking http to the socket at /path/foo/bar.sock
> + */
> ptr = ap_strchr((char *)url, '|');
> if (ptr) {
> *ptr = '\0';
> rv = apr_uri_parse(p, url, &urisock);
> if (rv == APR_SUCCESS && !strcasecmp(urisock.scheme, "unix")) {
> - sockpath = urisock.path;
> + sockpath = ap_runtime_dir_relative(p, urisock.path);;
> url = ptr+1; /* so we get the scheme for the uds */
> }
> else {
> @@ -1633,16 +1630,13 @@ PROXY_DECLARE(char *) ap_proxy_define_wo
> if (!uri.scheme) {
> return apr_pstrcat(p, "URL must be absolute!: ", url, NULL);
> }
> - /* allow for http:|sock:/path */
> + /* allow for unix:/path|http: */
> if (!uri.hostname && !sockpath) {
> return apr_pstrcat(p, "URL must be absolute!: ", url, NULL);;
> }
>
> if (sockpath) {
> uri.hostname = "localhost";
> - uri.path = ap_runtime_dir_relative(p, sockpath);
> - uri.query = NULL;
> - uri.fragment = NULL;
> }
> else {
> ap_str_tolower(uri.hostname);
> @@ -1702,7 +1696,15 @@ PROXY_DECLARE(char *) ap_proxy_define_wo
> wshared->hash.def = ap_proxy_hashfunc(wshared->name,
> PROXY_HASHFUNC_DEFAULT);
> wshared->hash.fnv = ap_proxy_hashfunc(wshared->name, PROXY_HASHFUNC_FNV);
> wshared->was_malloced = (do_malloc != 0);
> - wshared->uds = (sockpath != NULL);
> + if (sockpath) {
> + if (PROXY_STRNCPY(wshared->uds_path, sockpath) != APR_SUCCESS) {
> + return apr_psprintf(p, "worker uds path (%s) too long",
> sockpath);
> + }
> +
> + }
> + else {
> + *wshared->uds_path = '\0';
> + }
>
> (*worker)->hash = wshared->hash;
> (*worker)->context = NULL;
> @@ -2087,12 +2089,9 @@ PROXY_DECLARE(int) ap_proxy_acquire_conn
> (*conn)->close = 0;
> (*conn)->inreslist = 0;
>
> - if (worker->s->uds) {
> + if (*worker->s->uds_path) {
> if ((*conn)->uds_path == NULL) {
> - apr_uri_t puri;
> - if (apr_uri_parse(worker->cp->pool, worker->s->name, &puri) ==
> APR_SUCCESS) {
> - (*conn)->uds_path = apr_pstrdup(worker->cp->pool, puri.path);
> - }
> + (*conn)->uds_path = apr_pstrdup(worker->cp->pool,
> worker->s->uds_path);
> }
> if ((*conn)->uds_path) {
> ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(02545)
> @@ -2100,9 +2099,10 @@ PROXY_DECLARE(int) ap_proxy_acquire_conn
> proxy_function, (*conn)->uds_path);
> }
> else {
> + /* should never happen */
> ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(02546)
> - "%s: cannot parse for UDS (%s)",
> - proxy_function, worker->s->name);
> + "%s: cannot determine UDS (%s)",
> + proxy_function, worker->s->uds_path);
>
> }
> }
> @@ -2182,7 +2182,7 @@ ap_proxy_determine_connection(apr_pool_t
> * spilling the cached addr from the worker.
> */
> if (!conn->hostname || !worker->s->is_address_reusable ||
> - worker->s->disablereuse || worker->s->uds) {
> + worker->s->disablereuse || *worker->s->uds_path) {
> if (proxyname) {
> conn->hostname = apr_pstrdup(conn->pool, proxyname);
> conn->port = proxyport;
> @@ -2220,7 +2220,7 @@ ap_proxy_determine_connection(apr_pool_t
> conn->port = uri->port;
> }
> socket_cleanup(conn);
> - if (!worker->s->uds && worker->s->is_address_reusable &&
> !worker->s->disablereuse) {
> + if (!(*worker->s->uds_path) && worker->s->is_address_reusable &&
> !worker->s->disablereuse) {
> /*
> * Only do a lookup if we should not reuse the backend address.
> * Otherwise we will look it up once for the worker.
> @@ -2231,7 +2231,7 @@ ap_proxy_determine_connection(apr_pool_t
> conn->pool);
> }
> }
> - if (!worker->s->uds && worker->s->is_address_reusable &&
> !worker->s->disablereuse) {
> + if (!(*worker->s->uds_path) && worker->s->is_address_reusable &&
> !worker->s->disablereuse) {
> /*
> * Looking up the backend address for the worker only makes sense if
> * we can reuse the address.
>
>
>
> <httpd-trunk-mod_proxy_WORKER_FMT_ARG.patch>
