Mailing-List: contact dev-help@httpd.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@httpd.apache.org
Message-ID: <44CE5B9D.2030708@apache.org>
Date: Mon, 31 Jul 2006 21:35:57 +0200
From: Ruediger Pluem <rpluem@apache.org>
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.13) Gecko/20060417
MIME-Version: 1.0
To: dev@httpd.apache.org
Subject: Re: svn commit: r427172 - in /httpd/httpd/trunk: CHANGES
 modules/proxy/mod_proxy.c
 modules/proxy/mod_proxy.h modules/proxy/mod_proxy_balancer.c
References: <20060731170141.DF4EF1A981A@eris.apache.org>
In-Reply-To: <20060731170141.DF4EF1A981A@eris.apache.org>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit


On 07/31/2006 07:01 PM, jim@apache.org wrote:
> Author: jim
> Date: Mon Jul 31 10:01:40 2006
> New Revision: 427172
> 
> URL: http://svn.apache.org/viewvc?rev=427172&view=rev
> Log:
> Add in a very simple balancer "set" concept, which allows
> for members to be assigned to a particular cluster set
> such that members in lower-numbered sets are checked/used
> before those in higher ones.
> 
> Also bundled in this are some HTML cleanups for the balancer
> manager UI. Sorry for the mixins :)
> 
> Compiles/builds clean: passes test framework as well
> as more normal usage tests ;)
> 

> +    do {
> +        while (!mycandidate && !checked_standby) {
> +            worker = (proxy_worker *)balancer->workers->elts;
> +            for (i = 0; i < balancer->workers->nelts; i++, worker++) {
> +                if (!checking_standby) {    /* first time through */
> +                    if (worker->s->lbset > max_lbset)
> +                        max_lbset = worker->s->lbset;
> +                }
> +                if (worker->s->lbset > cur_lbset)
> +                    continue;
> +                if ( (checking_standby ? !PROXY_WORKER_IS_STANDBY(worker) : PROXY_WORKER_IS_STANDBY(worker)) )
> +                    continue;
> +                /* If the worker is in error state run
> +                 * retry on that worker. It will be marked as
> +                 * operational if the retry timeout is elapsed.
> +                 * The worker might still be unusable, but we try
> +                 * anyway.
> +                 */
> +                if (!PROXY_WORKER_IS_USABLE(worker))
> +                    ap_proxy_retry_worker("BALANCER", worker, r->server);
> +                /* Take into calculation only the workers that are
> +                 * not in error state or not disabled.
> +                 */
> +                if (PROXY_WORKER_IS_USABLE(worker)) {
> +                    mytraffic = (worker->s->transferred/worker->s->lbfactor) +
> +                                (worker->s->read/worker->s->lbfactor);
> +                    if (!mycandidate || mytraffic < curmin) {
> +                        mycandidate = worker;
> +                        curmin = mytraffic;
> +                    }
>                  }
>              }
> +            checked_standby = checking_standby++;
>          }
> -        checked_standby = checking_standby++;
> -    }
> +        cur_lbset++;
> +    } while (cur_lbset < max_lbset && !mycandidate);

Shouldn't that be "while (cur_lbset <= max_lbset && !mycandidate);"
(same question also for the other algorithm)?
I guess otherwise we would not check for the workers with the lbset max_lbset.

Regards

Rüdiger