From dd701651fe1a0e28c3634d6809485047dea3c2bb Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Tue, 25 May 2010 23:03:02 +0200 Subject: [PATCH] [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 ! --- src/cfgparse.c | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/src/cfgparse.c b/src/cfgparse.c index 0e3605094..cc82544e2 100644 --- a/src/cfgparse.c +++ b/src/cfgparse.c @@ -5045,6 +5045,22 @@ int check_config_validity() 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 @@ int check_config_validity() /* * 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);