]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
MEDIUM: threads/lb: Make LB algorithms (lb_*.c) thread-safe
authorChristopher Faulet <cfaulet@haproxy.com>
Fri, 9 Jun 2017 12:17:53 +0000 (14:17 +0200)
committerWilly Tarreau <w@1wt.eu>
Tue, 31 Oct 2017 12:58:31 +0000 (13:58 +0100)
A lock for LB parameters has been added inside the proxy structure and atomic
operations have been used to update server variables releated to lb.

The only significant change is about lb_map. Because the servers status are
updated in the sync-point, we can call recalc_server_map function synchronously
in map_set_server_status_up/down function.

12 files changed:
include/common/hathreads.h
include/proto/lb_map.h
include/types/backend.h
include/types/lb_map.h
src/backend.c
src/cfgparse.c
src/haproxy.c
src/lb_chash.c
src/lb_fas.c
src/lb_fwlc.c
src/lb_fwrr.c
src/lb_map.c

index 20d0c04794bf7f19a8afbab4a58a64f5b52479cb..9946d51163ab6eb954a9c83feec5b5fc1fdf0255 100644 (file)
@@ -150,6 +150,7 @@ enum lock_label {
        PROXY_LOCK,
        SERVER_LOCK,
        UPDATED_SERVERS_LOCK,
+       LBPRM_LOCK,
        SIGNALS_LOCK,
        LOCK_LABELS
 };
