From: Jim Jagielski Date: Tue, 22 Jan 2013 13:39:35 +0000 (+0000) Subject: Merge r1387110, r1387444, r1387979, r1387607, r1387693, r1407085, r1421953 from trunk: X-Git-Tag: 2.4.4~89 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=f842ed5353099c8ec003cac1c158fdc599d7f6c7;p=apache Merge r1387110, r1387444, r1387979, r1387607, r1387693, r1407085, r1421953 from trunk: Persist local balancer-manager changes across restart/graceful. Use identifying server_rec info when we know we have unique and useful data :) fix clang warning (dead initialization) Log whether or not the restore from shm actually resulted in a match of shm data, or whether it was stale. and this one as well... persist isn't inherited better logging for re-use/use of shm Allow for searching w/i shm slots for a specific worker and balancer Reviewed/backported by: jim git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1436919 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/CHANGES b/CHANGES index b34c346df1..d9a9879499 100644 --- a/CHANGES +++ b/CHANGES @@ -2,6 +2,13 @@ Changes with Apache 2.4.4 + *) mod_proxy: Allow for persistence of local changes made via the + balancer-manager between graceful/normal restarts and power + cycles. [Jim Jagielski] + + *) mod_proxy: Fix startup crash with mis-defined balancers. + PR 52402. [Jim Jagielski] + *) --with-module: Fix failure to integrate them into some existing module directories. PR 40097. [Jeff Trawick] diff --git a/STATUS b/STATUS index 0e525bd3f3..873692e3b2 100644 --- a/STATUS +++ b/STATUS @@ -100,19 +100,6 @@ PATCHES ACCEPTED TO BACKPORT FROM TRUNK: http://mail-archives.apache.org/mod_mbox/httpd-dev/201301.mbox/%3C20130103112044.GA19653%40redhat.com%3E kbrand: configure option added in r1429228 - * mod_proxy: Persist balancer and worker data across restarts and stop/start. - trunk patch: http://svn.apache.org/viewvc?view=revision&revision=1387110 - http://svn.apache.org/viewvc?view=revision&revision=1387444 - http://svn.apache.org/viewvc?view=revision&revision=1387979 - http://svn.apache.org/viewvc?view=revision&revision=1387607 - http://svn.apache.org/viewvc?view=revision&revision=1387693 - http://svn.apache.org/viewvc?view=revision&revision=1407085 - http://svn.apache.org/viewvc?view=revision&revision=1421953 - 2.4.x patch: http://people.apache.org/~jim/patches/proxy-persist.diff - +1: jim, druggeri, jerenkrantz - druggeri: Doc for 2.4 patch says version 2.5.0 - Worth noting in CHANGES that fix for 52402 is in here? - PATCHES PROPOSED TO BACKPORT FROM TRUNK: [ New proposals should be added at the end of the list ] diff --git a/docs/manual/mod/mod_proxy.xml b/docs/manual/mod/mod_proxy.xml index cb6d63d3d7..58cd73cb6c 100644 --- a/docs/manual/mod/mod_proxy.xml +++ b/docs/manual/mod/mod_proxy.xml @@ -648,6 +648,22 @@ expressions + + BalancerPersist + Attempt to persist changes made by the Balancer Manager across restarts. + BalancerPersist On|Off + BalancerPersist Off + server configvirtual host + BalancerPersist is only available in Apache HTTP Server 2.4.4 and later. + and later. + +

This directive will cause the shared memory storage associated + with the balancers and balancer members to be persisted across + restarts. This allows these local changes to not be lost during the + normal restart/graceful state transitions.

