mirrors/haproxy
1
0
Fork 0
mirror of https://git.haproxy.org/git/haproxy.git/ synced 2025-11-28 22:31:06 +01:00
Code Issues Projects Releases Wiki Activity

BUG/MAJOR: server: weight calculation fails for map-based algorithms

Browse Source 
A crash was reported by Igor at owind when changing a server's weight
on the CLI. Lukas Tribus could reproduce a related bug where setting
a server's weight would result in the new weight being multiplied by
the initial one. The two bugs are the same.

The incorrect weight calculation results in the total farm weight being
larger than what was initially allocated, causing the map index to be out
of bounds on some hashes. It's easy to reproduce using "balance url_param"
with a variable param, or with "balance static-rr".

It appears that the calculation is made at many places and is not always
right and not always wrong the same way. Thus, this patch introduces a
new function "server_recalc_eweight()" which is dedicated to this task
of computing ->eweight from many other elements including uweight and
current time (for slowstart), and all users now switch to use this
function.

The patch is a bit large but the code was not trivially fixable in a way
that could guarantee this situation would not occur anymore. The fix is
much more readable and has been verified to work with all algorithms,
with both consistent and map-based hashes, and even with static-rr.

Slowstart was tested as well, just like enable/disable server.

The same bug is very likely present in 1.4 as well, so the patch will
probably need to be backported eventhough it will not apply as-is.

Thanks to Lukas and Igor for the information they provided to reproduce it.
This commit is contained in:
Willy Tarreau 2013-11-21 11:22:01 +01:00
parent e7b73485d0
commit 004e045f31
9 changed files with 57 additions and 76 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
include/proto/server.h
View File

@ -58,6 +58,12 @@ struct srv_kw *srv_find_kw(const char *kw);
/* Dumps all registered "server" keywords to the <out> string pointer. */
void srv_dump_kws(char **out);
/* Recomputes the server's eweight based on its state, uweight, the current time,
* and the proxy's algorihtm. To be used after updating sv->uweight. The warmup
* state is automatically disabled if the time is elapsed.
*/
void server_recalc_eweight(struct server *sv);
/*
* Parses weight_str and configures sv accordingly.
* Returns NULL on success, error message string otherwise.

30
src/checks.c
View File

@ -481,18 +481,10 @@ void set_server_up(struct check *check) {
if (s->slowstart > 0) {
s->state |= SRV_WARMINGUP;
if (s->proxy->lbprm.algo & BE_LB_PROP_DYN) {
/* For dynamic algorithms, start at the first step of the weight,
* without multiplying by BE_WEIGHT_SCALE.
*/
s->eweight = s->uweight;
if (s->proxy->lbprm.update_server_eweight)
s->proxy->lbprm.update_server_eweight(s);
}
task_schedule(s->warmup, tick_add(now_ms, MS_TO_TICKS(MAX(1000, s->slowstart / 20))));
}
if (s->proxy->lbprm.set_server_status_up)
s->proxy->lbprm.set_server_status_up(s);
server_recalc_eweight(s);
/* If the server is set with "on-marked-up shutdown-backup-sessions",
* and it's not a backup server and its effective weight is > 0,
@ -1273,23 +1265,7 @@ static struct task *server_warmup(struct task *t)
if ((s->state & (SRV_RUNNING|SRV_WARMINGUP|SRV_MAINTAIN)) != (SRV_RUNNING|SRV_WARMINGUP))
return t;
if (now.tv_sec < s->last_change || now.tv_sec >= s->last_change + s->slowstart) {
/* go to full throttle if the slowstart interval is reached */
s->state &= ~SRV_WARMINGUP;
if (s->proxy->lbprm.algo & BE_LB_PROP_DYN)
s->eweight = s->uweight * BE_WEIGHT_SCALE;
if (s->proxy->lbprm.update_server_eweight)
s->proxy->lbprm.update_server_eweight(s);
}
else if (s->proxy->lbprm.algo & BE_LB_PROP_DYN) {
/* for dynamic algorithms, let's slowly update the weight */
s->eweight = (BE_WEIGHT_SCALE * (now.tv_sec - s->last_change) +
s->slowstart - 1) / s->slowstart;
s->eweight *= s->uweight;
if (s->proxy->lbprm.update_server_eweight)
s->proxy->lbprm.update_server_eweight(s);
}
/* Note that static algorithms are already running at full throttle */
server_recalc_eweight(s);
/* probably that we can refill this server with a bit more connections */
check_for_pending(s);

3
src/lb_chash.c
View File

@ -382,7 +382,8 @@ void chash_init_server_tree(struct proxy *p)
p->lbprm.wdiv = BE_WEIGHT_SCALE;
for (srv = p->srv; srv; srv = srv->next) {
srv->prev_eweight = srv->eweight = srv->uweight * BE_WEIGHT_SCALE;
srv->eweight = (srv->uweight * p->lbprm.wdiv + p->lbprm.wmult - 1) / p->lbprm.wmult;
srv->prev_eweight = srv->eweight;
srv->prev_state = srv->state;
}

3
src/lb_fas.c
View File

@ -252,7 +252,8 @@ void fas_init_server_tree(struct proxy *p)
p->lbprm.wdiv = BE_WEIGHT_SCALE;
for (srv = p->srv; srv; srv = srv->next) {
srv->prev_eweight = srv->eweight = srv->uweight * BE_WEIGHT_SCALE;
srv->eweight = (srv->uweight * p->lbprm.wdiv + p->lbprm.wmult - 1) / p->lbprm.wmult;
srv->prev_eweight = srv->eweight;
srv->prev_state = srv->state;
}

3
src/lb_fwlc.c
View File

