OPTIM: server: ebtree lookups for findserver_unique_* functions

4e5e2664 ("MINOR: proxy: add findserver_unique_id() and findserver_unique_name()")
added findserver_unique_id() and findserver_unique_name() functions that
were inspired from the historical findserver() function, so unfortunately
they don't perform well when used on large backend farms because they scan
the whole server list linearly.

I was about to provide a patch to optimize such functions when I stumbled
on Baptiste's work:
  19a106d24 ("MINOR: server: server_find functions: id, name, best_match")

It turns out Baptiste already implemented helper functions to supersed
the unoptimized findserver() function (at least at runtime when servers
have been assigned their final IDs and inserted in the lookup trees): they
offer more matching options and rely on eb lookups so they are much more
suitable for fast queries. I don't know how I missed that, but they are a
perfect base for the server rid matching functions.

So in this patch, we essentially revert 4e5e2664 to provide the optimized
equivalent functions named server_find_by_id_unique() and
server_find_by_name_unique(), then we force existing findserver_unique_*()
callers to switch to the new functions.

This patch depends on:
 - "OPTIM: server: eb lookup for server_find_by_name()"

This could be backported up to 2.8.

diff --git a/include/haproxy/proxy.h b/include/haproxy/proxy.h
index efdfa21540f6278f362e612c91fa9f1e2fc6dc9d..01b0f17b5e57700b9e29112668e5aea810c1c86c 100644 (file)
--- a/include/haproxy/proxy.h
+++ b/include/haproxy/proxy.h
@@ -59,8 +59,6 @@ struct proxy *proxy_find_by_id(int id, int cap, int table);
 struct proxy *proxy_find_by_name(const char *name, int cap, int table);
 struct proxy *proxy_find_best_match(int cap, const char *name, int id, int *diff);
 struct server *findserver(const struct proxy *px, const char *name);
-struct server *findserver_unique_id(const struct proxy *px, int puid, uint32_t rid);
-struct server *findserver_unique_name(const struct proxy *px, const char *name, uint32_t rid);
 int proxy_cfg_ensure_no_http(struct proxy *curproxy);
 int proxy_cfg_ensure_no_log(struct proxy *curproxy);
 void init_new_proxy(struct proxy *p);

