httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jean-frederic clere <jfcl...@gmail.com>
Subject Re: svn commit: r573264 - /httpd/httpd/trunk/include/scoreboard.h
Date Wed, 12 Sep 2007 10:14:37 GMT
Plüm wrote:
> 
>> -----Ursprüngliche Nachricht-----
>> Von: jean-frederic clere 
>> Gesendet: Mittwoch, 12. September 2007 10:34
>> An: dev@httpd.apache.org
>> Betreff: Re: svn commit: r573264 - 
>> /httpd/httpd/trunk/include/scoreboard.h
>>
>>
>> Ruediger Pluem wrote:
>>> On 09/11/2007 10:51 PM, Jim Jagielski wrote:
>>>
>>>> I would also suggest that we keep ap_proxy_lb_workers
>>>> in proxy_util.c (as it currently is), since even
>>>> though its not part of the API, other proxy module
>>>> make have a need for it (and again, keeping the
>>>> number of changes to a minimum).
>>> +1. That means also to keep it in mod_proxy.h.
>>> But we shouldn't do the same mistake with ap_proxy_lb_worker_size().
>>> So keep it static in mod_proxy.c.
>> Ok I have committed the patch taking in account all the comments.
> 
> Sorry, but I have still some comments :-).
> 
> Modified: httpd/httpd/trunk/modules/proxy/mod_proxy.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy.c?rev=574843&r1=574842&r2=574843&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/proxy/mod_proxy.c (original)
> +++ httpd/httpd/trunk/modules/proxy/mod_proxy.c Wed Sep 12 01:34:40 2007
> @@ -37,6 +37,17 @@
>  #define MAX(x,y) ((x) >= (y) ? (x) : (y))
>  #endif
>  
> +/* Global balancer counter */
> +static int lb_workers_limit = 0;
> 
> Why having it here again? It is not used in mod_proxy.c and we have it in proxy_util.c

Oops fixed. Thanks.

> 
> Modified: httpd/httpd/trunk/server/scoreboard.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/scoreboard.c?rev=574843&r1=574842&r2=574843&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/server/scoreboard.c (original)
> +++ httpd/httpd/trunk/server/scoreboard.c Wed Sep 12 01:34:40 2007
> @@ -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 = sizeof(lb_score);
> +
>      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)
> 
> The check for worker_size is not needed as worker_size is never zero.

Except if pfn_proxy_lb_worker_size() returns 0. But that means a
modified mod_proxy. Should we support than?

Cheers

Jean-Frederic

> 
> +        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) {
> 
> The check for worker_size is not needed as worker_size is never zero.
> 
> +        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) {
> 
> The check for worker_size is not needed as worker_size is never zero.
> 
>              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)) {
> 
> The check for worker_size is not needed as worker_size is never zero.
> 
>          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;
>  }
> 
> 
> Regards
> 
> Rüdiger
> 
> 


Mime
View raw message