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: r426604 - in /httpd/httpd/branches/httpd-proxy-scoreboard: modules/proxy/ support/
Date Sat, 29 Jul 2006 01:01:24 GMT
Ruediger Pluem wrote:

>On 28.07.2006 18:34, jfclere@apache.org wrote:
>  
>
>>Author: jfclere
>>Date: Fri Jul 28 09:33:58 2006
>>New Revision: 426604
>>
>>URL: http://svn.apache.org/viewvc?rev=426604&view=rev
>>Log:
>>First try to put togother an external health checker for mod_proxy.
>>
>>    
>>
>
>  
>
>>Modified: httpd/httpd/branches/httpd-proxy-scoreboard/modules/proxy/config.m4
>>URL: http://svn.apache.org/viewvc/httpd/httpd/branches/httpd-proxy-scoreboard/modules/proxy/config.m4?rev=426604&r1=426603&r2=426604&view=diff
>>==============================================================================
>>--- httpd/httpd/branches/httpd-proxy-scoreboard/modules/proxy/config.m4 (original)
>>+++ httpd/httpd/branches/httpd-proxy-scoreboard/modules/proxy/config.m4 Fri Jul 28
09:33:58 2006
>>@@ -17,6 +17,7 @@
>>    
>>
>
>
>  
>
>>+}
>>+/* copy the worker information in the shared area so the health-checker can extract
the part it need */
>>+static apr_status_t add_entry(proxy_worker *worker, char *balancer_name, int id)
>>    
>>
>
>What is the purpose of the id parameter. I do not see that it is used.
>  
>
The id of the slot where the worker has to be stored.

