MINOR: server: change srv_op_st_chg_cause storage type
This one is greatly inspired by "MINOR: server: change adm_st_chg_cause storage type".
While looking at current srv_op_st_chg_cause usage, it was clear that
the struct needed some cleanup since some leftovers from asynchronous server
state change updates were left behind and resulted in some useless code
duplication, and making the whole thing harder to maintain.
Two observations were made:
- by tracking down srv_set_{running, stopped, stopping} usage,
we can see that the <reason> argument is always a fixed statically
allocated string.
- check-related state change context (duration, status, code...) is
not used anymore since srv_append_status() directly extracts the
values from the server->check. This is pure legacy from when
the state changes were applied asynchronously.
To prevent code duplication, useless string copies and make the reason/cause
more exportable, we store it as an enum now, and we provide
srv_op_st_chg_cause() function to fetch the related description string.
HEALTH and AGENT causes (check related) are now explicitly identified to
make consumers like srv_append_op_chg_cause() able to fetch checks info
from the server itself if they need to.
diff --git a/include/haproxy/server-t.h b/include/haproxy/server-t.h
index 0036054..c45c614 100644
--- a/include/haproxy/server-t.h
+++ b/include/haproxy/server-t.h
@@ -202,6 +202,17 @@
SRV_ADM_STCHGC_STATS_STOP /* legacy stop from the stats */
};
+/* srv operational change causes */
+enum srv_op_st_chg_cause {
+ SRV_OP_STCHGC_NONE = 0,
+ SRV_OP_STCHGC_HEALTH, /* changed from a health check */
+ SRV_OP_STCHGC_AGENT, /* changed from an agent check */
+ SRV_OP_STCHGC_CLI, /* changed from the cli */
+ SRV_OP_STCHGC_LUA, /* changed from lua */
+ SRV_OP_STCHGC_STATS_WEB, /* changed from the web interface */
+ SRV_OP_STCHGC_STATEFILE /* changed from state file */
+};
+
struct pid_list {
struct list list;
pid_t pid;
@@ -412,11 +423,7 @@
int nb_low;
int nb_high;
} tmpl_info;
- struct {
- long duration;
- short status, code;
- char reason[128];
- } op_st_chg; /* operational status change's reason */
+ enum srv_op_st_chg_cause op_st_chg_cause; /* operational status change's cause */
enum srv_adm_st_chg_cause adm_st_chg_cause; /* administrative status change's cause */
event_hdl_sub_list e_subs; /* event_hdl: server's subscribers list (atomically updated) */
diff --git a/include/haproxy/server.h b/include/haproxy/server.h
index f58afd1..96c7a04 100644
--- a/include/haproxy/server.h
+++ b/include/haproxy/server.h
@@ -65,6 +65,7 @@
int srv_init_per_thr(struct server *srv);
void srv_set_ssl(struct server *s, int use_ssl);
const char *srv_adm_st_chg_cause(enum srv_adm_st_chg_cause cause);
+const char *srv_op_st_chg_cause(enum srv_op_st_chg_cause cause);
/* functions related to server name resolution */
int srv_prepare_for_resolution(struct server *srv, const char *hostname);
@@ -141,9 +142,9 @@
void srv_append_status(struct buffer *msg, struct server *s, struct check *,
int xferred, int forced);
-void srv_set_stopped(struct server *s, const char *reason, struct check *check);
-void srv_set_running(struct server *s, const char *reason, struct check *check);
-void srv_set_stopping(struct server *s, const char *reason, struct check *check);
+void srv_set_stopped(struct server *s, enum srv_op_st_chg_cause cause);
+void srv_set_running(struct server *s, enum srv_op_st_chg_cause cause);
+void srv_set_stopping(struct server *s, enum srv_op_st_chg_cause cause);
/* Enables admin flag <mode> (among SRV_ADMF_*) on server <s>. This is used to
* enforce either maint mode or drain mode. It is not allowed to set more than
diff --git a/src/check.c b/src/check.c
index 2ff3bdb..e759002 100644
--- a/src/check.c
+++ b/src/check.c
@@ -566,6 +566,16 @@
}
}
+static inline enum srv_op_st_chg_cause check_notify_cause(struct check *check)
+{
+ struct server *s = check->server;
+
+ /* We only report a cause for the check if we did not do so previously */
+ if (!s->track && !(s->proxy->options2 & PR_O2_LOGHCHKS))
+ return (check->state & CHK_ST_AGENT) ? SRV_OP_STCHGC_AGENT : SRV_OP_STCHGC_HEALTH;
+ return SRV_OP_STCHGC_NONE;
+}
+
/* Marks the check <check>'s server down if the current check is already failed
* and the server is not down yet nor in maintenance.
*/
@@ -586,8 +596,7 @@
return;
TRACE_STATE("health-check failed, set server DOWN", CHK_EV_HCHK_END|CHK_EV_HCHK_ERR, check);
- /* We only report a reason for the check if we did not do so previously */
- srv_set_stopped(s, NULL, (!s->track && !(s->proxy->options2 & PR_O2_LOGHCHKS)) ? check : NULL);
+ srv_set_stopped(s, check_notify_cause(check));
}
/* Marks the check <check> as valid and tries to set its server up, provided
@@ -621,7 +630,7 @@
return;
TRACE_STATE("health-check succeeded, set server RUNNING", CHK_EV_HCHK_END|CHK_EV_HCHK_SUCC, check);
- srv_set_running(s, NULL, (!s->track && !(s->proxy->options2 & PR_O2_LOGHCHKS)) ? check : NULL);
+ srv_set_running(s, check_notify_cause(check));
}
/* Marks the check <check> as valid and tries to set its server into stopping mode
@@ -650,7 +659,7 @@
return;
TRACE_STATE("health-check condionnaly succeeded, set server STOPPING", CHK_EV_HCHK_END|CHK_EV_HCHK_SUCC, check);
- srv_set_stopping(s, NULL, (!s->track && !(s->proxy->options2 & PR_O2_LOGHCHKS)) ? check : NULL);
+ srv_set_stopping(s, check_notify_cause(check));
}
/* note: use health_adjust() only, which first checks that the observe mode is
diff --git a/src/hlua_fcn.c b/src/hlua_fcn.c
index 5a46a0d..f275c54 100644
--- a/src/hlua_fcn.c
+++ b/src/hlua_fcn.c
@@ -1268,7 +1268,7 @@
HA_SPIN_LOCK(SERVER_LOCK, &sv->lock);
if (!(sv->track)) {
sv->check.health = sv->check.rise + sv->check.fall - 1;
- srv_set_running(sv, "changed from Lua script", NULL);
+ srv_set_running(sv, SRV_OP_STCHGC_LUA);
}
HA_SPIN_UNLOCK(SERVER_LOCK, &sv->lock);
return 0;
@@ -1285,7 +1285,7 @@
HA_SPIN_LOCK(SERVER_LOCK, &sv->lock);
if (!(sv->track)) {
sv->check.health = sv->check.rise + sv->check.fall - 1;
- srv_set_stopping(sv, "changed from Lua script", NULL);
+ srv_set_stopping(sv, SRV_OP_STCHGC_LUA);
}
HA_SPIN_UNLOCK(SERVER_LOCK, &sv->lock);
return 0;
@@ -1302,7 +1302,7 @@
HA_SPIN_LOCK(SERVER_LOCK, &sv->lock);
if (!(sv->track)) {
sv->check.health = 0;
- srv_set_stopped(sv, "changed from Lua script", NULL);
+ srv_set_stopped(sv, SRV_OP_STCHGC_LUA);
}
HA_SPIN_UNLOCK(SERVER_LOCK, &sv->lock);
return 0;
@@ -1351,7 +1351,7 @@
HA_SPIN_LOCK(SERVER_LOCK, &sv->lock);
if (sv->agent.state & CHK_ST_ENABLED) {
sv->agent.health = sv->agent.rise + sv->agent.fall - 1;
- srv_set_running(sv, "changed from Lua script", NULL);
+ srv_set_running(sv, SRV_OP_STCHGC_LUA);
}
HA_SPIN_UNLOCK(SERVER_LOCK, &sv->lock);
return 0;
@@ -1368,7 +1368,7 @@
HA_SPIN_LOCK(SERVER_LOCK, &sv->lock);
if (sv->agent.state & CHK_ST_ENABLED) {
sv->agent.health = 0;
- srv_set_stopped(sv, "changed from Lua script", NULL);
+ srv_set_stopped(sv, SRV_OP_STCHGC_LUA);
}
HA_SPIN_UNLOCK(SERVER_LOCK, &sv->lock);
return 0;
diff --git a/src/server.c b/src/server.c
index 501657b..f8695d9 100644
--- a/src/server.c
+++ b/src/server.c
@@ -121,6 +121,21 @@
return srv_adm_st_chg_cause_str[cause];
}
+static const char *srv_op_st_chg_cause_str[] = {
+ [SRV_OP_STCHGC_NONE] = "",
+ [SRV_OP_STCHGC_HEALTH] = "",
+ [SRV_OP_STCHGC_AGENT] = "",
+ [SRV_OP_STCHGC_CLI] = "changed from CLI",
+ [SRV_OP_STCHGC_LUA] = "changed from Lua script",
+ [SRV_OP_STCHGC_STATS_WEB] = "changed from Web interface",
+ [SRV_OP_STCHGC_STATEFILE] = "changed from server-state after a reload"
+};
+
+const char *srv_op_st_chg_cause(enum srv_op_st_chg_cause cause)
+{
+ return srv_op_st_chg_cause_str[cause];
+}
+
int srv_downtime(const struct server *s)
{
if ((s->cur_state != SRV_ST_STOPPED) || s->last_change >= now.tv_sec) // ignore negative time
@@ -1511,12 +1526,21 @@
srv_shutdown_streams(srv, why);
}
-static void srv_append_op_chg_cause(struct buffer *msg, struct server *s, struct check *check)
+static void srv_append_op_chg_cause(struct buffer *msg, struct server *s)
{
- if (check)
- check_append_info(msg, check);
- else if (s->op_st_chg.reason && *s->op_st_chg.reason)
- chunk_appendf(msg, ", %s", s->op_st_chg.reason);
+ switch (s->op_st_chg_cause) {
+ case SRV_OP_STCHGC_NONE:
+ break; /* do nothing */
+ case SRV_OP_STCHGC_HEALTH:
+ check_append_info(msg, &s->check);
+ break;
+ case SRV_OP_STCHGC_AGENT:
+ check_append_info(msg, &s->agent);
+ break;
+ default:
+ chunk_appendf(msg, ", %s", srv_op_st_chg_cause(s->op_st_chg_cause));
+ break;
+ }
}
static void srv_append_adm_chg_cause(struct buffer *msg, struct server *s)
@@ -1560,13 +1584,11 @@
/* Marks server <s> down, regardless of its checks' statuses. The server is
* registered in a list to postpone the counting of the remaining servers on
* the proxy and transfers queued streams whenever possible to other servers at
- * a sync point. Maintenance servers are ignored. It stores the <reason> if
- * non-null as the reason for going down or the available data from the check
- * struct to recompute this reason later.
+ * a sync point. Maintenance servers are ignored.
*
* Must be called with the server lock held.
*/
-void srv_set_stopped(struct server *s, const char *reason, struct check *check)
+void srv_set_stopped(struct server *s, enum srv_op_st_chg_cause cause)
{
struct server *srv;
@@ -1574,24 +1596,14 @@
return;
s->next_state = SRV_ST_STOPPED;
- *s->op_st_chg.reason = 0;
- s->op_st_chg.status = -1;
- if (reason) {
- strlcpy2(s->op_st_chg.reason, reason, sizeof(s->op_st_chg.reason));
- }
- else if (check) {
- strlcpy2(s->op_st_chg.reason, check->desc, sizeof(s->op_st_chg.reason));
- s->op_st_chg.code = check->code;
- s->op_st_chg.status = check->status;
- s->op_st_chg.duration = check->duration;
- }
+ s->op_st_chg_cause = cause;
/* propagate changes */
srv_update_status(s);
for (srv = s->trackers; srv; srv = srv->tracknext) {
HA_SPIN_LOCK(SERVER_LOCK, &srv->lock);
- srv_set_stopped(srv, NULL, NULL);
+ srv_set_stopped(srv, SRV_OP_STCHGC_NONE);
HA_SPIN_UNLOCK(SERVER_LOCK, &srv->lock);
}
}
@@ -1599,13 +1611,11 @@
/* Marks server <s> up regardless of its checks' statuses and provided it isn't
* in maintenance. The server is registered in a list to postpone the counting
* of the remaining servers on the proxy and tries to grab requests from the
- * proxy at a sync point. Maintenance servers are ignored. It stores the
- * <reason> if non-null as the reason for going down or the available data
- * from the check struct to recompute this reason later.
+ * proxy at a sync point. Maintenance servers are ignored.
*
* Must be called with the server lock held.
*/
-void srv_set_running(struct server *s, const char *reason, struct check *check)
+void srv_set_running(struct server *s, enum srv_op_st_chg_cause cause)
{
struct server *srv;
@@ -1616,17 +1626,7 @@
return;
s->next_state = SRV_ST_STARTING;
- *s->op_st_chg.reason = 0;
- s->op_st_chg.status = -1;
- if (reason) {
- strlcpy2(s->op_st_chg.reason, reason, sizeof(s->op_st_chg.reason));
- }
- else if (check) {
- strlcpy2(s->op_st_chg.reason, check->desc, sizeof(s->op_st_chg.reason));
- s->op_st_chg.code = check->code;
- s->op_st_chg.status = check->status;
- s->op_st_chg.duration = check->duration;
- }
+ s->op_st_chg_cause = cause;
if (s->slowstart <= 0)
s->next_state = SRV_ST_RUNNING;
@@ -1636,7 +1636,7 @@
for (srv = s->trackers; srv; srv = srv->tracknext) {
HA_SPIN_LOCK(SERVER_LOCK, &srv->lock);
- srv_set_running(srv, NULL, NULL);
+ srv_set_running(srv, SRV_OP_STCHGC_NONE);
HA_SPIN_UNLOCK(SERVER_LOCK, &srv->lock);
}
}
@@ -1644,15 +1644,11 @@
/* Marks server <s> stopping regardless of its checks' statuses and provided it
* isn't in maintenance. The server is registered in a list to postpone the
* counting of the remaining servers on the proxy and tries to grab requests
- * from the proxy. Maintenance servers are ignored. It stores the
- * <reason> if non-null as the reason for going down or the available data
- * from the check struct to recompute this reason later.
- * up. Note that it makes use of the trash to build the log strings, so <reason>
- * must not be placed there.
+ * from the proxy. Maintenance servers are ignored.
*
* Must be called with the server lock held.
*/
-void srv_set_stopping(struct server *s, const char *reason, struct check *check)
+void srv_set_stopping(struct server *s, enum srv_op_st_chg_cause cause)
{
struct server *srv;
@@ -1663,24 +1659,14 @@
return;
s->next_state = SRV_ST_STOPPING;
- *s->op_st_chg.reason = 0;
- s->op_st_chg.status = -1;
- if (reason) {
- strlcpy2(s->op_st_chg.reason, reason, sizeof(s->op_st_chg.reason));
- }
- else if (check) {
- strlcpy2(s->op_st_chg.reason, check->desc, sizeof(s->op_st_chg.reason));
- s->op_st_chg.code = check->code;
- s->op_st_chg.status = check->status;
- s->op_st_chg.duration = check->duration;
- }
+ s->op_st_chg_cause = cause;
/* propagate changes */
srv_update_status(s);
for (srv = s->trackers; srv; srv = srv->tracknext) {
HA_SPIN_LOCK(SERVER_LOCK, &srv->lock);
- srv_set_stopping(srv, NULL, NULL);
+ srv_set_stopping(srv, SRV_OP_STCHGC_NONE);
HA_SPIN_UNLOCK(SERVER_LOCK, &srv->lock);
}
}
@@ -4199,15 +4185,15 @@
cli_err(appctx, "cannot change health on a tracking server.\n");
else if (strcmp(args[4], "up") == 0) {
sv->check.health = sv->check.rise + sv->check.fall - 1;
- srv_set_running(sv, "changed from CLI", NULL);
+ srv_set_running(sv, SRV_OP_STCHGC_CLI);
}
else if (strcmp(args[4], "stopping") == 0) {
sv->check.health = sv->check.rise + sv->check.fall - 1;
- srv_set_stopping(sv, "changed from CLI", NULL);
+ srv_set_stopping(sv, SRV_OP_STCHGC_CLI);
}
else if (strcmp(args[4], "down") == 0) {
sv->check.health = 0;
- srv_set_stopped(sv, "changed from CLI", NULL);
+ srv_set_stopped(sv, SRV_OP_STCHGC_CLI);
}
else
cli_err(appctx, "'set server <srv> health' expects 'up', 'stopping', or 'down'.\n");
@@ -4219,11 +4205,11 @@
cli_err(appctx, "agent checks are not enabled on this server.\n");
else if (strcmp(args[4], "up") == 0) {
sv->agent.health = sv->agent.rise + sv->agent.fall - 1;
- srv_set_running(sv, "changed from CLI", NULL);
+ srv_set_running(sv, SRV_OP_STCHGC_CLI);
}
else if (strcmp(args[4], "down") == 0) {
sv->agent.health = 0;
- srv_set_stopped(sv, "changed from CLI", NULL);
+ srv_set_stopped(sv, SRV_OP_STCHGC_CLI);
}
else
cli_err(appctx, "'set server <srv> agent' expects 'up' or 'down'.\n");
@@ -5330,7 +5316,7 @@
"%sServer %s/%s is DOWN", s->flags & SRV_F_BACKUP ? "Backup " : "",
s->proxy->id, s->id);
- srv_append_op_chg_cause(tmptrash, s, NULL);
+ srv_append_op_chg_cause(tmptrash, s);
srv_append_more(tmptrash, s, xferred, 0);
ha_warning("%s.\n", tmptrash->area);
@@ -5360,7 +5346,7 @@
"%sServer %s/%s is stopping", s->flags & SRV_F_BACKUP ? "Backup " : "",
s->proxy->id, s->id);
- srv_append_op_chg_cause(tmptrash, s, NULL);
+ srv_append_op_chg_cause(tmptrash, s);
srv_append_more(tmptrash, s, xferred, 0);
ha_warning("%s.\n", tmptrash->area);
@@ -5403,7 +5389,7 @@
"%sServer %s/%s is UP", s->flags & SRV_F_BACKUP ? "Backup " : "",
s->proxy->id, s->id);
- srv_append_op_chg_cause(tmptrash, s, NULL);
+ srv_append_op_chg_cause(tmptrash, s);
srv_append_more(tmptrash, s, xferred, 0);
ha_warning("%s.\n", tmptrash->area);
@@ -5423,10 +5409,8 @@
s->next_admin = next_admin;
}
- /* reset operational state change */
- *s->op_st_chg.reason = 0;
- s->op_st_chg.status = s->op_st_chg.code = -1;
- s->op_st_chg.duration = 0;
+ /* reset operational change cause */
+ s->op_st_chg_cause = SRV_OP_STCHGC_NONE;
/* Now we try to apply pending admin changes */
diff --git a/src/server_state.c b/src/server_state.c
index e0a1373..677de88 100644
--- a/src/server_state.c
+++ b/src/server_state.c
@@ -244,7 +244,7 @@
switch (srv_op_state) {
case SRV_ST_STOPPED:
srv->check.health = 0;
- srv_set_stopped(srv, "changed from server-state after a reload", NULL);
+ srv_set_stopped(srv, SRV_OP_STCHGC_STATEFILE);
break;
case SRV_ST_STARTING:
/* If rise == 1 there is no STARTING state, let's switch to
@@ -252,7 +252,7 @@
*/
if (srv->check.rise == 1) {
srv->check.health = srv->check.rise + srv->check.fall - 1;
- srv_set_running(srv, "", NULL);
+ srv_set_running(srv, SRV_OP_STCHGC_NONE);
break;
}
if (srv->check.health < 1 || srv->check.health >= srv->check.rise)
@@ -265,17 +265,17 @@
*/
if (srv->check.fall == 1) {
srv->check.health = 0;
- srv_set_stopped(srv, "changed from server-state after a reload", NULL);
+ srv_set_stopped(srv, SRV_OP_STCHGC_STATEFILE);
break;
}
if (srv->check.health < srv->check.rise ||
srv->check.health > srv->check.rise + srv->check.fall - 2)
srv->check.health = srv->check.rise;
- srv_set_stopping(srv, "changed from server-state after a reload", NULL);
+ srv_set_stopping(srv, SRV_OP_STCHGC_STATEFILE);
break;
case SRV_ST_RUNNING:
srv->check.health = srv->check.rise + srv->check.fall - 1;
- srv_set_running(srv, "", NULL);
+ srv_set_running(srv, SRV_OP_STCHGC_NONE);
break;
}
diff --git a/src/stats.c b/src/stats.c
index 7bb7f81..79a9a06 100644
--- a/src/stats.c
+++ b/src/stats.c
@@ -4211,7 +4211,7 @@
case ST_ADM_ACTION_HRUNN:
if (!(sv->track)) {
sv->check.health = sv->check.rise + sv->check.fall - 1;
- srv_set_running(sv, "changed from Web interface", NULL);
+ srv_set_running(sv, SRV_OP_STCHGC_STATS_WEB);
altered_servers++;
total_servers++;
}
@@ -4219,7 +4219,7 @@
case ST_ADM_ACTION_HNOLB:
if (!(sv->track)) {
sv->check.health = sv->check.rise + sv->check.fall - 1;
- srv_set_stopping(sv, "changed from Web interface", NULL);
+ srv_set_stopping(sv, SRV_OP_STCHGC_STATS_WEB);
altered_servers++;
total_servers++;
}
@@ -4227,7 +4227,7 @@
case ST_ADM_ACTION_HDOWN:
if (!(sv->track)) {
sv->check.health = 0;
- srv_set_stopped(sv, "changed from Web interface", NULL);
+ srv_set_stopped(sv, SRV_OP_STCHGC_STATS_WEB);
altered_servers++;
total_servers++;
}
@@ -4249,7 +4249,7 @@
case ST_ADM_ACTION_ARUNN:
if (sv->agent.state & CHK_ST_ENABLED) {
sv->agent.health = sv->agent.rise + sv->agent.fall - 1;
- srv_set_running(sv, "changed from Web interface", NULL);
+ srv_set_running(sv, SRV_OP_STCHGC_STATS_WEB);
altered_servers++;
total_servers++;
}
@@ -4257,7 +4257,7 @@
case ST_ADM_ACTION_ADOWN:
if (sv->agent.state & CHK_ST_ENABLED) {
sv->agent.health = 0;
- srv_set_stopped(sv, "changed from Web interface", NULL);
+ srv_set_stopped(sv, SRV_OP_STCHGC_STATS_WEB);
altered_servers++;
total_servers++;
}