diff --git a/include/haproxy/server.h b/include/haproxy/server.h
index 2ba6e45a7f7530121dbddcd5126a82bfd6060354..26e9b60f904bfadcf288bf7b57c59b9621aaa505 100644 (file)
--- a/include/haproxy/server.h
+++ b/include/haproxy/server.h
@@ -53,7 +53,9 @@ const char *srv_update_addr_port(struct server *s, const char *addr, const char
 const char *srv_update_check_addr_port(struct server *s, const char *addr, const char *port);
 const char *srv_update_agent_addr_port(struct server *s, const char *addr, const char *port);
 struct server *server_find_by_id(struct proxy *bk, int id);
+struct server *server_find_by_id_unique(struct proxy *bk, int id, uint32_t rid);
 struct server *server_find_by_name(struct proxy *bk, const char *name);
+struct server *server_find_by_name_unique(struct proxy *bk, const char *name, uint32_t rid);
 struct server *server_find_best_match(struct proxy *bk, char *name, int id, int *diff);
 void apply_server_state(void);
 void srv_compute_all_admin_states(struct proxy *px);

diff --git a/src/hlua.c b/src/hlua.c
index 890215774806b8ab1ad21e95af19a2f04bdabb21..008f6726fc622744342bf69d59082f8def1c5658 100644 (file)
--- a/src/hlua.c
+++ b/src/hlua.c
@@ -9495,7 +9495,7 @@ __LJMP static void hlua_event_hdl_cb_push_args(struct hlua_event_sub *hlua_sub,
                 */
                px = proxy_find_by_id(e_server->safe.proxy_uuid, PR_CAP_BE, 0);
                BUG_ON(!px);
-               server = findserver_unique_id(px, e_server->safe.puid, e_server->safe.rid);
+               server = server_find_by_id_unique(px, e_server->safe.puid, e_server->safe.rid);
                if (server) {
                        lua_pushstring(hlua->T, "reference");
                        hlua_fcn_new_server(hlua->T, server);

diff --git a/src/proxy.c b/src/proxy.c
index 905c320957bf0e8fe53b02645cb3148e196195bd..349c2c632559d6f8315bfcc874e2f2c0d8981af6 100644 (file)
--- a/src/proxy.c
+++ b/src/proxy.c
@@ -1273,50 +1273,6 @@ struct server *findserver(const struct proxy *px, const char *name) {
        return target;
 }
 
-/*
- * This function finds a server with matching "<puid> x <rid>" within
- * selected proxy <px>.
- * Using the combination of proxy-uid + revision id ensures that the function
- * will either return the server we're expecting or NULL if it has been removed
- * from the proxy.
- */
-struct server *findserver_unique_id(const struct proxy *px, int puid, uint32_t rid) {
-
-       struct server *cursrv;
-
-       if (!px)
-               return NULL;
-
-       for (cursrv = px->srv; cursrv; cursrv = cursrv->next) {
-               if (cursrv->puid == puid && cursrv->rid == rid)
-                       return cursrv;
-       }
-
-       return NULL;
-}
-
-/*
- * This function finds a server with matching "<name> x <rid>" within
- * selected proxy <px>.
- * Using the combination of name + revision id ensures that the function will
- * either return the server we're expecting or NULL if it has been removed
- * from the proxy.
- */
-struct server *findserver_unique_name(const struct proxy *px, const char *name, uint32_t rid) {
-
-       struct server *cursrv;
-
-       if (!px)
-               return NULL;
-
-       for (cursrv = px->srv; cursrv; cursrv = cursrv->next) {
-               if (!strcmp(cursrv->id, name) && cursrv->rid == rid)
-                       return cursrv;
-       }
-
-       return NULL;
-}
-
 /* This function checks that the designated proxy has no http directives
  * enabled. It will output a warning if there are, and will fix some of them.
  * It returns the number of fatal errors encountered. This should be called

diff --git a/src/server.c b/src/server.c
index 25290fe2c2aaa69e21f7b0cdcaedfad98484aaf5..4dee6e3a135ebd2b83a91ea07ebb9fad87eb3497 100644 (file)
--- a/src/server.c
+++ b/src/server.c
@@ -253,7 +253,7 @@ static struct task *server_atomic_sync(struct task *task, void *context, unsigne
                        px = proxy_find_by_id(data->server.safe.proxy_uuid, PR_CAP_BE, 0);
                        if (!px)
                                continue;
-                       srv = findserver_unique_id(px, data->server.safe.puid, data->server.safe.rid);
+                       srv = server_find_by_id_unique(px, data->server.safe.puid, data->server.safe.rid);
                        if (!srv)
                                continue;
 
@@ -3581,6 +3581,25 @@ struct server *server_find_by_id(struct proxy *bk, int id)
        return curserver;
 }
 
+/*
+ * This function finds a server with matching "<puid> x <rid>" within
+ * selected backend <bk>.
+ * Using the combination of proxy-uid + revision id ensures that the function
+ * will either return the server we're expecting or NULL if it has been removed
+ * from the proxy (<id> is unique within the list, but it is not true over the
+ * process lifetime as new servers may reuse the id of a previously deleted
+ * server).
+ */
+struct server *server_find_by_id_unique(struct proxy *bk, int id, uint32_t rid)
+{
+       struct server *curserver;
+
+       curserver = server_find_by_id(bk, id);
+       if (!curserver || curserver->rid != rid)
+               return NULL;
+       return curserver;
+}
+
 /* Returns a pointer to the first server matching either name <name>, or id
  * if <name> starts with a '#'. NULL is returned if no match is found.
  * the lookup is performed in the backend <bk>
@@ -3611,6 +3630,33 @@ struct server *server_find_by_name(struct proxy *bk, const char *name)
        return curserver;
 }
 
+/*
+ * This function finds a server with matching "<name> x <rid>" within
+ * selected backend <bk>.
+ * Using the combination of name + revision id ensures that the function
+ * will either return the server we're expecting or NULL if it has been removed
+ * from the proxy. For this we assume that <name> is unique within the list,
+ * which is the case in most setups, but in rare cases the user may have
+ * enforced duplicate server names in the initial config (ie: if he intends to
+ * use numerical IDs for indentification instead). In this particular case, the
+ * function will not work as expected so server_find_by_id_unique() should be
+ * used to match a unique server instead.
+ *
+ * Just like server_find_by_id_unique(), if a server is deleted and a new server
+ * reuses the same name, the rid check will prevent the function from returning
+ * a different server from the one we were expecting to match against at a given
+ * time.
+ */
+struct server *server_find_by_name_unique(struct proxy *bk, const char *name, uint32_t rid)
+{
+       struct server *curserver;
+
+       curserver = server_find_by_name(bk, name);
+       if (!curserver || curserver->rid != rid)
+               return NULL;
+       return curserver;
+}
+
 struct server *server_find_best_match(struct proxy *bk, char *name, int id, int *diff)
 {
        struct server *byname;