@@ -236,7 +237,7 @@ static inline void show_lock_stats()
        const char *labels[LOCK_LABELS] = {"THREAD_SYNC", "FDTAB", "FDCACHE", "FD", "POLL",
                                           "TASK_RQ", "TASK_WQ", "POOL",
                                           "LISTENER", "LISTENER_QUEUE", "PROXY", "SERVER",
-                                          "UPDATED_SERVERS", "SIGNALS" };
+                                          "UPDATED_SERVERS", "LBPRM", "SIGNALS" };
        int lbl;
 
        for (lbl = 0; lbl < LOCK_LABELS; lbl++) {
index 061273d3f288ba1ab94cf0173d5c4d4362719179..cf7349ef4d5ef0b7f8cc2e1ed2cffd584a844f88 100644 (file)
@@ -26,8 +26,6 @@
 #include <types/proxy.h>
 #include <types/server.h>
 
-void map_set_server_status_down(struct server *srv);
-void map_set_server_status_up(struct server *srv);
 void recalc_server_map(struct proxy *px);
 void init_server_map(struct proxy *p);
 struct server *map_get_server_rr(struct proxy *px, struct server *srvtoavoid);
index 446ac2af13dc949d356a9ee91f7e0a878e5c99c5..68d312538737b3c66dcd4ba4dd796c15312db3cf 100644 (file)
@@ -23,6 +23,8 @@
 #define _TYPES_BACKEND_H
 
 #include <common/config.h>
+#include <common/hathreads.h>
+
 #include <types/lb_chash.h>
 #include <types/lb_fas.h>
 #include <types/lb_fwlc.h>
@@ -145,6 +147,9 @@ struct lbprm {
        struct lb_fwlc fwlc;
        struct lb_chash chash;
        struct lb_fas fas;
+#ifdef USE_THREAD
+       HA_SPINLOCK_T lock;
+#endif
        /* Call backs for some actions. Any of them may be NULL (thus should be ignored). */
        void (*update_server_eweight)(struct server *);  /* to be called after eweight change */
        void (*set_server_status_up)(struct server *);   /* to be called after status changes to UP */
index 4c12089d4fad425b8fe3e1a0d48f02d6703cb774..38e26af64fe0139741c3372d94467bb37bc7f9d4 100644 (file)
 #include <common/config.h>
 #include <types/server.h>
 
-/* values for map.state */
-#define LB_MAP_RECALC  (1 << 0)
-
 struct lb_map {
        struct server **srv;    /* the server map used to apply weights */
        int rr_idx;             /* next server to be elected in round robin mode */
-       int state;              /* LB_MAP_RECALC */
 };
 
 #endif /* _TYPES_LB_MAP_H */
index d17635ff5ff30157596e9d8693a9df9b3b01dc79..23b85ce6b956c33460f55132de88a066a0a6205a 100644 (file)
@@ -99,6 +99,10 @@ static unsigned int gen_hash(const struct proxy* px, const char* key, unsigned l
  * this.
  * This functions is designed to be called before server's weight and state
  * commit so it uses 'next' weight and states values.
+ *
+ * threads: this is the caller responsibility to lock data. For now, this
+ * function is called from lb modules, so it should be ok. But if you need to
+ * call it from another place, be careful (and update this comment).
  */
 void recount_servers(struct proxy *px)
 {
@@ -129,6 +133,10 @@ void recount_servers(struct proxy *px)
 /* This function simply updates the backend's tot_weight and tot_used values
  * after servers weights have been updated. It is designed to be used after
  * recount_servers() or equivalent.
+ *
+ * threads: this is the caller responsibility to lock data. For now, this
+ * function is called from lb modules, so it should be ok. But if you need to
+ * call it from another place, be careful (and update this comment).
  */
 void update_backend_weight(struct proxy *px)
 {
@@ -233,7 +241,7 @@ static struct server *get_server_uh(struct proxy *px, char *uri, int uri_len)
                return map_get_server_hash(px, hash);
 }
 
-/* 
+/*
  * This function tries to find a running server for the proxy <px> following
  * the URL parameter hash method. It looks for a specific parameter in the
  * URL and hashes it to compute the server ID. This is useful to optimize
@@ -503,7 +511,7 @@ static struct server *get_server_rch(struct stream *s)
        else
                return map_get_server_hash(px, hash);
 }
+
 /*
  * This function applies the load-balancing algorithm to the stream, as
  * defined by the backend it is assigned to. The stream is then marked as
@@ -579,6 +587,7 @@ int assign_server(struct stream *s)
                s->target = &srv->obj_type;
        }
        else if (s->be->lbprm.algo & BE_LB_KIND) {
+
                /* we must check if we have at least one server available */
                if (!s->be->lbprm.tot_weight) {
                        err = SRV_STATUS_NOSRV;
index e765fdb1acabe999d5272ac629aa74db1bdaa347..dd49009530ff8ca4c12d5f5d6128dd42c1dd51f6 100644 (file)
@@ -8434,6 +8434,7 @@ out_uri_auth_compat:
                        }
                        break;
                }
+               SPIN_INIT(&curproxy->lbprm.lock);
 
                if (curproxy->options & PR_O_LOGASAP)
                        curproxy->to_log &= ~LW_BYTES;
index 5710cc26e6b4a49df0c963f8e68d0ad7beb863d7..30ac15737e53795537ac260b154bf8ed5bbdf664 100644 (file)
@@ -2129,6 +2129,7 @@ void deinit(void)
 
                p0 = p;
                p = p->next;
+               SPIN_DESTROY(&p0->lbprm.lock);
                SPIN_DESTROY(&p0->lock);
                free(p0);
        }/* end while(p) */
index f368b684e8bedfa5ea9baddaa4ca7b6dc8426245..32d7d1d1166f766da968e7c02fe03f99b3c42dc2 100644 (file)
@@ -118,7 +118,7 @@ static void chash_set_server_status_down(struct server *srv)
        struct proxy *p = srv->proxy;
 
        if (!srv_lb_status_changed(srv))
-               return;
+               return;
 
        if (srv_willbe_usable(srv))
                goto out_update_state;
@@ -169,7 +169,7 @@ static void chash_set_server_status_up(struct server *srv)
        struct proxy *p = srv->proxy;
 
        if (!srv_lb_status_changed(srv))
-               return;
+               return;
 
        if (!srv_willbe_usable(srv))
                goto out_update_state;
@@ -364,14 +364,19 @@ struct server *chash_get_next_server(struct proxy *p, struct server *srvtoavoid)
        srv = avoided = NULL;
        avoided_node = NULL;
 
+       SPIN_LOCK(LBPRM_LOCK, &p->lbprm.lock);
        if (p->srv_act)
                root = &p->lbprm.chash.act;
-       else if (p->lbprm.fbck)
-               return p->lbprm.fbck;
+       else if (p->lbprm.fbck) {
+               srv = p->lbprm.fbck;
+               goto out;
+       }
        else if (p->srv_bck)
                root = &p->lbprm.chash.bck;
-       else
-               return NULL;
+       else {
+               srv = NULL;
+               goto out;
+       }
 
        stop = node = p->lbprm.chash.last;
        do {
@@ -415,6 +420,8 @@ struct server *chash_get_next_server(struct proxy *p, struct server *srvtoavoid)
                p->lbprm.chash.last = avoided_node;
        }
 
+ out:
+       SPIN_UNLOCK(LBPRM_LOCK, &p->lbprm.lock);
        return srv;
 }
 
index f8e739b1228be6784a1e23a0642d098300e47832..db292db8d6a2315575ecb396651f52c6c9dfa0af 100644 (file)
@@ -63,8 +63,11 @@ static void fas_srv_reposition(struct server *s)
 {
        if (!s->lb_tree)
                return;
+
+       SPIN_LOCK(LBPRM_LOCK, &s->proxy->lbprm.lock);
        fas_dequeue_srv(s);
        fas_queue_srv(s);
+       SPIN_UNLOCK(LBPRM_LOCK, &s->proxy->lbprm.lock);
 }
 
 /* This function updates the server trees according to server <srv>'s new
@@ -111,7 +114,7 @@ static void fas_set_server_status_down(struct server *srv)
        fas_dequeue_srv(srv);
        fas_remove_from_tree(srv);
 
-out_update_backend:
+ out_update_backend:
        /* check/update tot_used, tot_weight */
        update_backend_weight(p);
  out_update_state:
@@ -274,14 +277,19 @@ struct server *fas_get_next_server(struct proxy *p, struct server *srvtoavoid)
 
        srv = avoided = NULL;
 
+       SPIN_LOCK(LBPRM_LOCK, &p->lbprm.lock);
        if (p->srv_act)
                node = eb32_first(&p->lbprm.fas.act);
-       else if (p->lbprm.fbck)
-               return p->lbprm.fbck;
+       else if (p->lbprm.fbck) {
+               srv = p->lbprm.fbck;
+               goto out;
+       }
        else if (p->srv_bck)
                node = eb32_first(&p->lbprm.fas.bck);
-       else
-               return NULL;
+       else {
+               srv = NULL;
+               goto out;
+       }
 
        while (node) {
                /* OK, we have a server. However, it may be saturated, in which
@@ -304,7 +312,8 @@ struct server *fas_get_next_server(struct proxy *p, struct server *srvtoavoid)
 
        if (!srv)
                srv = avoided;
-
+  out:
+       SPIN_UNLOCK(LBPRM_LOCK, &p->lbprm.lock);
        return srv;
 }
 
index 589031236732515c8728f344687b087684db364f..8bd3ac22c5929c6753da9094052ab579009574b8 100644 (file)
@@ -55,8 +55,11 @@ static void fwlc_srv_reposition(struct server *s)
 {
        if (!s->lb_tree)
                return;
+
+       SPIN_LOCK(LBPRM_LOCK, &s->proxy->lbprm.lock);
        fwlc_dequeue_srv(s);
        fwlc_queue_srv(s);
+       SPIN_UNLOCK(LBPRM_LOCK, &s->proxy->lbprm.lock);
 }
 
 /* This function updates the server trees according to server <srv>'s new
@@ -266,14 +269,19 @@ struct server *fwlc_get_next_server(struct proxy *p, struct server *srvtoavoid)
 
        srv = avoided = NULL;
 
+       SPIN_LOCK(LBPRM_LOCK, &p->lbprm.lock);
        if (p->srv_act)
                node = eb32_first(&p->lbprm.fwlc.act);
-       else if (p->lbprm.fbck)
-               return p->lbprm.fbck;
+       else if (p->lbprm.fbck) {
+               srv = p->lbprm.fbck;
+               goto out;
+       }
        else if (p->srv_bck)
                node = eb32_first(&p->lbprm.fwlc.bck);
-       else
-               return NULL;
+       else {
+               srv = NULL;
+               goto out;
+       }
 
        while (node) {
                /* OK, we have a server. However, it may be saturated, in which
@@ -296,7 +304,8 @@ struct server *fwlc_get_next_server(struct proxy *p, struct server *srvtoavoid)
 
        if (!srv)
                srv = avoided;
-
+ out:
+       SPIN_UNLOCK(LBPRM_LOCK, &p->lbprm.lock);
        return srv;
 }
 
index e273a279a8676001760aafd27db502fd81582445..fe2777df90033b127ed0ce286f281e97df2dbdd0 100644 (file)
@@ -315,7 +315,7 @@ static void fwrr_queue_srv(struct server *s)
        struct fwrr_group *grp;
 
        grp = (s->flags & SRV_F_BACKUP) ? &p->lbprm.fwrr.bck : &p->lbprm.fwrr.act;
-       
+
        /* Delay everything which does not fit into the window and everything
         * which does not fit into the theorical new window.
         */
@@ -327,7 +327,7 @@ static void fwrr_queue_srv(struct server *s)
                 s->npos >= grp->curr_weight + grp->next_weight) {
                /* put into next tree, and readjust npos in case we could
                 * finally take this back to current. */
-               s->npos -= grp->curr_weight;
+               HA_ATOMIC_SUB(&s->npos, grp->curr_weight);
                fwrr_queue_by_weight(grp->next, s);
        }
        else {
@@ -359,7 +359,7 @@ static inline void fwrr_get_srv_next(struct server *s)
                &s->proxy->lbprm.fwrr.bck :
                &s->proxy->lbprm.fwrr.act;
 
-       s->npos += grp->curr_weight;
+       HA_ATOMIC_ADD(&s->npos, grp->curr_weight);
 }
 
 /* prepares a server when it was marked down */
@@ -414,7 +414,7 @@ static struct server *fwrr_get_server_from_group(struct fwrr_group *grp)
 
        node = eb32_first(&grp->curr);
        s = eb32_entry(node, struct server, lb_node);
-       
+
        if (!node || s->npos > grp->curr_pos) {
                /* either we have no server left, or we have a hole */
                struct eb32_node *node2;
@@ -442,20 +442,20 @@ static inline void fwrr_update_position(struct fwrr_group *grp, struct server *s
                /* first time ever for this server */
                s->lpos = grp->curr_pos;
                s->npos = grp->curr_pos + grp->next_weight / s->cur_eweight;
-               s->rweight += grp->next_weight % s->cur_eweight;
+               HA_ATOMIC_ADD(&s->rweight, (grp->next_weight % s->cur_eweight));
 
                if (s->rweight >= s->cur_eweight) {
-                       s->rweight -= s->cur_eweight;
-                       s->npos++;
+                       HA_ATOMIC_SUB(&s->rweight, s->cur_eweight);
+                       HA_ATOMIC_ADD(&s->npos, 1);
                }
        } else {
                s->lpos = s->npos;
-               s->npos += grp->next_weight / s->cur_eweight;
-               s->rweight += grp->next_weight % s->cur_eweight;
+               HA_ATOMIC_ADD(&s->npos, (grp->next_weight / s->cur_eweight));
+               HA_ATOMIC_ADD(&s->rweight, (grp->next_weight % s->cur_eweight));
 
                if (s->rweight >= s->cur_eweight) {
-                       s->rweight -= s->cur_eweight;
-                       s->npos++;
+                       HA_ATOMIC_SUB(&s->rweight, s->cur_eweight);
+                       HA_ATOMIC_ADD(&s->npos, 1);
                }
        }
 }
@@ -470,14 +470,19 @@ struct server *fwrr_get_next_server(struct proxy *p, struct server *srvtoavoid)
        struct fwrr_group *grp;
        int switched;
 
+       SPIN_LOCK(LBPRM_LOCK, &p->lbprm.lock);
        if (p->srv_act)
                grp = &p->lbprm.fwrr.act;
-       else if (p->lbprm.fbck)
-               return p->lbprm.fbck;
+       else if (p->lbprm.fbck) {
+               srv = p->lbprm.fbck;
+               goto out;
+       }
        else if (p->srv_bck)
                grp = &p->lbprm.fwrr.bck;
-       else
-               return NULL;
+       else {
+               srv = NULL;
+               goto out;
+       }
 
        switched = 0;
        avoided = NULL;
@@ -558,6 +563,8 @@ struct server *fwrr_get_next_server(struct proxy *p, struct server *srvtoavoid)
                        } while (full);
                }
        }
