MINOR: server: change adm_st_chg_cause storage type

Even though it doesn't look like it at first glance, this is more like
a cleanup than an actual code improvement:

Given that srv->adm_st_chg_cause has been used to exclusively store
static strings ever since it was implemented, we make the choice to
store it as an enum instead of a fixed-size string within server
struct.

This will allow to save some space in server struct, and will make
it more easily exportable (ie: event handlers) because of the
reduced memory footprint during handling and the ability to later get
the corresponding human-readable message when it's explicitly needed.
This commit is contained in:
Aurelien DARRAGON 2023-04-03 17:40:28 +02:00 committed by Christopher Faulet
parent 85b91375bf
commit 9b1ccd7325
4 changed files with 60 additions and 25 deletions

View File

@ -189,6 +189,19 @@ enum srv_log_proto {
SRV_LOG_PROTO_OCTET_COUNTING, // TCP frames: MSGLEN SP MSG SRV_LOG_PROTO_OCTET_COUNTING, // TCP frames: MSGLEN SP MSG
}; };
/* srv administrative change causes */
enum srv_adm_st_chg_cause {
SRV_ADM_STCHGC_NONE = 0,
SRV_ADM_STCHGC_DNS_NOENT, /* entry removed from srv record */
SRV_ADM_STCHGC_DNS_NOIP, /* no server ip in the srv record */
SRV_ADM_STCHGC_DNS_NX, /* resolution spent too much time in NX state */
SRV_ADM_STCHGC_DNS_TIMEOUT, /* resolution timeout */
SRV_ADM_STCHGC_DNS_REFUSED, /* query refused by dns server */
SRV_ADM_STCHGC_DNS_UNSPEC, /* unspecified dns error */
SRV_ADM_STCHGC_STATS_DISABLE, /* legacy disable from the stats */
SRV_ADM_STCHGC_STATS_STOP /* legacy stop from the stats */
};
struct pid_list { struct pid_list {
struct list list; struct list list;
pid_t pid; pid_t pid;
@ -403,8 +416,8 @@ struct server {
long duration; long duration;
short status, code; short status, code;
char reason[128]; char reason[128];
} op_st_chg; /* operational status change's reason */ } op_st_chg; /* operational status change's reason */
char adm_st_chg_cause[48]; /* administrative 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) */ event_hdl_sub_list e_subs; /* event_hdl: server's subscribers list (atomically updated) */

View File

@ -64,6 +64,7 @@ void srv_take(struct server *srv);
struct server *srv_drop(struct server *srv); struct server *srv_drop(struct server *srv);
int srv_init_per_thr(struct server *srv); int srv_init_per_thr(struct server *srv);
void srv_set_ssl(struct server *s, int use_ssl); void srv_set_ssl(struct server *s, int use_ssl);
const char *srv_adm_st_chg_cause(enum srv_adm_st_chg_cause cause);
/* functions related to server name resolution */ /* functions related to server name resolution */
int srv_prepare_for_resolution(struct server *srv, const char *hostname); int srv_prepare_for_resolution(struct server *srv, const char *hostname);
@ -152,7 +153,7 @@ void srv_set_stopping(struct server *s, const char *reason, struct check *check)
* is done. If <cause> is non-null, it will be displayed at the end of the log * is done. If <cause> is non-null, it will be displayed at the end of the log
* lines to justify the state change. * lines to justify the state change.
*/ */
void srv_set_admin_flag(struct server *s, enum srv_admin mode, const char *cause); void srv_set_admin_flag(struct server *s, enum srv_admin mode, enum srv_adm_st_chg_cause cause);
/* Disables admin flag <mode> (among SRV_ADMF_*) on server <s>. This is used to /* Disables admin flag <mode> (among SRV_ADMF_*) on server <s>. This is used to
* stop enforcing either maint mode or drain mode. It is not allowed to set more * stop enforcing either maint mode or drain mode. It is not allowed to set more
@ -212,7 +213,7 @@ static inline int server_is_draining(const struct server *s)
*/ */
static inline void srv_adm_set_maint(struct server *s) static inline void srv_adm_set_maint(struct server *s)
{ {
srv_set_admin_flag(s, SRV_ADMF_FMAINT, NULL); srv_set_admin_flag(s, SRV_ADMF_FMAINT, SRV_ADM_STCHGC_NONE);
srv_clr_admin_flag(s, SRV_ADMF_FDRAIN); srv_clr_admin_flag(s, SRV_ADMF_FDRAIN);
} }
@ -221,7 +222,7 @@ static inline void srv_adm_set_maint(struct server *s)
*/ */
static inline void srv_adm_set_drain(struct server *s) static inline void srv_adm_set_drain(struct server *s)
{ {
srv_set_admin_flag(s, SRV_ADMF_FDRAIN, NULL); srv_set_admin_flag(s, SRV_ADMF_FDRAIN, SRV_ADM_STCHGC_NONE);
srv_clr_admin_flag(s, SRV_ADMF_FMAINT); srv_clr_admin_flag(s, SRV_ADMF_FMAINT);
} }

View File

@ -104,6 +104,23 @@ struct dict server_key_dict = {
.values = EB_ROOT_UNIQUE, .values = EB_ROOT_UNIQUE,
}; };
static const char *srv_adm_st_chg_cause_str[] = {
[SRV_ADM_STCHGC_NONE] = "",
[SRV_ADM_STCHGC_DNS_NOENT] = "entry removed from SRV record",
[SRV_ADM_STCHGC_DNS_NOIP] = "No IP for server ",
[SRV_ADM_STCHGC_DNS_NX] = "DNS NX status",
[SRV_ADM_STCHGC_DNS_TIMEOUT] = "DNS timeout status",
[SRV_ADM_STCHGC_DNS_REFUSED] = "DNS refused status",
[SRV_ADM_STCHGC_DNS_UNSPEC] = "unspecified DNS error",
[SRV_ADM_STCHGC_STATS_DISABLE] = "'disable' on stats page",
[SRV_ADM_STCHGC_STATS_STOP] = "'stop' on stats page"
};
const char *srv_adm_st_chg_cause(enum srv_adm_st_chg_cause cause)
{
return srv_adm_st_chg_cause_str[cause];
}
int srv_downtime(const struct server *s) int srv_downtime(const struct server *s)
{ {
if ((s->cur_state != SRV_ST_STOPPED) || s->last_change >= now.tv_sec) // ignore negative time if ((s->cur_state != SRV_ST_STOPPED) || s->last_change >= now.tv_sec) // ignore negative time
@ -1702,7 +1719,7 @@ void srv_set_stopping(struct server *s, const char *reason, struct check *check)
* *
* Must be called with the server lock held. * Must be called with the server lock held.
*/ */
void srv_set_admin_flag(struct server *s, enum srv_admin mode, const char *cause) void srv_set_admin_flag(struct server *s, enum srv_admin mode, enum srv_adm_st_chg_cause cause)
{ {
struct server *srv; struct server *srv;
@ -1714,8 +1731,7 @@ void srv_set_admin_flag(struct server *s, enum srv_admin mode, const char *cause
return; return;
s->next_admin |= mode; s->next_admin |= mode;
if (cause) s->adm_st_chg_cause = cause;
strlcpy2(s->adm_st_chg_cause, cause, sizeof(s->adm_st_chg_cause));
/* propagate changes */ /* propagate changes */
srv_update_status(s); srv_update_status(s);
@ -1792,10 +1808,10 @@ static void srv_propagate_admin_state(struct server *srv)
for (srv2 = srv->trackers; srv2; srv2 = srv2->tracknext) { for (srv2 = srv->trackers; srv2; srv2 = srv2->tracknext) {
HA_SPIN_LOCK(SERVER_LOCK, &srv2->lock); HA_SPIN_LOCK(SERVER_LOCK, &srv2->lock);
if (srv->next_admin & (SRV_ADMF_MAINT | SRV_ADMF_CMAINT)) if (srv->next_admin & (SRV_ADMF_MAINT | SRV_ADMF_CMAINT))
srv_set_admin_flag(srv2, SRV_ADMF_IMAINT, NULL); srv_set_admin_flag(srv2, SRV_ADMF_IMAINT, SRV_ADM_STCHGC_NONE);
if (srv->next_admin & SRV_ADMF_DRAIN) if (srv->next_admin & SRV_ADMF_DRAIN)
srv_set_admin_flag(srv2, SRV_ADMF_IDRAIN, NULL); srv_set_admin_flag(srv2, SRV_ADMF_IDRAIN, SRV_ADM_STCHGC_NONE);
HA_SPIN_UNLOCK(SERVER_LOCK, &srv2->lock); HA_SPIN_UNLOCK(SERVER_LOCK, &srv2->lock);
} }
} }
@ -3510,7 +3526,7 @@ int srvrq_update_srv_status(struct server *s, int has_no_ip)
if (s->next_admin & SRV_ADMF_RMAINT) if (s->next_admin & SRV_ADMF_RMAINT)
return 1; return 1;
srv_set_admin_flag(s, SRV_ADMF_RMAINT, "entry removed from SRV record"); srv_set_admin_flag(s, SRV_ADMF_RMAINT, SRV_ADM_STCHGC_DNS_NOENT);
return 0; return 0;
} }
@ -3546,8 +3562,7 @@ int snr_update_srv_status(struct server *s, int has_no_ip)
if (has_no_ip) { if (has_no_ip) {
if (s->next_admin & SRV_ADMF_RMAINT) if (s->next_admin & SRV_ADMF_RMAINT)
return 1; return 1;
srv_set_admin_flag(s, SRV_ADMF_RMAINT, srv_set_admin_flag(s, SRV_ADMF_RMAINT, SRV_ADM_STCHGC_DNS_NOIP);
"No IP for server ");
return 0; return 0;
} }
@ -3569,7 +3584,7 @@ int snr_update_srv_status(struct server *s, int has_no_ip)
if (s->next_admin & SRV_ADMF_RMAINT) if (s->next_admin & SRV_ADMF_RMAINT)
return 1; return 1;
srv_set_admin_flag(s, SRV_ADMF_RMAINT, "DNS NX status"); srv_set_admin_flag(s, SRV_ADMF_RMAINT, SRV_ADM_STCHGC_DNS_NX);
return 0; return 0;
case RSLV_STATUS_TIMEOUT: case RSLV_STATUS_TIMEOUT:
@ -3580,7 +3595,7 @@ int snr_update_srv_status(struct server *s, int has_no_ip)
if (s->next_admin & SRV_ADMF_RMAINT) if (s->next_admin & SRV_ADMF_RMAINT)
return 1; return 1;
srv_set_admin_flag(s, SRV_ADMF_RMAINT, "DNS timeout status"); srv_set_admin_flag(s, SRV_ADMF_RMAINT, SRV_ADM_STCHGC_DNS_TIMEOUT);
return 0; return 0;
case RSLV_STATUS_REFUSED: case RSLV_STATUS_REFUSED:
@ -3591,7 +3606,7 @@ int snr_update_srv_status(struct server *s, int has_no_ip)
if (s->next_admin & SRV_ADMF_RMAINT) if (s->next_admin & SRV_ADMF_RMAINT)
return 1; return 1;
srv_set_admin_flag(s, SRV_ADMF_RMAINT, "DNS refused status"); srv_set_admin_flag(s, SRV_ADMF_RMAINT, SRV_ADM_STCHGC_DNS_REFUSED);
return 0; return 0;
default: default:
@ -3602,7 +3617,7 @@ int snr_update_srv_status(struct server *s, int has_no_ip)
if (s->next_admin & SRV_ADMF_RMAINT) if (s->next_admin & SRV_ADMF_RMAINT)
return 1; return 1;
srv_set_admin_flag(s, SRV_ADMF_RMAINT, "unspecified DNS error"); srv_set_admin_flag(s, SRV_ADMF_RMAINT, SRV_ADM_STCHGC_DNS_UNSPEC);
return 0; return 0;
} }
@ -4021,7 +4036,7 @@ static int srv_iterate_initaddr(struct server *srv)
break; break;
case SRV_IADDR_NONE: case SRV_IADDR_NONE:
srv_set_admin_flag(srv, SRV_ADMF_RMAINT, NULL); srv_set_admin_flag(srv, SRV_ADMF_RMAINT, SRV_ADM_STCHGC_NONE);
if (return_code) { if (return_code) {
ha_warning("could not resolve address '%s', disabling server.\n", ha_warning("could not resolve address '%s', disabling server.\n",
name); name);
@ -5447,7 +5462,9 @@ static void srv_update_status(struct server *s)
chunk_printf(tmptrash, chunk_printf(tmptrash,
"%sServer %s/%s was DOWN and now enters maintenance%s%s%s", "%sServer %s/%s was DOWN and now enters maintenance%s%s%s",
s->flags & SRV_F_BACKUP ? "Backup " : "", s->proxy->id, s->id, s->flags & SRV_F_BACKUP ? "Backup " : "", s->proxy->id, s->id,
*(s->adm_st_chg_cause) ? " (" : "", s->adm_st_chg_cause, *(s->adm_st_chg_cause) ? ")" : ""); (s->adm_st_chg_cause) ? " (" : "",
srv_adm_st_chg_cause(s->adm_st_chg_cause),
(s->adm_st_chg_cause) ? ")" : "");
srv_append_status(tmptrash, s, NULL, -1, (s->next_admin & SRV_ADMF_FMAINT)); srv_append_status(tmptrash, s, NULL, -1, (s->next_admin & SRV_ADMF_FMAINT));
@ -5486,7 +5503,9 @@ static void srv_update_status(struct server *s)
"%sServer %s/%s is going DOWN for maintenance%s%s%s", "%sServer %s/%s is going DOWN for maintenance%s%s%s",
s->flags & SRV_F_BACKUP ? "Backup " : "", s->flags & SRV_F_BACKUP ? "Backup " : "",
s->proxy->id, s->id, s->proxy->id, s->id,
*(s->adm_st_chg_cause) ? " (" : "", s->adm_st_chg_cause, *(s->adm_st_chg_cause) ? ")" : ""); (s->adm_st_chg_cause) ? " (" : "",
srv_adm_st_chg_cause(s->adm_st_chg_cause),
(s->adm_st_chg_cause) ? ")" : "");
srv_append_status(tmptrash, s, NULL, xferred, (s->next_admin & SRV_ADMF_FMAINT)); srv_append_status(tmptrash, s, NULL, xferred, (s->next_admin & SRV_ADMF_FMAINT));
@ -5669,7 +5688,9 @@ static void srv_update_status(struct server *s)
if (tmptrash) { if (tmptrash) {
chunk_printf(tmptrash, "%sServer %s/%s enters drain state%s%s%s", chunk_printf(tmptrash, "%sServer %s/%s enters drain state%s%s%s",
s->flags & SRV_F_BACKUP ? "Backup " : "", s->proxy->id, s->id, s->flags & SRV_F_BACKUP ? "Backup " : "", s->proxy->id, s->id,
*(s->adm_st_chg_cause) ? " (" : "", s->adm_st_chg_cause, *(s->adm_st_chg_cause) ? ")" : ""); (s->adm_st_chg_cause) ? " (" : "",
srv_adm_st_chg_cause(s->adm_st_chg_cause),
(s->adm_st_chg_cause) ? ")" : "");
srv_append_status(tmptrash, s, NULL, xferred, (s->next_admin & SRV_ADMF_FDRAIN)); srv_append_status(tmptrash, s, NULL, xferred, (s->next_admin & SRV_ADMF_FDRAIN));
@ -5748,8 +5769,8 @@ static void srv_update_status(struct server *s)
} }
} }
/* Re-set log strings to empty */ /* Re-set adm st change to none */
*s->adm_st_chg_cause = 0; s->adm_st_chg_cause = SRV_ADM_STCHGC_NONE;
/* explicitly commit state changes (even if it was already applied implicitly /* explicitly commit state changes (even if it was already applied implicitly
* by some lb state change function), so we don't miss anything * by some lb state change function), so we don't miss anything

View File

@ -4170,7 +4170,7 @@ static int stats_process_http_post(struct stconn *sc)
if (!(sv->cur_admin & SRV_ADMF_FMAINT)) { if (!(sv->cur_admin & SRV_ADMF_FMAINT)) {
altered_servers++; altered_servers++;
total_servers++; total_servers++;
srv_set_admin_flag(sv, SRV_ADMF_FMAINT, "'disable' on stats page"); srv_set_admin_flag(sv, SRV_ADMF_FMAINT, SRV_ADM_STCHGC_STATS_DISABLE);
} }
break; break;
case ST_ADM_ACTION_ENABLE: case ST_ADM_ACTION_ENABLE:
@ -4182,7 +4182,7 @@ static int stats_process_http_post(struct stconn *sc)
break; break;
case ST_ADM_ACTION_STOP: case ST_ADM_ACTION_STOP:
if (!(sv->cur_admin & SRV_ADMF_FDRAIN)) { if (!(sv->cur_admin & SRV_ADMF_FDRAIN)) {
srv_set_admin_flag(sv, SRV_ADMF_FDRAIN, "'stop' on stats page"); srv_set_admin_flag(sv, SRV_ADMF_FDRAIN, SRV_ADM_STCHGC_STATS_STOP);
altered_servers++; altered_servers++;
total_servers++; total_servers++;
} }