You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by jf...@apache.org on 2009/07/06 23:14:21 UTC

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

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
@@ -25,6 +25,7 @@
 #include "ap_mpm.h"
 #include "scoreboard.h"
 #include "mod_watchdog.h"
+#include "ap_slotmem.h"
 
 
 #ifndef HM_UPDATE_SEC
@@ -35,6 +36,12 @@
 
 #define HM_WATHCHDOG_NAME ("_heartmonitor_")
 
+#define MAXIPSIZE  64
+
+const ap_slotmem_provider_t *storage = NULL;
+static ap_slotmem_instance_t *slotmem = NULL;
+static int maxworkers = 0;
+
 module AP_MODULE_DECLARE_DATA heartmonitor_module;
 
 typedef struct hm_server_t
@@ -45,6 +52,15 @@
     apr_time_t seen;
 } hm_server_t;
 
+typedef struct hm_slot_server_t
+{
+    char ip[MAXIPSIZE];
+    int busy;
+    int ready;
+    apr_time_t seen;
+    int id;
+} hm_slot_server_t;
+
 typedef struct hm_ctx_t
 {
     int active;
@@ -60,6 +76,11 @@
     server_rec *s;
 } hm_ctx_t;
 
+typedef struct hm_slot_server_ctx_t {
+  hm_server_t *s;
+  int updated;
+} hm_slot_server_ctx_t;
+
 static apr_status_t hm_listen(hm_ctx_t *ctx)
 {
     apr_status_t rv;
@@ -151,7 +172,50 @@
 
 #define SEEN_TIMEOUT (30)
 
-static apr_status_t hm_update_stats(hm_ctx_t *ctx, apr_pool_t *p)
+/* Store in the slotmem */
+static apr_status_t hm_update(void* mem, void *data, apr_pool_t *p)
+{
+    hm_slot_server_t *old = (hm_slot_server_t *) mem;
+    hm_slot_server_ctx_t *s = (hm_slot_server_ctx_t *) data;
+    hm_server_t *new = s->s;
+    if (strncmp(old->ip, new->ip, MAXIPSIZE)==0) {
+        s->updated = 1;
+        old->busy = new->busy;
+        old->ready = new->ready;
+        old->seen = new->seen;
+    }
+    return APR_SUCCESS;
+}
+static  apr_status_t  hm_slotmem_update_stat(hm_server_t *s, request_rec *r)
+{
+    /* We call do_all (to try to update) otherwise grab + put */
+    hm_slot_server_ctx_t ctx;
+
+    /* TODO: REMOVE ME BEFORE PRODUCTION (????) */
+    ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
+                 "Heartmonitor: %s busy=%d ready=%d", s->ip,
+                 s->busy, s->ready);
+
+    ctx.s = s;
+    ctx.updated = 0;
+    storage->doall(slotmem, hm_update, &ctx, r->pool);
+    if (!ctx.updated) {
+        int i;
+        hm_slot_server_t hmserver;
+        memcpy(hmserver.ip, s->ip, MAXIPSIZE);
+        hmserver.busy = s->busy;
+        hmserver.ready = s->ready;
+        hmserver.seen = s->seen;
+        /* XXX locking for grab() / put() */
+        storage->grab(slotmem, &i);
+        hmserver.id = i;
+        storage->put(slotmem, i, (char *)&hmserver, sizeof(hmserver));
+    }
+    return APR_SUCCESS;
+}
+
+/* Store in a file */
+static apr_status_t hm_file_update_stats(hm_ctx_t *ctx, apr_pool_t *p)
 {
     apr_status_t rv;
     apr_file_t *fp;
@@ -333,7 +397,8 @@
             }
         break;
         case AP_WATCHDOG_STATE_RUNNING:
-            hm_update_stats(ctx, pool);
+            /* XXX slotmem, if used by the handler is that looks a bad ideas (we are not the one receiving the information  */
+            hm_file_update_stats(ctx, pool);
             cur = now = apr_time_sec(apr_time_now());
             /* TODO: Insted HN_UPDATE_SEC use
              * the ctx->interval
@@ -386,9 +451,32 @@
                           apr_pool_t *ptemp, server_rec *s)
 {
     apr_status_t rv;
+    const char *userdata_key = "mod_heartmonitor_init";
+    void *data;
     hm_ctx_t *ctx = ap_get_module_config(s->module_config,
                                          &heartmonitor_module);
 
+
+    /* Create the slotmem */
+    apr_pool_userdata_get(&data, userdata_key, s->process->pool);
+    if (!data) {
+        /* first call do nothing */
+        apr_pool_userdata_set((const void *)1, userdata_key, apr_pool_cleanup_null, s->process->pool);
+    } else {
+        if (maxworkers) {
+            storage = ap_lookup_provider(AP_SLOTMEM_PROVIDER_GROUP, "shared", "0");
+            if (!storage) {
+                ap_log_error(APLOG_MARK, APLOG_NOERRNO|APLOG_EMERG, 0, s, "ap_lookup_provider %s failed", AP_SLOTMEM_PROVIDER_GROUP);
+                return !OK;
+            }
+            storage->create(&slotmem, "mod_heartmonitor", sizeof(hm_slot_server_t), maxworkers, AP_SLOTMEM_TYPE_PREGRAB, p);
+            if (!slotmem) {
+                ap_log_error(APLOG_MARK, APLOG_NOERRNO|APLOG_EMERG, 0, s, "slotmem_create for status failed");
+                return !OK;
+            }
+        }
+    }
+
     if (!ctx->active) {
         return OK;
     }
@@ -414,7 +502,6 @@
     }
     ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
                  "Heartmonitor: wd callback %s", HM_WATHCHDOG_NAME);
-
     return OK;
 }
 
@@ -424,6 +511,9 @@
     apr_size_t len=MAX_MSG_LEN;
     char *buf;
     apr_status_t status;
+    apr_table_t *tbl;
+    hm_server_t hmserver;
+    char *ip;
     hm_ctx_t *ctx = ap_get_module_config(r->server->module_config,
                                          &heartmonitor_module);
 
@@ -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);
 
     ap_set_content_type(r, "text/plain");
     ap_set_content_length(r, 2);
@@ -541,11 +641,33 @@
     return NULL;
 }
 
+static const char *cmd_hm_maxworkers(cmd_parms *cmd,
+                                  void *dconf, const char *data)
+{
+    apr_pool_t *p = cmd->pool;
+    hm_ctx_t *ctx =
+        (hm_ctx_t *) ap_get_module_config(cmd->server->module_config,
+                                          &heartmonitor_module);
+    const char *err = ap_check_cmd_context(cmd, GLOBAL_ONLY);
+
+    if (err != NULL) {
+        return err;
+    }
+
+    maxworkers = atoi(data);
+    if (maxworkers < 10)
+        return "HeartbeatMaxServers: Should be bigger than 10"; 
+
+    return NULL;
+}
+
 static const command_rec hm_cmds[] = {
     AP_INIT_TAKE1("HeartbeatListen", cmd_hm_listen, NULL, RSRC_CONF,
                   "Address to listen for heartbeat requests"),
     AP_INIT_TAKE1("HeartbeatStorage", cmd_hm_storage, NULL, RSRC_CONF,
                   "Path to store heartbeat data."),
+    AP_INIT_TAKE1("HeartbeatMaxServers", cmd_hm_maxworkers, NULL, RSRC_CONF,
+                  "Max number of servers when using slotmem (instead file) to store heartbeat data."),
     {NULL}
 };
 

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
@@ -19,6 +19,7 @@
 #include "ap_mpm.h"
 #include "apr_version.h"
 #include "apr_hooks.h"
+#include "ap_slotmem.h"
 
 #ifndef LBM_HEARTBEAT_MAX_LASTSEEN
 /* If we haven't seen a heartbeat in the last N seconds, don't count this IP
@@ -29,6 +30,13 @@
 
 module AP_MODULE_DECLARE_DATA lbmethod_heartbeat_module;
 
+const ap_slotmem_provider_t *storage = NULL;
+static ap_slotmem_instance_t *hm_serversmem = NULL;
+
+/*
+ * configuration structure
+ * path: path of the file where the heartbeat information is stored.
+ */
 typedef struct lb_hb_ctx_t
 {
     const char *path;
@@ -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;
+
 static void
 argstr_to_table(apr_pool_t *p, char *str, apr_table_t *parms)
 {
@@ -70,7 +89,7 @@
     }
 }
 
