[BUG] consistent hash: balance on all servers, not only 2 !

It was once reported at least by Dirk Taggesell that the consistent
hash had a very poor distribution, making use of only two servers.
Jeff Persch analysed the code and found the root cause. Consistent
hash makes use of the server IDs, which are completed after the chash
array initialization. This implies that each server which does not
have an explicit "id" parameter will be merged at the same place in
the chash tree and that in the end, only the first or last servers
may be used.

The now obvious fix (thanks to Jeff) is to assign the missing IDs
earlier. However, it should be clearly understood that changing a
hash algorithm on live systems will rebalance the whole system.
Anyway, the only affected users will be the ones for which the
system is quite unbalanced already. The ones who fix their IDs are
not affected at all.

Kudos to Jeff for spotting that bug which got merged 3 days after
the consistent hashing !

diff --git a/src/cfgparse.c b/src/cfgparse.c
index 0e36050947d0b2f95950c6fe1f4fbc8955b5bbb8..cc82544e2bc58dbcb2649a92b18f0f00e2b7e79e 100644 (file)
--- a/src/cfgparse.c
+++ b/src/cfgparse.c
@@ -5045,6 +5045,22 @@ out_uri_auth_compat:
                        curproxy->srv = next;
                }
 
+               /* assign automatic UIDs to servers which don't have one yet */
+               next_id = 1;
+               newsrv = curproxy->srv;
+               while (newsrv != NULL) {
+                       if (!newsrv->puid) {
+                               /* server ID not set, use automatic numbering with first
+                                * spare entry starting with next_svid.
+                                */
+                               next_id = get_next_id(&curproxy->conf.used_server_id, next_id);
+                               newsrv->conf.id.key = newsrv->puid = next_id;
+                               eb32_insert(&curproxy->conf.used_server_id, &newsrv->conf.id);
+                       }
+                       next_id++;
+                       newsrv = newsrv->next;
+               }
+
                curproxy->lbprm.wmult = 1; /* default weight multiplier */
                curproxy->lbprm.wdiv  = 1; /* default weight divider */
 
@@ -5155,19 +5171,8 @@ out_uri_auth_compat:
                /*
                 * ensure that we're not cross-dressing a TCP server into HTTP.
                 */
-               next_id = 1;
                newsrv = curproxy->srv;
                while (newsrv != NULL) {
-                       if (!newsrv->puid) {
-                               /* server ID not set, use automatic numbering with first
-                                * spare entry starting with next_svid.
-                                */
-                               next_id = get_next_id(&curproxy->conf.used_server_id, next_id);
-                               newsrv->conf.id.key = newsrv->puid = next_id;
-                               eb32_insert(&curproxy->conf.used_server_id, &newsrv->conf.id);
-                       }
-                       next_id++;
-
                        if ((curproxy->mode != PR_MODE_HTTP) && (newsrv->rdr_len || newsrv->cklen)) {
                                Alert("config : %s '%s' : server cannot have cookie or redirect prefix in non-HTTP mode.\n",
                                      proxy_type_str(curproxy), curproxy->id);