BUG/MEDIUM: lb-leastconn: Reposition a server using the right eweight

Depending on the context, the current eweight or the next one must be used
to reposition a server in the tree. When the server state is updated, for
instance its weight, the next eweight must be used because it is not yet
committed. However, when the server is used, on normal conditions, the
current eweight must be used.

In fact, it is only a bug on the 1.8. On newer versions, the changes on a
server are performed synchronously. But it is safer to rely on the right
eweight value to avoid any futur bugs.

On the 1.8, it is important to do so, because the server state is updated
and committed inside the rendez-vous point. Thus, the next server state may
be unsync with the current state for a short time, waiting all threads join
the rendez-vous point. It is especially a problem if the next eweight is set
to 0. Because otherwise, it must not be used to reposition the server in the
tree, leading to a divide by 0.

This patch must be backported as far as 1.8.

(cherry picked from commit cb33d3ac7f8dbc3f7323606a521d29dc100adbda)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 0881842bf7a8de3e3cf93bece19f160945ba984a)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 8498c5fe48b36371f2bf8e12217d57b76adde561)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

diff --git a/src/lb_fwlc.c b/src/lb_fwlc.c
index bd7793a..c0f74bf 100644 (file)
--- a/src/lb_fwlc.c
+++ b/src/lb_fwlc.c
@@ -42,7 +42,7 @@ static inline void fwlc_dequeue_srv(struct server *s)
        eb32_delete(&s->lb_node);
 }
 
-/* Queue a server in its associated tree, assuming the weight is >0.
+/* Queue a server in its associated tree, assuming the <eweight> is >0.
  * Servers are sorted by (#conns+1)/weight. To ensure maximum accuracy,
  * we use (#conns+1)*SRV_EWGHT_MAX/eweight as the sorting key. The reason
  * for using #conns+1 is to sort by weights in case the server is picked
@@ -52,11 +52,17 @@ static inline void fwlc_dequeue_srv(struct server *s)
  * connection are always picked first so that under low loads, it's not
  * always the single server with the highest weight that gets picked.
  *
+ * NOTE: Depending on the calling context, we use s->next_eweight or
+ *       s->cur_eweight. The next value is used when the server state is updated
+ *       (because the weight changed for instance). During this step, the server
+ *       state is not yet committed. The current value is used to reposition the
+ *       server in the tree. This happens when the server is used.
+ *
  * The server's lock and the lbprm's lock must be held.
  */
-static inline void fwlc_queue_srv(struct server *s)
+static inline void fwlc_queue_srv(struct server *s, unsigned int eweight)
 {
-       s->lb_node.key = s->served ? (s->served + 1) * SRV_EWGHT_MAX / s->next_eweight : 0;
+       s->lb_node.key = s->served ? (s->served + 1) * SRV_EWGHT_MAX / eweight : 0;
        eb32_insert(s->lb_tree, &s->lb_node);
 }
 
@@ -71,7 +77,7 @@ static void fwlc_srv_reposition(struct server *s)
        HA_SPIN_LOCK(LBPRM_LOCK, &s->proxy->lbprm.lock);
        if (s->lb_tree) {
                fwlc_dequeue_srv(s);
-               fwlc_queue_srv(s);
+               fwlc_queue_srv(s, s->cur_eweight);
        }
        HA_SPIN_UNLOCK(LBPRM_LOCK, &s->proxy->lbprm.lock);
 }
@@ -186,7 +192,7 @@ static void fwlc_set_server_status_up(struct server *srv)
        }
 
        /* note that eweight cannot be 0 here */
-       fwlc_queue_srv(srv);
+       fwlc_queue_srv(srv, srv->next_eweight);
 
  out_update_backend:
        /* check/update tot_used, tot_weight */
@@ -247,7 +253,7 @@ static void fwlc_update_server_weight(struct server *srv)
                srv->lb_tree = &p->lbprm.fwlc.act;
        }
 
-       fwlc_queue_srv(srv);
+       fwlc_queue_srv(srv, srv->next_eweight);
 
        update_backend_weight(p);
        HA_SPIN_UNLOCK(LBPRM_LOCK, &p->lbprm.lock);
@@ -287,7 +293,7 @@ void fwlc_init_server_tree(struct proxy *p)
                if (!srv_currently_usable(srv))
                        continue;
                srv->lb_tree = (srv->flags & SRV_F_BACKUP) ? &p->lbprm.fwlc.bck : &p->lbprm.fwlc.act;
-               fwlc_queue_srv(srv);
+               fwlc_queue_srv(srv, srv->next_eweight);
        }
 }