Merge pull request #376 from jajik/lbstatus-calculation

Add checks for lbfactor < 0, minor refactor

Author: jajik

native/balancers/mod_lbmethod_cluster.c

@@ -37,8 +37,8 @@ static proxy_worker *internal_find_best_byrequests(request_rec *r, const proxy_b
     int i;
 
     for (i = 0; i < balancer->workers->nelts; i++) {
-        const nodeinfo_t *node;
-        int id;
+        const nodeinfo_t *node, *node1;
+        int id, id1;
         proxy_worker *worker = *((proxy_worker **)(ptr + i * sizew));
 
         if (!PROXY_WORKER_IS_USABLE(worker)) {
@@ -54,18 +54,22 @@ static proxy_worker *internal_find_best_byrequests(request_rec *r, const proxy_b
         }
         if (!mycandidate) {
             mycandidate = worker;
-        } else {
-            const nodeinfo_t *node1;
-            int id1;
-            node1 = table_get_node_route(node_table, mycandidate->s->route, &id1);
-            if (node1) {
-                int lbstatus, lbstatus1;
-                lbstatus1 = ((mycandidate->s->elected - node1->mess.oldelected) * 1000) / mycandidate->s->lbfactor;
-                lbstatus = ((worker->s->elected - node->mess.oldelected) * 1000) / worker->s->lbfactor;
-                if (lbstatus1 > lbstatus) {
-                    mycandidate = worker;
-                }
+            continue;
+        }
+
+        node1 = table_get_node_route(node_table, mycandidate->s->route, &id1);
+        if (node1 && mycandidate->s->lbfactor > 0 && worker->s->lbfactor > 0) {
+            int lbstatus, lbstatus1;
+            lbstatus1 = ((mycandidate->s->elected - node1->mess.oldelected) * 1000) / mycandidate->s->lbfactor;
+            lbstatus = ((worker->s->elected - node->mess.oldelected) * 1000) / worker->s->lbfactor;
+            if (lbstatus1 > lbstatus) {
+                mycandidate = worker;
             }
+        } else {
+            ap_log_error(
+                APLOG_MARK, APLOG_DEBUG, 0, r->server,
+                "internal_find_best_byrequests: skipping node %d mycandidate->s->lbfactor %d worker->s->lbfactor %d",
+                id1, mycandidate->s->lbfactor, worker->s->lbfactor);
         }
     }
 

native/mod_proxy_cluster/mod_proxy_cluster.c

@@ -1672,26 +1672,35 @@ static proxy_worker *internal_process_worker(proxy_worker *worker, int checking_
         *mycandidate = worker;
         *mynodecontext = best1;
         return worker; /* Done */
-    } else if (!(*mycandidate)) {
+    }
+
+    if (!(*mycandidate)) {
         *mycandidate = worker;
         *mynodecontext = best1;
         *node1 = node;
-    } else {
-        int lbstatus, lbstatus1;
+        return worker;
+    }
 
-        /* Let's avoid repeat reads of mycandidate through our loop iterations */
-        if (!(*node1) && node_storage->read_node((*mycandidate)->s->index, node1) != APR_SUCCESS) {
-            *mycandidate = NULL;
-            return worker;
-        }
+    /* Let's avoid repeat reads of mycandidate through our loop iterations */
+    if (!(*node1) && node_storage->read_node((*mycandidate)->s->index, node1) != APR_SUCCESS) {
+        *mycandidate = NULL;
+        return worker;
+    }
 
-        lbstatus1 = (((*mycandidate)->s->elected - (*node1)->mess.oldelected) * 1000) / (*mycandidate)->s->lbfactor +
-                    (*mycandidate)->s->lbstatus;
-        lbstatus = ((worker->s->elected - node->mess.oldelected) * 1000) / worker->s->lbfactor + worker->s->lbstatus;
+    if ((*mycandidate)->s->lbfactor > 0 && worker->s->lbfactor) {
+        int lbstatus1 =
+            (((*mycandidate)->s->elected - (*node1)->mess.oldelected) * 1000) / (*mycandidate)->s->lbfactor +
+            (*mycandidate)->s->lbstatus;
+        int lbstatus =
+            ((worker->s->elected - node->mess.oldelected) * 1000) / worker->s->lbfactor + worker->s->lbstatus;
         if (lbstatus1 > lbstatus) {
             *mycandidate = worker;
             *mynodecontext = best1;
         }
+    } else {
+        ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
+                     "find_session_route: Could not recalculate lbstatus (candidate lbfactor: %d, worker lbfactor %d)",
+                     (*mycandidate)->s->lbfactor, worker->s->lbfactor);
     }
 
     return worker;