+ out:
+       SPIN_UNLOCK(LBPRM_LOCK, &p->lbprm.lock);
        return srv;
 }
 
index fef16ac2ececbc430ecc99b48139d33717cd749f..028e85ba0b163d892bb32913152c19fdb2583cf3 100644 (file)
@@ -19,6 +19,7 @@
 #include <types/server.h>
 
 #include <proto/backend.h>
+#include <proto/lb_map.h>
 #include <proto/proto_http.h>
 #include <proto/proto_tcp.h>
 #include <proto/queue.h>
@@ -37,7 +38,7 @@ static void map_set_server_status_down(struct server *srv)
        /* FIXME: could be optimized since we know what changed */
        recount_servers(p);
        update_backend_weight(p);
-       p->lbprm.map.state |= LB_MAP_RECALC;
+       recalc_server_map(p);
  out_update_state:
        srv_lb_commit_status(srv);
 }
@@ -56,7 +57,7 @@ static void map_set_server_status_up(struct server *srv)
        /* FIXME: could be optimized since we know what changed */
        recount_servers(p);
        update_backend_weight(p);
-       p->lbprm.map.state |= LB_MAP_RECALC;
+       recalc_server_map(p);
  out_update_state:
        srv_lb_commit_status(srv);
 }
@@ -73,7 +74,6 @@ void recalc_server_map(struct proxy *px)
 
        switch (px->lbprm.tot_used) {
        case 0: /* no server */
-               px->lbprm.map.state &= ~LB_MAP_RECALC;
                return;
        default:
                tot = px->lbprm.tot_weight;
@@ -113,7 +113,7 @@ void recalc_server_map(struct proxy *px)
                                        break;
                                }
 