@ -244,7 +244,8 @@ void fwlc_init_server_tree(struct proxy *p)
p->lbprm.wdiv = BE_WEIGHT_SCALE;
for (srv = p->srv; srv; srv = srv->next) {
srv->prev_eweight = srv->eweight = srv->uweight * BE_WEIGHT_SCALE;
srv->eweight = (srv->uweight * p->lbprm.wdiv + p->lbprm.wmult - 1) / p->lbprm.wmult;
srv->prev_eweight = srv->eweight;
srv->prev_state = srv->state;
}

3
src/lb_fwrr.c
View File

@ -272,7 +272,8 @@ void fwrr_init_server_groups(struct proxy *p)
p->lbprm.wdiv = BE_WEIGHT_SCALE;
for (srv = p->srv; srv; srv = srv->next) {
srv->prev_eweight = srv->eweight = srv->uweight * BE_WEIGHT_SCALE;
srv->eweight = (srv->uweight * p->lbprm.wdiv + p->lbprm.wmult - 1) / p->lbprm.wmult;
srv->prev_eweight = srv->eweight;
srv->prev_state = srv->state;
}

2
src/lb_map.c
View File

@ -180,7 +180,7 @@ void init_server_map(struct proxy *p)
act = bck = 0;
for (srv = p->srv; srv; srv = srv->next) {
srv->eweight = srv->uweight / pgcd;
srv->eweight = (srv->uweight * p->lbprm.wdiv + p->lbprm.wmult - 1) / p->lbprm.wmult;
srv->prev_eweight = srv->eweight;
srv->prev_state = srv->state;
if (srv->state & SRV_BACKUP)

22
src/proto_http.c
View File

@ -2932,28 +2932,8 @@ int http_process_req_stat_post(struct stream_interface *si, struct http_txn *txn
else
sv->uweight = 0;
if (px->lbprm.algo & BE_LB_PROP_DYN) {
/* we must take care of not pushing the server to full throttle during slow starts */
if ((sv->state & SRV_WARMINGUP) && (px->lbprm.algo & BE_LB_PROP_DYN))
sv->eweight = (BE_WEIGHT_SCALE * (now.tv_sec - sv->last_change) + sv->slowstart - 1) / sv->slowstart;
else
sv->eweight = BE_WEIGHT_SCALE;
sv->eweight *= sv->uweight;
} else {
sv->eweight = sv->uweight;
}
server_recalc_eweight(sv);
/* static LB algorithms are a bit harder to update */
if (px->lbprm.update_server_eweight)
px->lbprm.update_server_eweight(sv);
else if (sv->eweight) {
if (px->lbprm.set_server_status_up)
px->lbprm.set_server_status_up(sv);
}
else {
if (px->lbprm.set_server_status_down)
px->lbprm.set_server_status_down(sv);
}
altered_servers++;
total_servers++;
break;

61
src/server.c
View File

@ -158,6 +158,43 @@ static void __listener_init(void)
srv_register_keywords(&srv_kws);
}
/* Recomputes the server's eweight based on its state, uweight, the current time,
* and the proxy's algorihtm. To be used after updating sv->uweight. The warmup
* state is automatically disabled if the time is elapsed.
*/
void server_recalc_eweight(struct server *sv)
{
struct proxy *px = sv->proxy;
unsigned w;
if (now.tv_sec < sv->last_change || now.tv_sec >= sv->last_change + sv->slowstart) {
/* go to full throttle if the slowstart interval is reached */
sv->state &= ~SRV_WARMINGUP;
}
/* We must take care of not pushing the server to full throttle during slow starts.
* It must also start immediately, at least at the minimal step when leaving maintenance.
*/
if ((sv->state & SRV_WARMINGUP) && (px->lbprm.algo & BE_LB_PROP_DYN))
w = (px->lbprm.wdiv * (now.tv_sec - sv->last_change) + sv->slowstart) / sv->slowstart;
else
w = px->lbprm.wdiv;
sv->eweight = (sv->uweight * w + px->lbprm.wmult - 1) / px->lbprm.wmult;
/* now propagate the status change to any LB algorithms */
if (px->lbprm.update_server_eweight)
px->lbprm.update_server_eweight(sv);
else if (sv->eweight) {
if (px->lbprm.set_server_status_up)
px->lbprm.set_server_status_up(sv);
}
else {
if (px->lbprm.set_server_status_down)
px->lbprm.set_server_status_down(sv);
}
}
/*
* Parses weight_str and configures sv accordingly.
* Returns NULL on success, error message string otherwise.
@ -199,29 +236,7 @@ const char *server_parse_weight_change_request(struct server *sv,
return "Backend is using a static LB algorithm and only accepts weights '0%' and '100%'.\n";
sv->uweight = w;
if (px->lbprm.algo & BE_LB_PROP_DYN) {
/* we must take care of not pushing the server to full throttle during slow starts */
if ((sv->state & SRV_WARMINGUP))
sv->eweight = (BE_WEIGHT_SCALE * (now.tv_sec - sv->last_change) + sv->slowstart - 1) / sv->slowstart;
else
sv->eweight = BE_WEIGHT_SCALE;
sv->eweight *= sv->uweight;
} else {
sv->eweight = sv->uweight;
}
/* static LB algorithms are a bit harder to update */
if (px->lbprm.update_server_eweight)
px->lbprm.update_server_eweight(sv);
else if (sv->eweight) {
if (px->lbprm.set_server_status_up)
px->lbprm.set_server_status_up(sv);
}
else {
if (px->lbprm.set_server_status_down)
px->lbprm.set_server_status_down(sv);
}
server_recalc_eweight(sv);
return NULL;
}