From 1b9292a4c930cfe9f0dd2b13d3fab89a9f0abd18 Mon Sep 17 00:00:00 2001 From: Jean-Frederic Clere Date: Mon, 11 Sep 2023 13:50:21 +0000 Subject: [PATCH] Arrange the bybusyness logic and prevent bad busy values this closes #383 git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1912245 13f79535-47bb-0310-9956-ffa450edef68 --- .../proxy/balancers/mod_lbmethod_bybusyness.c | 13 +- modules/proxy/mod_proxy_balancer.c | 18 +-- modules/proxy/proxy_util.c | 119 ++++++++++++++++++ modules/proxy/proxy_util.h | 35 ++++++ 4 files changed, 168 insertions(+), 17 deletions(-) diff --git a/modules/proxy/balancers/mod_lbmethod_bybusyness.c b/modules/proxy/balancers/mod_lbmethod_bybusyness.c index 80d538cdaaa..e714715eb36 100644 --- a/modules/proxy/balancers/mod_lbmethod_bybusyness.c +++ b/modules/proxy/balancers/mod_lbmethod_bybusyness.c @@ -15,6 +15,7 @@ */ #include "mod_proxy.h" +#include "proxy_util.h" #include "scoreboard.h" #include "ap_mpm.h" #include "apr_version.h" @@ -25,18 +26,24 @@ module AP_MODULE_DECLARE_DATA lbmethod_bybusyness_module; static APR_OPTIONAL_FN_TYPE(proxy_balancer_get_best_worker) *ap_proxy_balancer_get_best_worker_fn = NULL; + static int is_best_bybusyness(proxy_worker *current, proxy_worker *prev_best, void *baton) { int *total_factor = (int *)baton; + apr_size_t current_busy = getbusy_count(current); + apr_size_t prev_best_busy = 0; current->s->lbstatus += current->s->lbfactor; *total_factor += current->s->lbfactor; + if (prev_best) + prev_best_busy = getbusy_count(prev_best); + return ( !prev_best - || (current->s->busy < prev_best->s->busy) + || (current_busy < prev_best_busy) || ( - (current->s->busy == prev_best->s->busy) + (current_busy == prev_best_busy) && (current->s->lbstatus > prev_best->s->lbstatus) ) ); @@ -65,7 +72,7 @@ static apr_status_t reset(proxy_balancer *balancer, server_rec *s) worker = (proxy_worker **)balancer->workers->elts; for (i = 0; i < balancer->workers->nelts; i++, worker++) { (*worker)->s->lbstatus = 0; - (*worker)->s->busy = 0; + setbusy_count(*worker, 0); } return APR_SUCCESS; } diff --git a/modules/proxy/mod_proxy_balancer.c b/modules/proxy/mod_proxy_balancer.c index 0bf4e9db15b..97993afc52d 100644 --- a/modules/proxy/mod_proxy_balancer.c +++ b/modules/proxy/mod_proxy_balancer.c @@ -17,6 +17,7 @@ /* Load balancer module for Apache proxy */ #include "mod_proxy.h" +#include "proxy_util.h" #include "scoreboard.h" #include "ap_mpm.h" #include "apr_version.h" @@ -486,17 +487,6 @@ static void force_recovery(proxy_balancer *balancer, server_rec *s) } } -static apr_status_t decrement_busy_count(void *worker_) -{ - proxy_worker *worker = worker_; - - if (worker->s->busy) { - worker->s->busy--; - } - - return APR_SUCCESS; -} - static int proxy_balancer_pre_request(proxy_worker **worker, proxy_balancer **balancer, request_rec *r, @@ -635,7 +625,7 @@ static int proxy_balancer_pre_request(proxy_worker **worker, *worker = runtime; } - (*worker)->s->busy++; + increment_busy_count(*worker); apr_pool_cleanup_register(r->pool, *worker, decrement_busy_count, apr_pool_cleanup_null); @@ -1575,7 +1565,7 @@ static void balancer_display_page(request_rec *r, proxy_server_conf *conf, "\n", NULL); ap_rprintf(r, " %" APR_SIZE_T_FMT "\n", - worker->s->busy); + getbusy_count(worker)); ap_rprintf(r, " %d\n", worker->s->lbset); /* End proxy_worker_stat */ @@ -1748,7 +1738,7 @@ static void balancer_display_page(request_rec *r, proxy_server_conf *conf, ap_rvputs(r, ap_proxy_parse_wstatus(r->pool, worker), NULL); ap_rputs("", r); ap_rprintf(r, "%" APR_SIZE_T_FMT "", worker->s->elected); - ap_rprintf(r, "%" APR_SIZE_T_FMT "", worker->s->busy); + ap_rprintf(r, "%" APR_SIZE_T_FMT "", getbusy_count(worker)); ap_rprintf(r, "%d", worker->s->lbstatus); ap_rputs(apr_strfsize(worker->s->transferred, fbuf), r); ap_rputs("", r); diff --git a/modules/proxy/proxy_util.c b/modules/proxy/proxy_util.c index b41f3ced3b1..dfa030ff482 100644 --- a/modules/proxy/proxy_util.c +++ b/modules/proxy/proxy_util.c @@ -21,6 +21,7 @@ #include "apr_version.h" #include "apr_strings.h" #include "apr_hash.h" +#include "apr_atomic.h" #include "http_core.h" #include "proxy_util.h" #include "ajp.h" @@ -4984,6 +4985,124 @@ PROXY_DECLARE(apr_status_t) ap_proxy_tunnel_create(proxy_tunnel_rec **ptunnel, return APR_SUCCESS; } +PROXY_DECLARE(apr_status_t) decrement_busy_count(void *worker_) +{ + apr_size_t val; + proxy_worker *worker = worker_; + +#if APR_SIZEOF_VOIDP == 4 + AP_DEBUG_ASSERT(sizeof(apr_size_t) == sizeof(apr_uint32_t)); + val = apr_atomic_read32(&worker->s->busy); + while (val > 0) { + apr_size_t old = val; + val = apr_atomic_cas32(&worker->s->busy, val - 1, old); + if (val == old) { + break; + } + } +#elif APR_VERSION_AT_LEAST(1,7,4) /* APR 64bit atomics not safe before 1.7.4 */ + AP_DEBUG_ASSERT(sizeof(apr_size_t) == sizeof(apr_uint64_t)); + val = apr_atomic_read64(&worker->s->busy); + while (val > 0) { + apr_size_t old = val; + val = apr_atomic_cas64(&worker->s->busy, val - 1, old); + if (val == old) { + break; + } + } +#else /* Use atomics for (64bit) pointers */ + void *volatile *busy_p = (void *)&worker->s->busy; + AP_DEBUG_ASSERT(sizeof(apr_size_t) == sizeof(void*)); + AP_DEBUG_ASSERT((apr_uintptr_t)busy_p % sizeof(void*) == 0); + val = (apr_uintptr_t)apr_atomic_casptr((void *)busy_p, NULL, NULL); + while (val > 0) { + apr_size_t old = val; + val = (apr_uintptr_t)apr_atomic_casptr((void *)busy_p, + (void *)(apr_uintptr_t)(val - 1), + (void *)(apr_uintptr_t)old); + if (val == old) { + break; + } + } +#endif + return APR_SUCCESS; +} + +PROXY_DECLARE(void) increment_busy_count(proxy_worker *worker) +{ + apr_size_t val; +#if APR_SIZEOF_VOIDP == 4 + AP_DEBUG_ASSERT(sizeof(apr_size_t) == sizeof(apr_uint32_t)); + val = apr_atomic_read32(&worker->s->busy); + while (val < APR_INT32_MAX) { + apr_size_t old = val; + val = apr_atomic_cas32(&worker->s->busy, val + 1, old); + if (val == old) { + break; + } + } +#elif APR_VERSION_AT_LEAST(1,7,4) /* APR 64bit atomics not safe before 1.7.4 */ + AP_DEBUG_ASSERT(sizeof(apr_size_t) == sizeof(apr_uint64_t)); + val = apr_atomic_read64(&worker->s->busy); + while (val < APR_INT64_MAX) { + apr_size_t old = val; + val = apr_atomic_cas64(&worker->s->busy, val + 1, old); + if (val == old) { + break; + } + } +#else /* Use atomics for (64bit) pointers */ + void *volatile *busy_p = (void *)&worker->s->busy; + AP_DEBUG_ASSERT(sizeof(apr_size_t) == sizeof(void*)); + AP_DEBUG_ASSERT((apr_uintptr_t)busy_p % sizeof(void*) == 0); + val = (apr_uintptr_t)apr_atomic_casptr((void *)busy_p, NULL, NULL); + while (val < APR_INT64_MAX) { + apr_size_t old = val; + val = (apr_uintptr_t)apr_atomic_casptr((void *)busy_p, + (void *)(apr_uintptr_t)(val + 1), + (void *)(apr_uintptr_t)old); + if (val == old) { + break; + } + } +#endif +} + +PROXY_DECLARE(apr_size_t) getbusy_count(proxy_worker *worker) +{ + apr_size_t val; +#if APR_SIZEOF_VOIDP == 4 + AP_DEBUG_ASSERT(sizeof(apr_size_t) == sizeof(apr_uint32_t)); + val = apr_atomic_read32(&worker->s->busy); +#elif APR_VERSION_AT_LEAST(1,7,4) /* APR 64bit atomics not safe before 1.7.4 */ + AP_DEBUG_ASSERT(sizeof(apr_size_t) == sizeof(apr_uint64_t)); + val = apr_atomic_read64(&worker->s->busy); +#else /* Use atomics for (64bit) pointers */ + void *volatile *busy_p = (void *)&worker->s->busy; + AP_DEBUG_ASSERT(sizeof(apr_size_t) == sizeof(void*)); + AP_DEBUG_ASSERT((apr_uintptr_t)busy_p % sizeof(void*) == 0); + val = (apr_uintptr_t)apr_atomic_casptr((void *)busy_p, NULL, NULL); +#endif + + return val; +} + +PROXY_DECLARE(void) setbusy_count(proxy_worker *worker, apr_size_t to) +{ +#if APR_SIZEOF_VOIDP == 4 + AP_DEBUG_ASSERT(sizeof(apr_size_t) == sizeof(apr_uint32_t)); + apr_atomic_set32(&worker->s->busy, to); +#elif APR_VERSION_AT_LEAST(1,7,4) /* APR 64bit atomics not safe before 1.7.4 */ + AP_DEBUG_ASSERT(sizeof(apr_size_t) == sizeof(apr_uint64_t)); + apr_atomic_set64(&worker->s->busy, to); +#else /* Use atomics for (64bit) pointers */ + void *volatile *busy_p = (void *)&worker->s->busy; + AP_DEBUG_ASSERT(sizeof(apr_size_t) == sizeof(void*)); + AP_DEBUG_ASSERT((apr_uintptr_t)busy_p % sizeof(void*) == 0); + apr_atomic_xchgptr((void *)busy_p, (void *)(apr_uintptr_t)to); +#endif +} + static void add_pollset(apr_pollset_t *pollset, apr_pollfd_t *pfd, apr_int16_t events) { diff --git a/modules/proxy/proxy_util.h b/modules/proxy/proxy_util.h index bc131da0f0f..0dde2487bb2 100644 --- a/modules/proxy/proxy_util.h +++ b/modules/proxy/proxy_util.h @@ -40,6 +40,41 @@ extern PROXY_DECLARE_DATA const apr_strmatch_pattern *ap_proxy_strmatch_domain; */ void proxy_util_register_hooks(apr_pool_t *p); +/* + * Get the busy counter from the shared worker memory + * + * @param worker Pointer to the worker structure. + * @return apr_size_t value atomically read for the worker. + */ +PROXY_DECLARE(apr_size_t) getbusy_count(proxy_worker *worker); + +/* + * Set the busy counter from the shared worker memory + * + * @param worker Pointer to the worker structure. + * @param to value to set the busy counter. + * @return void + */ +PROXY_DECLARE(void) setbusy_count(proxy_worker *worker, apr_size_t to); + +/* + * decrement the busy counter from the shared worker memory + * note it is called by apr_pool_cleanup_register() + * therfore the void * and apr_status_t. + * + * @param worker_ Pointer to the worker structure. + * @return apr_status_t returns APR_SUCCESS. + */ +PROXY_DECLARE(apr_status_t) decrement_busy_count(void *worker_); + +/* + * increment the busy counter from the shared worker memory + * + * @param worker Pointer to the worker structure. + * @return void + */ +PROXY_DECLARE(void) increment_busy_count(proxy_worker *worker); + /** @} */ #endif /* PROXY_UTIL_H_ */