-                               cur->wscore += cur->next_eweight;
+                               HA_ATOMIC_ADD(&cur->wscore, cur->next_eweight);
                                v = (cur->wscore + tot) / tot; /* result between 0 and 3 */
                                if (best == NULL || v > max) {
                                        max = v;
@@ -122,9 +122,8 @@ void recalc_server_map(struct proxy *px)
                        }
                }
                px->lbprm.map.srv[o] = best;
-               best->wscore -= tot;
+               HA_ATOMIC_ADD(&best->wscore, tot);
        }
-       px->lbprm.map.state &= ~LB_MAP_RECALC;
 }
 
 /* This function is responsible of building the server MAP for map-based LB
@@ -193,7 +192,6 @@ void init_server_map(struct proxy *p)
 
        p->lbprm.map.srv = calloc(act, sizeof(struct server *));
        /* recounts servers and their weights */
-       p->lbprm.map.state = LB_MAP_RECALC;
        recount_servers(p);
        update_backend_weight(p);
        recalc_server_map(p);
@@ -210,11 +208,11 @@ struct server *map_get_server_rr(struct proxy *px, struct server *srvtoavoid)
        int newidx, avoididx;
        struct server *srv, *avoided;
 
-       if (px->lbprm.tot_weight == 0)
-               return NULL;
-
-       if (px->lbprm.map.state & LB_MAP_RECALC)
-               recalc_server_map(px);
+       SPIN_LOCK(LBPRM_LOCK, &px->lbprm.lock);
+       if (px->lbprm.tot_weight == 0) {
+               avoided = NULL;
+               goto out;
+       }
 
        if (px->lbprm.map.rr_idx < 0 || px->lbprm.map.rr_idx >= px->lbprm.tot_weight)
                px->lbprm.map.rr_idx = 0;
@@ -241,6 +239,8 @@ struct server *map_get_server_rr(struct proxy *px, struct server *srvtoavoid)
        if (avoided)
                px->lbprm.map.rr_idx = avoididx;
 
+  out:
+       SPIN_UNLOCK(LBPRM_LOCK, &px->lbprm.lock);
        /* return NULL or srvtoavoid if found */
        return avoided;
 }
@@ -255,10 +255,6 @@ struct server *map_get_server_hash(struct proxy *px, unsigned int hash)
 {
        if (px->lbprm.tot_weight == 0)
                return NULL;
-
-       if (px->lbprm.map.state & LB_MAP_RECALC)
-               recalc_server_map(px);
-
        return px->lbprm.map.srv[hash % px->lbprm.tot_weight];
 }