-static apr_status_t read_heartbeats(const char *path, apr_hash_t *servers,
+static apr_status_t readfile_heartbeats(const char *path, apr_hash_t *servers,
                                     apr_pool_t *pool)
 {
     apr_finfo_t fi;
@@ -187,6 +206,35 @@
     return APR_SUCCESS;
 }
 
+static apr_status_t hm_read(void* mem, void *data, apr_pool_t *pool)
+{
+    hm_slot_server_t *slotserver = (hm_slot_server_t *) mem;
+    apr_hash_t *servers = (apr_hash_t *) data;
+    hb_server_t *server = apr_hash_get(servers, slotserver->ip, APR_HASH_KEY_STRING);
+    if (server == NULL) {
+        server = apr_pcalloc(pool, sizeof(hb_server_t));
+        server->ip = apr_pstrdup(pool, slotserver->ip);
+        server->seen = -1;
+
+        apr_hash_set(servers, server->ip, APR_HASH_KEY_STRING, server);
+
+    }
+    server->busy = slotserver->busy;
+    server->ready = slotserver->ready;
+    server->seen = slotserver->seen;
+    server->id = slotserver->id;
+    if (server->busy == 0 && server->ready != 0) {
+        server->ready = server->ready / 4;
+    }
+    return APR_SUCCESS;
+}
+static apr_status_t readslot_heartbeats(apr_hash_t *servers,
+                                    apr_pool_t *pool)
+{
+    storage->doall(hm_serversmem, hm_read, servers, pool);
+    return APR_SUCCESS;
+}
+
 /*
  * Finding a random number in a range. 
  *      n' = a + n(b-a+1)/(M+1)
@@ -238,7 +286,10 @@
 
     servers = apr_hash_make(tpool);
 
-    rv = read_heartbeats(ctx->path, servers, tpool);
+    if (hm_serversmem)
+        rv = readslot_heartbeats(servers, tpool);
+    else
+        rv = readfile_heartbeats(ctx->path, servers, tpool);
 
     if (rv) {
         ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r,
@@ -316,9 +367,43 @@
     &age
 };
 
+static int lb_hb_init(apr_pool_t *p, apr_pool_t *plog,
+                          apr_pool_t *ptemp, server_rec *s)
+{
+    const char *userdata_key = "mod_lbmethod_heartbeat_init";
+    void *data;
+    apr_size_t size;
+    int num;
+    lb_hb_ctx_t *ctx =
+    (lb_hb_ctx_t *) ap_get_module_config(s->module_config,
+                                         &lbmethod_heartbeat_module);
+    apr_pool_userdata_get(&data, userdata_key, s->process->pool);
+    if (!data) {
+        /* first call do nothing */
+        apr_pool_userdata_set((const void *)1, userdata_key, apr_pool_cleanup_null, s->process->pool);
+        return OK;
+    }
+    storage = ap_lookup_provider(AP_SLOTMEM_PROVIDER_GROUP, "shared", "0");
+    if (!storage) {
+        ap_log_error(APLOG_MARK, APLOG_NOERRNO|APLOG_NOTICE, 0, s, "ap_lookup_provider %s failed", AP_SLOTMEM_PROVIDER_GROUP);
+        return OK;
+    }
+
+    /* Try to use a slotmem created by mod_heartmonitor */
+    storage->attach(&hm_serversmem, "mod_heartmonitor", &size, &num, p);
+    if (!hm_serversmem) {
+        ap_log_error(APLOG_MARK, APLOG_NOERRNO|APLOG_NOTICE, 0, s, "No slotmem from mod_heartmonitor");
+    } else
+        ap_log_error(APLOG_MARK, APLOG_NOERRNO|APLOG_NOTICE, 0, s, "Using slotmem from mod_heartmonitor");
+
+    return OK;
+}
+
 static void register_hooks(apr_pool_t *p)
 {
+    static const char * const aszPred[]={ "mod_heartmonitor.c", NULL };
     ap_register_provider(p, PROXY_LBMETHOD, "heartbeat", "0", &heartbeat);
+    ap_hook_post_config(lb_hb_init, aszPred, NULL, APR_HOOK_MIDDLE);
 }
 
 static void *lb_hb_create_config(apr_pool_t *p, server_rec *s)



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

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

Posted by Ruediger Pluem <rp...@apache.org>.

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