+ + + BalancerMember Add a member to a load balancing group diff --git a/include/ap_mmn.h b/include/ap_mmn.h index 1df0f3e9d3..b412941a49 100644 --- a/include/ap_mmn.h +++ b/include/ap_mmn.h @@ -399,6 +399,7 @@ * 20120211.7 (2.4.3-dev) Add ap_get_loadavg() * 20120211.8 (2.4.3-dev) Add sticky_separator to proxy_balancer_shared struct. * 20120211.9 (2.4.4-dev) Add fgrab() to ap_slotmem_provider_t. + * 20120211.10 (2.4.4-dev) Add in bal_persist field to proxy_server_conf */ #define MODULE_MAGIC_COOKIE 0x41503234UL /* "AP24" */ @@ -406,7 +407,7 @@ #ifndef MODULE_MAGIC_NUMBER_MAJOR #define MODULE_MAGIC_NUMBER_MAJOR 20120211 #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 diff --git a/modules/proxy/mod_proxy.c b/modules/proxy/mod_proxy.c index 8e346a0a94..0bd3a6c9f9 100644 --- a/modules/proxy/mod_proxy.c +++ b/modules/proxy/mod_proxy.c @@ -1142,7 +1142,6 @@ cleanup: static void * create_proxy_config(apr_pool_t *p, server_rec *s) { - unsigned int id; proxy_server_conf *ps = apr_pcalloc(p, sizeof(proxy_server_conf)); ps->sec_proxy = apr_array_make(p, 10, sizeof(ap_conf_vector_t *)); @@ -1155,16 +1154,12 @@ static void * create_proxy_config(apr_pool_t *p, server_rec *s) ps->forward = NULL; ps->reverse = NULL; ps->domain = NULL; -#if 0 - id = ap_proxy_hashfunc(apr_psprintf(p, "%pp-%" APR_TIME_T_FMT, ps, apr_time_now()), PROXY_HASHFUNC_DEFAULT); -#else - id = ap_proxy_hashfunc(apr_psprintf(p, "%pp", ps), PROXY_HASHFUNC_DEFAULT); -#endif - ps->id = apr_psprintf(p, "s%x", id); + ps->id = apr_psprintf(p, "p%x", 1); /* simply for storage size */ ps->viaopt = via_off; /* initially backward compatible with 1.3.1 */ ps->viaopt_set = 0; /* 0 means default */ ps->req = 0; ps->max_balancers = 0; + ps->bal_persist = 0; ps->bgrowth = 5; ps->bgrowth_set = 0; ps->req_set = 0; @@ -1210,6 +1205,7 @@ static void * merge_proxy_config(apr_pool_t *p, void *basev, void *overridesv) ps->bgrowth = (overrides->bgrowth_set == 0) ? base->bgrowth : overrides->bgrowth; ps->bgrowth_set = overrides->bgrowth_set || base->bgrowth_set; ps->max_balancers = overrides->max_balancers || base->max_balancers; + ps->bal_persist = overrides->bal_persist; ps->recv_buffer_size = (overrides->recv_buffer_size_set == 0) ? base->recv_buffer_size : overrides->recv_buffer_size; ps->recv_buffer_size_set = overrides->recv_buffer_size_set || base->recv_buffer_size_set; ps->io_buffer_size = (overrides->io_buffer_size_set == 0) ? base->io_buffer_size : overrides->io_buffer_size; @@ -1885,6 +1881,15 @@ static const char *set_bgrowth(cmd_parms *parms, void *dummy, const char *arg) return NULL; } +static const char *set_persist(cmd_parms *parms, void *dummy, int flag) +{ + proxy_server_conf *psf = + ap_get_module_config(parms->server->module_config, &proxy_module); + + psf->bal_persist = flag; + return NULL; +} + static const char *add_member(cmd_parms *cmd, void *dummy, const char *arg) { server_rec *s = cmd->server; @@ -2272,6 +2277,8 @@ static const command_rec proxy_cmds[] = "A balancer name and scheme with list of params"), AP_INIT_TAKE1("BalancerGrowth", set_bgrowth, NULL, RSRC_CONF, "Number of additional Balancers that can be added post-config"), + AP_INIT_FLAG("BalancerPersist", set_persist, NULL, RSRC_CONF, + "on if the balancer should persist changes on reboot/restart made via the Balancer Manager"), AP_INIT_TAKE1("ProxyStatus", set_status_opt, NULL, RSRC_CONF, "Configure Status: proxy status to one of: on | off | full"), AP_INIT_RAW_ARGS("ProxySet", set_proxy_param, NULL, RSRC_CONF|ACCESS_CONF, diff --git a/modules/proxy/mod_proxy.h b/modules/proxy/mod_proxy.h index 6e2fbb5253..5074aa1615 100644 --- a/modules/proxy/mod_proxy.h +++ b/modules/proxy/mod_proxy.h @@ -178,6 +178,7 @@ typedef struct { unsigned int proxy_status_set:1; unsigned int source_address_set:1; unsigned int bgrowth_set:1; + unsigned int bal_persist:1; } proxy_server_conf; @@ -702,6 +703,32 @@ PROXY_DECLARE(apr_status_t) ap_proxy_initialize_balancer(proxy_balancer *balance server_rec *s, apr_pool_t *p); +/** + * Find the shm of the worker as needed + * @param storage slotmem provider + * @param slot slotmem instance + * @param worker worker to find + * @param index pointer to index within slotmem of worker + * @return pointer to shm of worker, or NULL + */ +PROXY_DECLARE(proxy_worker_shared *) ap_proxy_find_workershm(ap_slotmem_provider_t *storage, + ap_slotmem_instance_t *slot, + proxy_worker *worker, + unsigned int *index); + +/** + * Find the shm of the balancer as needed + * @param storage slotmem provider + * @param slot slotmem instance + * @param worker worker to find + * @param index pointer to index within slotmem of balancer + * @return pointer to shm of balancer, or NULL + */ +PROXY_DECLARE(proxy_balancer_shared *) ap_proxy_find_balancershm(ap_slotmem_provider_t *storage, + ap_slotmem_instance_t *slot, + proxy_balancer *balancer, + unsigned int *index); + /** * Get the most suitable worker and/or balancer for the request * @param worker worker used for processing request diff --git a/modules/proxy/mod_proxy_balancer.c b/modules/proxy/mod_proxy_balancer.c index ac1e6f7343..cd9987e192 100644 --- a/modules/proxy/mod_proxy_balancer.c +++ b/modules/proxy/mod_proxy_balancer.c @@ -703,7 +703,6 @@ static int balancer_post_config(apr_pool_t *pconf, apr_pool_t *plog, apr_pool_t *ptemp, server_rec *s) { apr_status_t rv; - void *sconf = s->module_config; proxy_server_conf *conf; ap_slotmem_instance_t *new = NULL; apr_time_t tstamp; @@ -744,16 +743,36 @@ static int balancer_post_config(apr_pool_t *pconf, apr_pool_t *plog, */ while (s) { int i,j; + char *id; proxy_balancer *balancer; - sconf = s->module_config; + ap_slotmem_type_t type; + void *sconf = s->module_config; conf = (proxy_server_conf *)ap_get_module_config(sconf, &proxy_module); - + /* + * During create_proxy_config() we created a dummy id. Now that + * we have identifying info, we can create the real id + */ + id = apr_psprintf(pconf, "%s.%s.%d.%s.%s.%u.%s", + (s->server_scheme ? s->server_scheme : "????"), + (s->server_hostname ? s->server_hostname : "???"), + (int)s->port, + (s->server_admin ? s->server_admin : "??"), + (s->defn_name ? s->defn_name : "?"), + s->defn_line_number, + (s->error_fname ? s->error_fname : DEFAULT_ERRORLOG)); + conf->id = apr_psprintf(pconf, "p%x", + ap_proxy_hashfunc(id, PROXY_HASHFUNC_DEFAULT)); if (conf->bslot) { /* Shared memory already created for this proxy_server_conf. */ s = s->next; continue; } + if (conf->bal_persist) { + type = AP_SLOTMEM_TYPE_PREGRAB | AP_SLOTMEM_TYPE_PERSIST; + } else { + type = AP_SLOTMEM_TYPE_PREGRAB; + } if (conf->balancers->nelts) { conf->max_balancers = conf->balancers->nelts + conf->bgrowth; ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(01178) "Doing balancers create: %d, %d (%d)", @@ -762,7 +781,7 @@ static int balancer_post_config(apr_pool_t *pconf, apr_pool_t *plog, rv = storage->create(&new, conf->id, ALIGNED_PROXY_BALANCER_SHARED_SIZE, - conf->max_balancers, AP_SLOTMEM_TYPE_PREGRAB, pconf); + conf->max_balancers, type, pconf); if (rv != APR_SUCCESS) { ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01179) "balancer slotmem_create failed"); return !OK; @@ -777,8 +796,15 @@ static int balancer_post_config(apr_pool_t *pconf, apr_pool_t *plog, proxy_worker **workers; proxy_worker *worker; proxy_balancer_shared *bshm; + const char *sname; unsigned int index; + /* now that we have the right id, we need to redo the sname field */ + ap_pstr2_alnum(pconf, balancer->s->name + sizeof(BALANCER_PREFIX) - 1, + &sname); + sname = apr_pstrcat(pconf, conf->id, "_", sname, NULL); + PROXY_STRNCPY(balancer->s->sname, sname); /* We know this will succeed */ + balancer->max_workers = balancer->workers->nelts + balancer->growth; /* Create global mutex */ @@ -795,14 +821,22 @@ static int balancer_post_config(apr_pool_t *pconf, apr_pool_t *plog, apr_pool_cleanup_null); /* setup shm for balancers */ - if ((rv = storage->grab(conf->bslot, &index)) != APR_SUCCESS) { - ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01181) "balancer slotmem_grab failed"); - return !OK; - + bshm = ap_proxy_find_balancershm(storage, conf->bslot, balancer, &index); + if (bshm) { + if ((rv = storage->fgrab(conf->bslot, index)) != APR_SUCCESS) { + ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(02408) "balancer slotmem_fgrab failed"); + return !OK; + } } - if ((rv = storage->dptr(conf->bslot, index, (void *)&bshm)) != APR_SUCCESS) { - ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01182) "balancer slotmem_dptr failed"); - return !OK; + else { + if ((rv = storage->grab(conf->bslot, &index)) != APR_SUCCESS) { + ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01181) "balancer slotmem_grab failed"); + return !OK; + } + if ((rv = storage->dptr(conf->bslot, index, (void *)&bshm)) != APR_SUCCESS) { + ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01182) "balancer slotmem_dptr failed"); + return !OK; + } } if ((rv = ap_proxy_share_balancer(balancer, bshm, index)) != APR_SUCCESS) { ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01183) "Cannot share balancer"); @@ -810,14 +844,14 @@ static int balancer_post_config(apr_pool_t *pconf, apr_pool_t *plog, } /* create slotmem slots for workers */ - ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(01184) "Doing workers create: %s (%s), %d, %d", + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(01184) "Doing workers create: %s (%s), %d, %d [%u]", balancer->s->name, balancer->s->sname, (int)ALIGNED_PROXY_WORKER_SHARED_SIZE, - (int)balancer->max_workers); + (int)balancer->max_workers, i); rv = storage->create(&new, balancer->s->sname, ALIGNED_PROXY_WORKER_SHARED_SIZE, - balancer->max_workers, AP_SLOTMEM_TYPE_PREGRAB, pconf); + balancer->max_workers, type, pconf); if (rv != APR_SUCCESS) { ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01185) "worker slotmem_create failed"); return !OK; @@ -834,14 +868,24 @@ static int balancer_post_config(apr_pool_t *pconf, apr_pool_t *plog, proxy_worker_shared *shm; worker = *workers; - if ((rv = storage->grab(balancer->wslot, &index)) != APR_SUCCESS) { - ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01186) "worker slotmem_grab failed"); - return !OK; + shm = ap_proxy_find_workershm(storage, balancer->wslot, worker, &index); + if (shm) { + if ((rv = storage->fgrab(balancer->wslot, index)) != APR_SUCCESS) { + ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(02409) "worker slotmem_fgrab failed"); + return !OK; + } } - if ((rv = storage->dptr(balancer->wslot, index, (void *)&shm)) != APR_SUCCESS) { - ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01187) "worker slotmem_dptr failed"); - return !OK; + else { + if ((rv = storage->grab(balancer->wslot, &index)) != APR_SUCCESS) { + ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01186) "worker slotmem_grab failed"); + return !OK; + + } + if ((rv = storage->dptr(balancer->wslot, index, (void *)&shm)) != APR_SUCCESS) { + ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01187) "worker slotmem_dptr failed"); + return !OK; + } } if ((rv = ap_proxy_share_worker(worker, shm, index)) != APR_SUCCESS) { ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01188) "Cannot share worker"); @@ -849,6 +893,11 @@ static int balancer_post_config(apr_pool_t *pconf, apr_pool_t *plog, } worker->s->updated = tstamp; } + if (conf->bal_persist) { + /* We could have just read-in a persisted config. Force a sync. */ + balancer->wupdated--; + ap_proxy_sync_balancer(balancer, s, conf); + } } s = s->next; } @@ -1412,7 +1461,7 @@ static int balancer_handler(request_rec *r) balancer->s->name + sizeof(BALANCER_PREFIX) - 1, "&nonce=", balancer->s->nonce, "\">", NULL); - ap_rvputs(r, balancer->s->name, "\n", NULL); + ap_rvputs(r, balancer->s->name, " [",balancer->s->sname, "]\n", NULL); ap_rputs("\n\n" "" "\n", r); diff --git a/modules/proxy/proxy_util.c b/modules/proxy/proxy_util.c index 38976f7e7e..5ab5d9144a 100644 --- a/modules/proxy/proxy_util.c +++ b/modules/proxy/proxy_util.c @@ -1171,6 +1171,10 @@ PROXY_DECLARE(char *) ap_proxy_define_balancer(apr_pool_t *p, if (PROXY_STRNCPY(bshared->name, uri) != APR_SUCCESS) { return apr_psprintf(p, "balancer name (%s) too long", uri); } + /* + * We do the below for verification. The real sname will be + * done post_config + */ ap_pstr2_alnum(p, bshared->name + sizeof(BALANCER_PREFIX) - 1, &sname); sname = apr_pstrcat(p, conf->id, "_", sname, NULL); @@ -1200,12 +1204,21 @@ PROXY_DECLARE(apr_status_t) ap_proxy_share_balancer(proxy_balancer *balancer, { apr_status_t rv = APR_SUCCESS; proxy_balancer_method *lbmethod; + char *action = "copying"; if (!shm || !balancer->s) return APR_EINVAL; - memcpy(shm, balancer->s, sizeof(proxy_balancer_shared)); - if (balancer->s->was_malloced) - free(balancer->s); + if ((balancer->s->hash.def != shm->hash.def) || + (balancer->s->hash.fnv != shm->hash.fnv)) { + memcpy(shm, balancer->s, sizeof(proxy_balancer_shared)); + if (balancer->s->was_malloced) + free(balancer->s); + } else { + action = "re-using"; + } + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, ap_server_conf, APLOGNO(02337) + "%s shm[%d] (0x%pp) for %s", action, i, (void *)shm, + balancer->s->name); balancer->s = shm; balancer->s->index = i; /* the below should always succeed */ @@ -1643,12 +1656,22 @@ PROXY_DECLARE(char *) ap_proxy_define_worker(apr_pool_t *p, PROXY_DECLARE(apr_status_t) ap_proxy_share_worker(proxy_worker *worker, proxy_worker_shared *shm, int i) { + char *action = "copying"; if (!shm || !worker->s) return APR_EINVAL; - memcpy(shm, worker->s, sizeof(proxy_worker_shared)); - if (worker->s->was_malloced) - free(worker->s); /* was malloced in ap_proxy_define_worker */ + if ((worker->s->hash.def != shm->hash.def) || + (worker->s->hash.fnv != shm->hash.fnv)) { + memcpy(shm, worker->s, sizeof(proxy_worker_shared)); + if (worker->s->was_malloced) + free(worker->s); /* was malloced in ap_proxy_define_worker */ + } else { + action = "re-using"; + } + 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, + worker->s->name); + worker->s = shm; worker->s->index = i; return APR_SUCCESS; @@ -2735,6 +2758,9 @@ PROXY_DECLARE(apr_status_t) ap_proxy_sync_balancer(proxy_balancer *b, server_rec proxy_worker *worker = *workers; 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, + worker->s->name); break; } } @@ -2752,6 +2778,9 @@ PROXY_DECLARE(apr_status_t) ap_proxy_sync_balancer(proxy_balancer *b, server_rec ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(00966) "Cannot init worker"); return rv; } + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(02403) + "grabbing shm[%d] (0x%pp) for worker: %s", i, (void *)shm, + (*runtime)->s->name); } } if (b->s->need_reset) { @@ -2763,6 +2792,48 @@ PROXY_DECLARE(apr_status_t) ap_proxy_sync_balancer(proxy_balancer *b, server_rec return APR_SUCCESS; } +PROXY_DECLARE(proxy_worker_shared *) ap_proxy_find_workershm(ap_slotmem_provider_t *storage, + ap_slotmem_instance_t *slot, + proxy_worker *worker, + unsigned int *index) +{ + proxy_worker_shared *shm; + unsigned int i, limit; + limit = storage->num_slots(slot); + for (i = 0; i < limit; i++) { + if (storage->dptr(slot, i, (void *)&shm) != APR_SUCCESS) { + return NULL; + } + if ((worker->s->hash.def == shm->hash.def) && + (worker->s->hash.fnv == shm->hash.fnv)) { + *index = i; + return shm; + } + } + return NULL; +} + +PROXY_DECLARE(proxy_balancer_shared *) ap_proxy_find_balancershm(ap_slotmem_provider_t *storage, + ap_slotmem_instance_t *slot, + proxy_balancer *balancer, + unsigned int *index) +{ + proxy_balancer_shared *shm; + unsigned int i, limit; + limit = storage->num_slots(slot); + for (i = 0; i < limit; i++) { + if (storage->dptr(slot, i, (void *)&shm) != APR_SUCCESS) { + return NULL; + } + if ((balancer->s->hash.def == shm->hash.def) && + (balancer->s->hash.fnv == shm->hash.fnv)) { + *index = i; + return shm; + } + } + return NULL; +} + void proxy_util_register_hooks(apr_pool_t *p) { APR_REGISTER_OPTIONAL_FN(ap_proxy_retry_worker);
MaxMembersStickySessionDisableFailoverTimeoutFailoverAttemptsMethodPathActive