>  
>
>>+{
>>+    struct proxy_worker_conf *workerconf = NULL;
>>+    apr_status_t rv;
>>+
>>+    if (myscore == NULL)
>>+        return APR_ENOSHMAVAIL;
>>+    rv = checkstorage->ap_slotmem_mem(myscore, worker->id, (void *) &workerconf);
>>+    if (rv != APR_SUCCESS) {
>>+        return rv;
>>+    }
>>+
>>+    if (balancer_name)
>>+        strcpy(workerconf->balancer_name, balancer_name);
>>+    workerconf->id = worker->id;
>>+    workerconf->retry = worker->retry;
>>+    workerconf->lbfactor = worker->lbfactor;
>>    
>>
>
>Is this approach thread safe / process safe or is there no need to be?
>  
>
It should be used while parse the configuration or adding an entry... 
Well it isn't my idea is to have the thread/process synchro stuff before 
calling add_entry.

>  
>
>>+    if (worker->name)
>>+        strcpy(workerconf->name, worker->name);
>>+    if (worker->scheme)
>>+        strcpy(workerconf->scheme, worker->scheme);
>>+    if (worker->hostname)
>>+        strcpy(workerconf->hostname, worker->hostname);
>>+    if (worker->route)
>>+        strcpy(workerconf->route, worker->route);
>>+    if (worker->redirect)
>>+        strcpy(workerconf->redirect, worker->redirect);
>>    
>>
>
>Don't you need to use strncpy here to avoid buffer overflows?
>  
>
Of course.

>
>  
>
>>+/* read the entry stored in the shared area and build the corresponding worker structure
*/
>>+static apr_status_t get_entry(int id, proxy_worker **worker, char **balancer_name,
apr_pool_t *pool)
>>+{
>>+    struct proxy_worker_conf *workerconf = NULL;
>>+    apr_status_t rv;
>>+
>>+    if (myscore == NULL)
>>+        return APR_ENOSHMAVAIL;
>>+    rv = checkstorage->ap_slotmem_mem(myscore, id, (void *) &workerconf);
>>+    if (rv != APR_SUCCESS)
>>+        return rv;
>>+
>>+    /* allocate the data */
>>+    *worker = apr_pcalloc(pool, sizeof(proxy_worker));
>>+    if (workerconf->balancer_name)
>>+        *balancer_name = apr_pcalloc(pool, strlen(workerconf->balancer_name));
>>+    else
>>+        *balancer_name = NULL;
>>+
>>+    /* The httpstatus is handle by httpd don't touch it here */
>>+    (* worker)->id = workerconf->id;
>>+    // XXX: what to do (* worker)->s = workerconf;
>>+    (* worker)->retry = workerconf->retry;
>>+    (* worker)->lbfactor = workerconf->lbfactor;
>>+    if (workerconf->name)
>>+        strcpy((* worker)->name, workerconf->name);
>>+    if (workerconf->scheme)
>>+        strcpy((* worker)->scheme, workerconf->scheme);
>>+    if (workerconf->hostname)
>>+        strcpy((* worker)->hostname, workerconf->hostname);
>>+    if (workerconf->route)
>>+        strcpy((* worker)->route, workerconf->route);
>>+    if (workerconf->redirect)
>>+        strcpy((* worker)->redirect, workerconf->redirect);
>>    
>>
>
>Don't you need to allocate space for this first (like with the balancer name)?
>  
>
Yes, done.

>
>  
>
>>Added: httpd/httpd/branches/httpd-proxy-scoreboard/modules/proxy/mod_proxy_health_checker.c
>>URL: http://svn.apache.org/viewvc/httpd/httpd/branches/httpd-proxy-scoreboard/modules/proxy/mod_proxy_health_checker.c?rev=426604&view=auto
>>==============================================================================
>>--- httpd/httpd/branches/httpd-proxy-scoreboard/modules/proxy/mod_proxy_health_checker.c
(added)
>>+++ httpd/httpd/branches/httpd-proxy-scoreboard/modules/proxy/mod_proxy_health_checker.c
Fri Jul 28 09:33:58 2006
>>    
>>
>
>  
>
>>+
>>+static int healthck_pre_config(apr_pool_t *pconf, apr_pool_t *plog,
>>+                              apr_pool_t *ptemp)
>>+{
>>+    slotmem_storage_method *checkstorage;
>>+    health_worker_method *worker_storage = health_checker_get_storage();
>>+    ap_slotmem_t *myscore;
>>+    
>>+    checkstorage = ap_lookup_provider(SLOTMEM_STORAGE, "shared", "0");
>>+    if (checkstorage) {
>>+        health_checker_init_slotmem_storage(checkstorage);
>>+    }
>>+    if (checkstorage && worker_storage) {
>>+        checkstorage->ap_slotmem_create(&myscore, "proxy/checker", worker_storage->getentrysize(),
128, pconf);
>>+        health_checker_init_slotmem(myscore);
>>+    }
>>+    return OK;
>>+}
>>+
>>+/* XXX: Was to get ap_proxy_lb_workers()
>>+static int healthck_post_config(apr_pool_t *pconf, apr_pool_t *plog,
>>+                                apr_pool_t *ptemp, server_rec *s)
>>+{
>>+    slotmem_storage_method *checkstorage = health_checker_get_slotmem_storage();
>>+    health_worker_method *worker_storage = health_checker_get_storage();
>>+    ap_slotmem_t *myscore;
>>+
>>+    if (checkstorage && worker_storage) {
>>+        checkstorage->ap_slotmem_create(&myscore, "proxy/checker", worker_storage->getentrysize(),
ap_proxy_lb_workers(), pconf);
>>    
>>
>
>Sorry, but I am confused, but this only returns myscore created in preconfig
>without changing anything.
>  
>
Oops I have forgotten to remove it.

>
>  
>
>>Added: httpd/httpd/branches/httpd-proxy-scoreboard/modules/proxy/mod_proxy_health_checker.h
>>URL: http://svn.apache.org/viewvc/httpd/httpd/branches/httpd-proxy-scoreboard/modules/proxy/mod_proxy_health_checker.h?rev=426604&view=auto
>>==============================================================================
>>    
>>
>
>  
>
>>+/* allow health check method on workers in a non httpd process */
>>+struct health_worker_method {
>>+    /* read the size of the entry: to create the shared area */
>>+    int (* getentrysize)();
>>+    /* copy the worker information in the shared area so the health-checker can extract
the part it need */
>>+    apr_status_t (*add_entry)(proxy_worker *worker, char *balancer_name, int id);
>>    
>>
>
>What about dynamic updates via the manager? Do they get updated or is there no
>need to update the health check data with the manager data?
>  
>
They have to be updated. ;-)

>
>
>
>  
>
>>Added: httpd/httpd/branches/httpd-proxy-scoreboard/support/proxymonitor.c
>>URL: http://svn.apache.org/viewvc/httpd/httpd/branches/httpd-proxy-scoreboard/support/proxymonitor.c?rev=426604&view=auto
>>==============================================================================
>>--- httpd/httpd/branches/httpd-proxy-scoreboard/support/proxymonitor.c (added)
>>+++ httpd/httpd/branches/httpd-proxy-scoreboard/support/proxymonitor.c Fri Jul 28
09:33:58 2006
>>    
>>
>
>  
>
>>+/*
>>+ * httpd routine to be able to link with the modules.
>>+ */
>>+char * ap_server_root_relative(apr_pool_t *p, const char *name)
>>+{
>>+    char *fname;
>>+
>>+    /* XXX: apr_filepath_merge better ? */
>>+    if (basedir && name[0] != '/') {
>>+        fname = apr_pcalloc(p, strlen(basedir)+strlen(name)+1);
>>+        strcpy(fname, basedir);
>>+        strcat(fname, "/");
>>+        strcat(fname, name);
>>    
>>
>
>fname = apr_strcat(p, basedir, "/", name, NULL) ?
>
>
>  
>
>>+int main(int argc, const char * const argv[])
>>+{
>>    
>>
>
>  
>
>>+
>>+    while (repeat && ! interrupted) {
>>+
>>+        if (instance_socket == NULL) {
>>+            apr_pool_create(&instance_socket, pool);
>>+            init_healthck(instance_socket, &num);
>>+        }
>>+
>>+        apr_pool_create(&instance, instance_socket);
>>+        apr_sleep(delay);
>>+        now = apr_time_now();
>>+        process_sharedmem(instance_socket, num);
>>+        current = apr_time_now();
>>+        apr_rfc822_date(datestring, current);
>>+        apr_file_printf(outfile,"at %s in %d\n", datestring, current-now);
>>+
>>+        if (repeat>0)
>>+            repeat--;
>>+        apr_pool_destroy(instance);
>>+        /* If something goes really wrong we should clean all */
>>+        if (0) {
>>    
>>
>
>When does this condition become true?
>  
>
I have removed the code.

Thanks

Jean-Frederic

>Regards
>
>Rüdiger
>
>  
>


Mime
View raw message