You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Ruediger Pluem <rp...@apache.org> on 2009/07/07 21:05:33 UTC

Re: svn commit: r791617 - in /httpd/httpd/trunk/modules: cluster/mod_heartmonitor.c proxy/balancers/mod_lbmethod_heartbeat.c


On 07/06/2009 11:14 PM, jfclere@apache.org wrote:
> Author: jfclere
> Date: Mon Jul  6 21:14:21 2009
> New Revision: 791617
> 
> URL: http://svn.apache.org/viewvc?rev=791617&view=rev
> Log:
> Add use slotmem. Directive HeartbeatMaxServers > 10 to activate the logic.
> Otherwise it uses the file logic to store the heartbeats.
> 
> Modified:
>     httpd/httpd/trunk/modules/cluster/mod_heartmonitor.c
>     httpd/httpd/trunk/modules/proxy/balancers/mod_lbmethod_heartbeat.c
> 
> Modified: httpd/httpd/trunk/modules/cluster/mod_heartmonitor.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/cluster/mod_heartmonitor.c?rev=791617&r1=791616&r2=791617&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/cluster/mod_heartmonitor.c (original)
> +++ httpd/httpd/trunk/modules/cluster/mod_heartmonitor.c Mon Jul  6 21:14:21 2009

>  
> @@ -440,7 +530,17 @@
>          return HTTP_INTERNAL_SERVER_ERROR;
>      }
>      apr_brigade_flatten(input_brigade, buf, &len);
> -    hm_processmsg(ctx, r->pool, r->connection->remote_addr, buf, len);
> +
> +    /* we can't use hm_processmsg because it uses hm_get_server() */
> +    buf[len] = '\0';
> +    tbl = apr_table_make(r->pool, 10);
> +    qs_to_table(buf, tbl, r->pool);
> +    apr_sockaddr_ip_get(&ip, r->connection->remote_addr);
> +    hmserver.ip = ip;
> +    hmserver.busy = atoi(apr_table_get(tbl, "busy"));
> +    hmserver.ready = atoi(apr_table_get(tbl, "ready"));
> +    hmserver.seen = apr_time_now();
> +    hm_slotmem_update_stat(&hmserver, r);

Sorry for being confused, but this means that we are storing the data in different
locations dependent on whether we use the handler or the UDP listener and more so
we provide them in different locations for other modules to use (sharedmem / file).
Does this make sense?
IMHO we should either provide them in both locations (sharedmem / file) no matter
which source contributed it or we should make it configurable where this information
is offered.


> Modified: httpd/httpd/trunk/modules/proxy/balancers/mod_lbmethod_heartbeat.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/balancers/mod_lbmethod_heartbeat.c?rev=791617&r1=791616&r2=791617&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/proxy/balancers/mod_lbmethod_heartbeat.c (original)
> +++ httpd/httpd/trunk/modules/proxy/balancers/mod_lbmethod_heartbeat.c Mon Jul  6 21:14:21 2009

> @@ -39,9 +47,20 @@
>      int busy;
>      int ready;
>      int seen;
> +    int id;
>      proxy_worker *worker;
>  } hb_server_t;
>  
> +#define MAXIPSIZE  64
> +typedef struct hm_slot_server_t
> +{
> +    char ip[MAXIPSIZE];
> +    int busy;
> +    int ready;
> +    apr_time_t seen;
> +    int id;
> +} hm_slot_server_t;
> +

Shouldn't these things go to a common include file?
I guess defining them in each file is waiting for a missed-to-update
error to happen.


Regards

RĂ¼diger

Re: svn commit: r791617 - in /httpd/httpd/trunk/modules: cluster/mod_heartmonitor.c proxy/balancers/mod_lbmethod_heartbeat.c

Posted by jean-frederic clere <jf...@gmail.com>.
On 07/07/2009 09:05 PM, Ruediger Pluem wrote:
>
> On 07/06/2009 11:14 PM, jfclere@apache.org wrote:
>> Author: jfclere
>> Date: Mon Jul  6 21:14:21 2009
>> New Revision: 791617
>>
>> URL: http://svn.apache.org/viewvc?rev=791617&view=rev
>> Log:
>> Add use slotmem. Directive HeartbeatMaxServers>  10 to activate the logic.
>> Otherwise it uses the file logic to store the heartbeats.
>>
>> Modified:
>>      httpd/httpd/trunk/modules/cluster/mod_heartmonitor.c
>>      httpd/httpd/trunk/modules/proxy/balancers/mod_lbmethod_heartbeat.c
>>
>> Modified: httpd/httpd/trunk/modules/cluster/mod_heartmonitor.c
>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/cluster/mod_heartmonitor.c?rev=791617&r1=791616&r2=791617&view=diff
>> ==============================================================================
>> --- httpd/httpd/trunk/modules/cluster/mod_heartmonitor.c (original)
>> +++ httpd/httpd/trunk/modules/cluster/mod_heartmonitor.c Mon Jul  6 21:14:21 2009
>
>>
>> @@ -440,7 +530,17 @@
>>           return HTTP_INTERNAL_SERVER_ERROR;
>>       }
>>       apr_brigade_flatten(input_brigade, buf,&len);
>> -    hm_processmsg(ctx, r->pool, r->connection->remote_addr, buf, len);
>> +
>> +    /* we can't use hm_processmsg because it uses hm_get_server() */
>> +    buf[len] = '\0';
>> +    tbl = apr_table_make(r->pool, 10);
>> +    qs_to_table(buf, tbl, r->pool);
>> +    apr_sockaddr_ip_get(&ip, r->connection->remote_addr);
>> +    hmserver.ip = ip;
>> +    hmserver.busy = atoi(apr_table_get(tbl, "busy"));
>> +    hmserver.ready = atoi(apr_table_get(tbl, "ready"));
>> +    hmserver.seen = apr_time_now();
>> +    hm_slotmem_update_stat(&hmserver, r);
>
> Sorry for being confused, but this means that we are storing the data in different
> locations dependent on whether we use the handler or the UDP listener and more so
> we provide them in different locations for other modules to use (sharedmem / file).
> Does this make sense?

The file logic for the handler is tricky, I need to work on it.

> IMHO we should either provide them in both locations (sharedmem / file) no matter
> which source contributed it or we should make it configurable where this information
> is offered.

Yep.

>
>
>> Modified: httpd/httpd/trunk/modules/proxy/balancers/mod_lbmethod_heartbeat.c
>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/balancers/mod_lbmethod_heartbeat.c?rev=791617&r1=791616&r2=791617&view=diff
>> ==============================================================================
>> --- httpd/httpd/trunk/modules/proxy/balancers/mod_lbmethod_heartbeat.c (original)
>> +++ httpd/httpd/trunk/modules/proxy/balancers/mod_lbmethod_heartbeat.c Mon Jul  6 21:14:21 2009
>
>> @@ -39,9 +47,20 @@
>>       int busy;
>>       int ready;
>>       int seen;
>> +    int id;
>>       proxy_worker *worker;
>>   } hb_server_t;
>>
>> +#define MAXIPSIZE  64
>> +typedef struct hm_slot_server_t
>> +{
>> +    char ip[MAXIPSIZE];
>> +    int busy;
>> +    int ready;
>> +    apr_time_t seen;
>> +    int id;
>> +} hm_slot_server_t;
>> +
>
> Shouldn't these things go to a common include file?
> I guess defining them in each file is waiting for a missed-to-update
> error to happen.

I will move that in a common include file.

Cheers

Jean-Frederic