From 1a53a3af131b0d85d2cb2b70fe9166f1ce830b3f Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Wed, 11 Dec 2013 15:27:05 +0100 Subject: [PATCH] MINOR: checks: improve handling of the servers tracking chain Server tracking uses the same "tracknext" list for servers tracking another one and for the servers being tracked. This caused an issue which was fixed by commit f39c71c ([CRITICAL] fix server state tracking: it was O(n!) instead of O(n)), consisting in ensuring that a server is being checked before walking down the list, so that we don't propagate the up/down information via servers being part of the track chain. But the root cause is the fact that all servers share the same list. The correct solution consists in having a list head for the tracked servers and a list of next tracking servers. This simplifies the propagation logic, especially for the case where status changes might be passed to individual servers via the CLI. --- include/types/server.h | 4 +++- src/cfgparse.c | 4 ++-- src/checks.c | 28 ++++++++++++---------------- 3 files changed, 17 insertions(+), 19 deletions(-) diff --git a/include/types/server.h b/include/types/server.h index 05d47835ec..a3775a7c88 100644 --- a/include/types/server.h +++ b/include/types/server.h @@ -158,7 +158,9 @@ struct server { struct conn_src conn_src; /* connection source settings */ - struct server *tracknext, *track; /* next server in a tracking list, tracked server */ + struct server *track; /* the server we're currently tracking, if any */ + struct server *trackers; /* the list of servers tracking us, if any */ + struct server *tracknext; /* next server tracking in 's trackers list */ char *trackit; /* temporary variable to make assignment deferrable */ int consecutive_errors; /* current number of consecutive errors */ int consecutive_errors_limit; /* number of consecutive errors that triggers an event */ diff --git a/src/cfgparse.c b/src/cfgparse.c index 579dbcc530..aa8bb90cf2 100644 --- a/src/cfgparse.c +++ b/src/cfgparse.c @@ -7044,8 +7044,8 @@ int check_config_validity() } newsrv->track = srv; - newsrv->tracknext = srv->tracknext; - srv->tracknext = newsrv; + newsrv->tracknext = srv->trackers; + srv->trackers = newsrv; free(newsrv->trackit); newsrv->trackit = NULL; diff --git a/src/checks.c b/src/checks.c index 4af3db7261..069d06d448 100644 --- a/src/checks.c +++ b/src/checks.c @@ -455,11 +455,10 @@ void set_server_down(struct check *check) s->counters.down_trans++; - if (s->state & SRV_CHECKED) - for (srv = s->tracknext; srv; srv = srv->tracknext) - if (!(srv->state & SRV_MAINTAIN)) - /* Only notify tracking servers that are not already in maintenance. */ - set_server_down(&srv->check); + for (srv = s->trackers; srv; srv = srv->tracknext) + if (!(srv->state & SRV_MAINTAIN)) + /* Only notify tracking servers that are not already in maintenance. */ + set_server_down(&srv->check); } check->health = 0; /* failure */ @@ -531,11 +530,10 @@ void set_server_up(struct check *check) { Warning("%s.\n", trash.str); send_log(s->proxy, LOG_NOTICE, "%s.\n", trash.str); - if (s->state & SRV_CHECKED) - for (srv = s->tracknext; srv; srv = srv->tracknext) - if (!(srv->state & SRV_MAINTAIN)) - /* Only notify tracking servers if they're not in maintenance. */ - set_server_up(&srv->check); + for (srv = s->trackers; srv; srv = srv->tracknext) + if (!(srv->state & SRV_MAINTAIN)) + /* Only notify tracking servers if they're not in maintenance. */ + set_server_up(&srv->check); } if (check->health >= check->rise) @@ -576,9 +574,8 @@ static void set_server_disabled(struct check *check) { if (!s->proxy->srv_bck && !s->proxy->srv_act) set_backend_down(s->proxy); - if (s->state & SRV_CHECKED) - for(srv = s->tracknext; srv; srv = srv->tracknext) - set_server_disabled(&srv->check); + for (srv = s->trackers; srv; srv = srv->tracknext) + set_server_disabled(&srv->check); } static void set_server_enabled(struct check *check) { @@ -610,9 +607,8 @@ static void set_server_enabled(struct check *check) { Warning("%s.\n", trash.str); send_log(s->proxy, LOG_NOTICE, "%s.\n", trash.str); - if (s->state & SRV_CHECKED) - for(srv = s->tracknext; srv; srv = srv->tracknext) - set_server_enabled(&srv->check); + for (srv = s->trackers; srv; srv = srv->tracknext) + set_server_enabled(&srv->check); } static void check_failed(struct check *check)