Plüm wrote:
>
>> -----Ursprüngliche Nachricht-----
>> Von: jean-frederic clere
>> Gesendet: Montag, 10. September 2007 16:38
>> An: [email protected]
>> Betreff: Re: svn commit: r573264 -
>> /httpd/httpd/trunk/include/scoreboard.h
>>
>>
>> Jim Jagielski wrote:
>>> On Sep 10, 2007, at 6:37 AM, Plüm, Rüdiger, VF-Group wrote:
>>>
>>>>> For example what about adding:
>>>>> static APR_OPTIONAL_FN_TYPE(ap_proxy_lb_worker_size)
>>>>> *proxy_lb_worker_size;
>>>>> and use a void * in scoreboard and an int for the size?
>>>> For me this sounds fine, but I would guess that Jim doesn't like
>>>> the void * idea in the scoreboard.
>>>>
>>> I don't mind it at all, if we use it because we don't know
>>> what will be stored away or because we may use the storage
>>> differently at different times. But this is never the
>>> case. lb_score always is proxy_worker_stat.
>>>
>>>
>> The attached patch remove "lb_score" from scoreboard.c.
>>
>> Comments?
>
> 1. IMHO requires a minor bump.
Find a patch that covers all the points you raised below.
More comments?
Cheers
Jean-Frederic
> 2. Why messing around with lb_score any longer? Instead of
>
> return sizeof(lb_score);
>
> we could do
>
> return sizeof(proxy_worker_stat);
>
> 3. We should register the optional functions ap_proxy_lb_workers and
> ap_proxy_lb_worker_size in the same place and should keep the code
> of them next to each other. With your patch they are distributed over
> mod_proxy.c and mod_proxy_balancer.c. IMHO they belong to mod_proxy.c
> as we call ap_proxy_initialize_worker_share from mod_proxy's child init
> (so even if we do not load mod_proxy_balancer we need the scoreboard
> entries).
> 4. All
> if (lb_limit)
> in scoreboard.c should be extended to
> if (lb_limit && worker_size)
>
>
> Regards
>
> Rüdiger
>
>
Index: server/scoreboard.c
===================================================================
--- server/scoreboard.c (revision 574500)
+++ server/scoreboard.c (working copy)
@@ -62,13 +62,15 @@
static APR_OPTIONAL_FN_TYPE(ap_proxy_lb_workers)
*pfn_proxy_lb_workers;
+static APR_OPTIONAL_FN_TYPE(ap_proxy_lb_worker_size)
+ *pfn_proxy_lb_worker_size;
struct ap_sb_handle_t {
int child_num;
int thread_num;
};
-static int server_limit, thread_limit, lb_limit;
+static int server_limit, thread_limit, lb_limit, worker_size;
static apr_size_t scoreboard_size;
/*
@@ -101,11 +103,18 @@
else
lb_limit = 0;
+ if (!pfn_proxy_lb_worker_size)
+ pfn_proxy_lb_worker_size =
APR_RETRIEVE_OPTIONAL_FN(ap_proxy_lb_worker_size);
+ if (pfn_proxy_lb_worker_size)
+ worker_size = pfn_proxy_lb_worker_size();
+ else
+ worker_size = 0;
+
scoreboard_size = sizeof(global_score);
scoreboard_size += sizeof(process_score) * server_limit;
scoreboard_size += sizeof(worker_score) * server_limit * thread_limit;
- if (lb_limit)
- scoreboard_size += sizeof(lb_score) * lb_limit;
+ if (lb_limit && worker_size)
+ scoreboard_size += worker_size * lb_limit;
return scoreboard_size;
}
@@ -129,9 +138,9 @@
ap_scoreboard_image->servers[i] = (worker_score *)more_storage;
more_storage += thread_limit * sizeof(worker_score);
}
- if (lb_limit) {
- ap_scoreboard_image->balancers = (lb_score *)more_storage;
- more_storage += lb_limit * sizeof(lb_score);
+ if (lb_limit && worker_size) {
+ ap_scoreboard_image->balancers = (void *)more_storage;
+ more_storage += lb_limit * worker_size;
}
ap_assert(more_storage == (char*)shared_score + scoreboard_size);
ap_scoreboard_image->global->server_limit = server_limit;
@@ -281,9 +290,9 @@
sizeof(worker_score) * thread_limit);
}
/* Clean up the lb workers data */
- if (lb_limit) {
+ if (lb_limit && worker_size) {
memset(ap_scoreboard_image->balancers, 0,
- sizeof(lb_score) * lb_limit);
+ worker_size * lb_limit);
}
return OK;
}
@@ -490,8 +499,10 @@
AP_DECLARE(lb_score *) ap_get_scoreboard_lb(int lb_num)
{
- if (((lb_num < 0) || (lb_limit < lb_num))) {
+ char *ptr;
+ if (((lb_num < 0) || (lb_limit < lb_num)) || worker_size==0) {
return(NULL); /* Out of range */
}
- return &ap_scoreboard_image->balancers[lb_num];
+ ptr = (char *) ap_scoreboard_image->balancers + lb_num*worker_size;
+ return (lb_score *) ptr;
}
Index: modules/proxy/mod_proxy_balancer.c
===================================================================
--- modules/proxy/mod_proxy_balancer.c (revision 574500)
+++ modules/proxy/mod_proxy_balancer.c (working copy)
@@ -19,7 +19,6 @@
#define CORE_PRIVATE
#include "mod_proxy.h"
-#include "scoreboard.h"
#include "ap_mpm.h"
#include "apr_version.h"
#include "apr_hooks.h"
Index: modules/proxy/proxy_util.c
===================================================================
--- modules/proxy/proxy_util.c (revision 574500)
+++ modules/proxy/proxy_util.c (working copy)
@@ -1670,7 +1670,7 @@
proxy_worker *worker,
server_rec *s)
{
- lb_score *score = NULL;
+ proxy_worker_stat *score = NULL;
if (PROXY_WORKER_IS_INITIALIZED(worker)) {
/* The worker share is already initialized */
@@ -1681,7 +1681,7 @@
}
/* Get scoreboard slot */
if (ap_scoreboard_image) {
- score = ap_get_scoreboard_lb(worker->id);
+ score = (proxy_worker_stat *) ap_get_scoreboard_lb(worker->id);
if (!score) {
ap_log_error(APLOG_MARK, APLOG_ERR, 0, s,
"proxy: ap_get_scoreboard_lb(%d) failed in child %"
APR_PID_T_FMT " for worker %s",
@@ -1694,12 +1694,12 @@
}
}
if (!score) {
- score = apr_pcalloc(conf->pool, sizeof(proxy_worker_stat));
+ score = (proxy_worker_stat *) apr_pcalloc(conf->pool,
sizeof(proxy_worker_stat));
ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
"proxy: initialized plain memory in child %" APR_PID_T_FMT " for
worker %s",
getpid(), worker->name);
}
- worker->s = (proxy_worker_stat *)score;
+ worker->s = score;
/*
* recheck to see if we've already been here. Possible
* if proxy is using scoreboard to hold shared stats
Index: modules/proxy/mod_proxy.c
===================================================================
--- modules/proxy/mod_proxy.c (revision 574500)
+++ modules/proxy/mod_proxy.c (working copy)
@@ -37,6 +37,33 @@
#define MAX(x,y) ((x) >= (y) ? (x) : (y))
#endif
+/* Global balancer counter */
+static int lb_workers_limit = 0;
+
+/**
+ * Calculate number of maximum number of workers in scoreboard.
+ * @return number of workers to allocate in the scoreboard
+ */
+static int ap_proxy_lb_workers(void)
+{
+ /*
+ * Since we can't resize the scoreboard when reconfiguring, we
+ * have to impose a limit on the number of workers, we are
+ * able to reconfigure to.
+ */
+ if (!lb_workers_limit)
+ lb_workers_limit = proxy_lb_workers + PROXY_DYNAMIC_BALANCER_LIMIT;
+ return lb_workers_limit;
+}
+
+/* return the sizeof of one lb_worker in scoreboard. */
+static int ap_proxy_lb_worker_size(void)
+{
+ return sizeof(proxy_worker_stat);
+}
+
+
+
/*
* A Web proxy module. Stages:
*
@@ -2216,6 +2243,7 @@
static const char *const aszPred[] = { "mpm_winnt.c", NULL};
APR_REGISTER_OPTIONAL_FN(ap_proxy_lb_workers);
+ APR_REGISTER_OPTIONAL_FN(ap_proxy_lb_worker_size);
/* handler */
ap_hook_handler(proxy_handler, NULL, NULL, APR_HOOK_FIRST);
/* filename-to-URI translation */
Index: modules/proxy/mod_proxy.h
===================================================================
--- modules/proxy/mod_proxy.h (revision 574500)
+++ modules/proxy/mod_proxy.h (working copy)
@@ -730,11 +730,6 @@
* If this limit is reached you must stop and restart the server.
*/
#define PROXY_DYNAMIC_BALANCER_LIMIT 16
-/**
- * Calculate number of maximum number of workers in scoreboard.
- * @return number of workers to allocate in the scoreboard
- */
-int ap_proxy_lb_workers(void);
/* For proxy_util */
extern module PROXY_DECLARE_DATA proxy_module;
Index: include/scoreboard.h
===================================================================
--- include/scoreboard.h (revision 574500)
+++ include/scoreboard.h (working copy)
@@ -143,9 +143,6 @@
/* stuff which is lb specific */
typedef struct lb_score lb_score;
-struct lb_score {
- unsigned char data[1024];
-};
/* Scoreboard is now in 'local' memory, since it isn't updated once created,
* even in forked architectures. Child created-processes (non-fork) will
@@ -205,6 +202,12 @@
*/
APR_DECLARE_OPTIONAL_FN(int, ap_proxy_lb_workers,
(void));
+/**
+ * proxy load balancer
+ * @return the size of lb_workers.
+ */
+APR_DECLARE_OPTIONAL_FN(int, ap_proxy_lb_worker_size,
+ (void));
/* for time_process_request() in http_main.c */
#define START_PREQUEST 1
