these patches replace to 2 previous ones. I fixed a compilation warning
about possible used of uninitialized variable in the second patch.
I also ran the reg-tests successfully.
Cheers
>
From 0c5b17976ec703b12040d813bdd6ac975af7b4d7 Mon Sep 17 00:00:00 2001
From: Baptiste Assmann <bed...@gmail.com>
Date: Thu, 13 Jun 2019 13:24:29 +0200
Subject: [PATCH 2/2] MEDIUM: server: server-state global file stored in a tree
Server states can be recovered from either a "global" file (all backends)
or a "local" file (per backend).
The way the algorithm to parse the state file was first implemented was good
enough for a low number of backends and servers per backend.
Basically, for each backend the state file (global or local) is opened,
parsed entirely and for each line we check if it contains data related to
a server from the backend we're currently processing.
We must read the file entirely, just in case some lines for the current
backend are stored at the end of the file.
This does not scale at all!
This patch changes the behavior above for the "global" file only. Now,
the global file is read and parsed once and all lines it contains are
stored in a tree, for faster discovery.
This result in way much less fopen, fgets, and strcmp calls, which make
loading of very big state files very quick now.
---
include/types/server.h | 11 ++
src/server.c | 412 ++++++++++++++++++++++++++++-------------
2 files changed, 294 insertions(+), 129 deletions(-)
diff --git a/include/types/server.h b/include/types/server.h
index 4a0772685..568528976 100644
--- a/include/types/server.h
+++ b/include/types/server.h
@@ -345,6 +345,17 @@ struct server {
struct sockaddr_storage socks4_addr; /* the address of the SOCKS4 Proxy, including the port */
};
+
+/* Storage structure to load server-state lines from a flat file into
+ * an ebtree, for faster processing
+ */
+struct state_line {
+ char *line;
+ struct ebmb_node name_name;
+ /* WARNING don't put anything after name_name, it's used by the key */
+};
+
+
/* Descriptor for a "server" keyword. The ->parse() function returns 0 in case of
* success, or a combination of ERR_* flags if an error is encountered. The
* function pointer can be NULL if not implemented. The function also has an
diff --git a/src/server.c b/src/server.c
index 66fba992d..1a9dd1617 100644
--- a/src/server.c
+++ b/src/server.c
@@ -47,10 +47,14 @@
#include <proto/dns.h>
#include <netinet/tcp.h>
+#include <ebsttree.h>
+
static void srv_update_status(struct server *s);
static void srv_update_state(struct server *srv, int version, char **params);
static int srv_apply_lastaddr(struct server *srv, int *err_code);
static int srv_set_fqdn(struct server *srv, const char *fqdn, int dns_locked);
+static void srv_state_parse_line(char *buf, const int version, char **params, char **srv_params);
+static int srv_state_get_version(FILE *f);
/* List head of all known server keywords */
static struct srv_kw_list srv_keywords = {
@@ -69,6 +73,9 @@ struct dict server_name_dict = {
.values = EB_ROOT_UNIQUE,
};
+/* tree where global state_file is loaded */
+struct eb_root state_file = EB_ROOT;
+
int srv_downtime(const struct server *s)
{
if ((s->cur_state != SRV_ST_STOPPED) && s->last_change < now.tv_sec) // ignore negative time
@@ -3363,6 +3370,130 @@ static void srv_update_state(struct server *srv, int version, char **params)
}
}
+
+/*
+ * read next line from file <f> and return the server state version if one found.
+ * If no version is found, then 0 is returned
+ * Note that this should be the first read on <f>
+ */
+static int srv_state_get_version(FILE *f) {
+ char buf[2];
+ int ret;
+
+ /* first character of first line of the file must contain the version of the export */
+ if (fgets(buf, 2, f) == NULL) {
+ return 0;
+ }
+
+ ret = atoi(buf);
+ if ((ret < SRV_STATE_FILE_VERSION_MIN) ||
+ (ret > SRV_STATE_FILE_VERSION_MAX))
+ return 0;
+
+ return ret;
+}
+
+
+/*
+ * parses server state line stored in <buf> and supposedly in version <version>.
+ * Set <params> and <srv_params> accordingly.
+ * In case of error, params[0] is set to NULL.
+ */
+static void srv_state_parse_line(char *buf, const int version, char **params, char **srv_params)
+{
+ int buflen, arg, srv_arg;
+ char *cur, *end;
+
+ buflen = strlen(buf);
+ cur = buf;
+ end = cur + buflen;
+
+ /* we need at least one character */
+ if (buflen == 0) {
+ params[0] = NULL;
+ return;
+ }
+
+ /* ignore blank characters at the beginning of the line */
+ while (isspace(*cur))
+ ++cur;
+
+ /* Ignore empty or commented lines */
+ if (cur == end || *cur == '#') {
+ params[0] = NULL;
+ return;
+ }
+
+ /* truncated lines */
+ if (buf[buflen - 1] != '\n') {
+ //ha_warning("server-state file '%s': truncated line\n", filepath);
+ params[0] = NULL;
+ return;
+ }
+
+ /* Removes trailing '\n' */
+ buf[buflen - 1] = '\0';
+
+ /* we're now ready to move the line into *srv_params[] */
+ params[0] = cur;
+ arg = 1;
+ srv_arg = 0;
+ while (*cur && arg < SRV_STATE_FILE_MAX_FIELDS) {
+ if (isspace(*cur)) {
+ *cur = '\0';
+ ++cur;
+ while (isspace(*cur))
+ ++cur;
+ switch (version) {
+ case 1:
+ /*
+ * srv_addr: params[4] => srv_params[0]
+ * srv_op_state: params[5] => srv_params[1]
+ * srv_admin_state: params[6] => srv_params[2]
+ * srv_uweight: params[7] => srv_params[3]
+ * srv_iweight: params[8] => srv_params[4]
+ * srv_last_time_change: params[9] => srv_params[5]
+ * srv_check_status: params[10] => srv_params[6]
+ * srv_check_result: params[11] => srv_params[7]
+ * srv_check_health: params[12] => srv_params[8]
+ * srv_check_state: params[13] => srv_params[9]
+ * srv_agent_state: params[14] => srv_params[10]
+ * bk_f_forced_id: params[15] => srv_params[11]
+ * srv_f_forced_id: params[16] => srv_params[12]
+ * srv_fqdn: params[17] => srv_params[13]
+ * srv_port: params[18] => srv_params[14]
+ * srvrecord: params[19] => srv_params[15]
+ */
+ if (arg >= 4) {
+ srv_params[srv_arg] = cur;
+ ++srv_arg;
+ }
+ break;
+ }
+
+ params[arg] = cur;
+ ++arg;
+ }
+ else {
+ ++cur;
+ }
+ }
+
+ /* if line is incomplete line, then ignore it.
+ * otherwise, update useful flags */
+ switch (version) {
+ case 1:
+ if (arg < SRV_STATE_FILE_NB_FIELDS_VERSION_1) {
+ params[0] = NULL;
+ return;
+ }
+ break;
+ }
+
+ return;
+}
+
+
/* This function parses all the proxies and only take care of the backends (since we're looking for server)
* For each proxy, it does the following:
* - opens its server state file (either one or local one)
@@ -3379,12 +3510,10 @@ static void srv_update_state(struct server *srv, int version, char **params)
*/
void apply_server_state(void)
{
- char *cur, *end;
char mybuf[SRV_STATE_LINE_MAXLEN];
- int mybuflen;
char *params[SRV_STATE_FILE_MAX_FIELDS] = {0};
char *srv_params[SRV_STATE_FILE_MAX_FIELDS] = {0};
- int arg, srv_arg, version;
+ int version, global_file_version;
FILE *f;
char *filepath;
char globalfilepath[MAXPATHLEN + 1];
@@ -3392,7 +3521,13 @@ void apply_server_state(void)
int len, fileopenerr, globalfilepathlen, localfilepathlen;
struct proxy *curproxy, *bk;
struct server *srv;
+ char *line;
+ char *bkname, *srvname;
+ struct state_line *st;
+ struct ebmb_node *node, *next_node;
+
+ global_file_version = 0;
globalfilepathlen = 0;
/* create the globalfilepath variable */
if (global.server_state_file) {
@@ -3440,7 +3575,57 @@ void apply_server_state(void)
if (globalfilepathlen == 0)
globalfilepath[0] = '\0';
- /* read servers state from local file */
+ /* Load global server state in a tree */
+ if (globalfilepathlen > 0) {
+ errno = 0;
+ f = fopen(globalfilepath, "r");
+ if (errno)
+ ha_warning("Can't open global server state file '%s': %s\n", globalfilepath, strerror(errno));
+
+ global_file_version = srv_state_get_version(f);
+ if (global_file_version == 0)
+ goto out_load_server_state_in_tree;
+
+ while (fgets(mybuf, SRV_STATE_LINE_MAXLEN, f)) {
+ line = NULL;
+
+ line = strdup(mybuf);
+ if (line == NULL)
+ continue;
+
+ srv_state_parse_line(mybuf, global_file_version, params, srv_params);
+ if (params[0] == NULL)
+ continue;
+
+ /* bkname */
+ bkname = params[1];
+ /* srvname */
+ srvname = params[3];
+
+ /* key */
+ chunk_printf(&trash, "%s %s", bkname, srvname);
+
+ /* store line in tree */
+ st = calloc(1, sizeof(*st) + trash.size);
+ if (st == NULL) {
+ goto nextline;
+ }
+ memcpy(st->name_name.key, trash.area, trash.size);
+ ebst_insert(&state_file, &st->name_name);
+
+ /* save line */
+ st->line = line;
+
+ continue;
+
+ nextline:
+ /* free up memory in case of error during the processing of the line */
+ free(line);
+ }
+ }
+ out_load_server_state_in_tree:
+
+ /* parse all proxies and load states form tree (global file) or from local file */
for (curproxy = proxies_list; curproxy != NULL; curproxy = curproxy->next) {
/* servers are only in backends */
if (!(curproxy->cap & PR_CAP_BE))
@@ -3515,156 +3700,125 @@ void apply_server_state(void)
continue;
}
- /* preload global state file */
- errno = 0;
- f = fopen(filepath, "r");
- if (errno && fileopenerr)
- ha_warning("Can't open server state file '%s': %s\n", filepath, strerror(errno));
- if (!f)
- continue;
+ /* when global file is used, we get data from the tree
+ * Note that in such case we don't check backend name neither uuid.
+ * Backend name can't be wrong since it's used as a key to retrieve the server state
+ * line from the tree.
+ */
+ if (curproxy->load_server_state_from_file == PR_SRV_STATE_FILE_GLOBAL) {
+ struct server *srv = curproxy->srv;
+ while (srv) {
+ struct ebmb_node *node;
+ struct state_line *st;
- mybuf[0] = '\0';
- mybuflen = 0;
- version = 0;
+ chunk_printf(&trash, "%s %s", curproxy->id, srv->id);
+ node = ebst_lookup(&state_file, trash.area);
+ if (!node)
+ goto next;
- /* first character of first line of the file must contain the version of the export */
- if (fgets(mybuf, SRV_STATE_LINE_MAXLEN, f) == NULL) {
- ha_warning("Can't read first line of the server state file '%s'\n", filepath);
- goto fileclose;
- }
+ st = container_of(node, struct state_line, name_name);
+ memcpy(mybuf, st->line, strlen(st->line));
+ mybuf[strlen(st->line)] = 0;
- cur = mybuf;
- version = atoi(cur);
- if ((version < SRV_STATE_FILE_VERSION_MIN) ||
- (version > SRV_STATE_FILE_VERSION_MAX))
- goto fileclose;
+ srv_state_parse_line(mybuf, global_file_version, params, srv_params);
+ if (params[0] == NULL)
+ goto next;
- while (fgets(mybuf, SRV_STATE_LINE_MAXLEN, f)) {
- int bk_f_forced_id = 0;
- int check_id = 0;
- int check_name = 0;
-
- mybuflen = strlen(mybuf);
- cur = mybuf;
- end = cur + mybuflen;
+ srv_update_state(srv, global_file_version, srv_params);
- bk = NULL;
- srv = NULL;
+ next:
+ srv = srv->next;
+ }
- /* we need at least one character */
- if (mybuflen == 0)
+ continue; /* next proxy in list */
+ }
+ else {
+ /* load 'local' state file */
+ errno = 0;
+ f = fopen(filepath, "r");
+ if (errno && fileopenerr)
+ ha_warning("Can't open server state file '%s': %s\n", filepath, strerror(errno));
+ if (!f)
continue;
- /* ignore blank characters at the beginning of the line */
- while (isspace(*cur))
- ++cur;
-
- /* Ignore empty or commented lines */
- if (cur == end || *cur == '#')
- continue;
+ mybuf[0] = '\0';
- /* truncated lines */
- if (mybuf[mybuflen - 1] != '\n') {
- ha_warning("server-state file '%s': truncated line\n", filepath);
- continue;
+ /* first character of first line of the file must contain the version of the export */
+ version = srv_state_get_version(f);
+ if (version == 0) {
+ ha_warning("Can't get version of the server state file '%s'\n", filepath);
+ goto fileclose;
}
- /* Removes trailing '\n' */
- mybuf[mybuflen - 1] = '\0';
-
- /* we're now ready to move the line into *srv_params[] */
- params[0] = cur;
- arg = 1;
- srv_arg = 0;
- while (*cur && arg < SRV_STATE_FILE_MAX_FIELDS) {
- if (isspace(*cur)) {
- *cur = '\0';
- ++cur;
- while (isspace(*cur))
- ++cur;
- switch (version) {
- case 1:
- /*
- * srv_addr: params[4] => srv_params[0]
- * srv_op_state: params[5] => srv_params[1]
- * srv_admin_state: params[6] => srv_params[2]
- * srv_uweight: params[7] => srv_params[3]
- * srv_iweight: params[8] => srv_params[4]
- * srv_last_time_change: params[9] => srv_params[5]
- * srv_check_status: params[10] => srv_params[6]
- * srv_check_result: params[11] => srv_params[7]
- * srv_check_health: params[12] => srv_params[8]
- * srv_check_state: params[13] => srv_params[9]
- * srv_agent_state: params[14] => srv_params[10]
- * bk_f_forced_id: params[15] => srv_params[11]
- * srv_f_forced_id: params[16] => srv_params[12]
- * srv_fqdn: params[17] => srv_params[13]
- * srv_port: params[18] => srv_params[14]
- * srvrecord: params[19] => srv_params[15]
- */
- if (arg >= 4) {
- srv_params[srv_arg] = cur;
- ++srv_arg;
- }
- break;
- }
+ while (fgets(mybuf, SRV_STATE_LINE_MAXLEN, f)) {
+ int bk_f_forced_id = 0;
+ int check_id = 0;
+ int check_name = 0;
+
+ srv_state_parse_line(mybuf, version, params, srv_params);
- params[arg] = cur;
- ++arg;
+ if (params[0] == NULL) {
+ continue;
}
- else {
- ++cur;
+
+ /* if line is incomplete line, then ignore it.
+ * otherwise, update useful flags */
+ switch (version) {
+ case 1:
+ bk_f_forced_id = (atoi(params[15]) & PR_O_FORCED_ID);
+ check_id = (atoi(params[0]) == curproxy->uuid);
+ check_name = (strcmp(curproxy->id, params[1]) == 0);
+ break;
}
- }
- /* if line is incomplete line, then ignore it.
- * otherwise, update useful flags */
- switch (version) {
- case 1:
- if (arg < SRV_STATE_FILE_NB_FIELDS_VERSION_1)
+ bk = curproxy;
+
+ /* if backend can't be found, let's continue */
+ if (!check_id && !check_name)
+ continue;
+ else if (!check_id && check_name) {
+ ha_warning("backend ID mismatch: from server state file: '%s', from running config '%d'\n", params[0], bk->uuid);
+ send_log(bk, LOG_NOTICE, "backend ID mismatch: from server state file: '%s', from running config '%d'\n", params[0], bk->uuid);
+ }
+ else if (check_id && !check_name) {
+ ha_warning("backend name mismatch: from server state file: '%s', from running config '%s'\n", params[1], bk->id);
+ send_log(bk, LOG_NOTICE, "backend name mismatch: from server state file: '%s', from running config '%s'\n", params[1], bk->id);
+ /* if name doesn't match, we still want to update curproxy if the backend id
+ * was forced in previous the previous configuration */
+ if (!bk_f_forced_id)
continue;
- bk_f_forced_id = (atoi(params[15]) & PR_O_FORCED_ID);
- check_id = (atoi(params[0]) == curproxy->uuid);
- check_name = (strcmp(curproxy->id, params[1]) == 0);
- break;
- }
+ }
- bk = curproxy;
+ /* look for the server by its name: param[3] */
+ srv = server_find_best_match(bk, params[3], 0, NULL);
- /* if backend can't be found, let's continue */
- if (!check_id && !check_name)
- continue;
- else if (!check_id && check_name) {
- ha_warning("backend ID mismatch: from server state file: '%s', from running config '%d'\n", params[0], bk->uuid);
- send_log(bk, LOG_NOTICE, "backend ID mismatch: from server state file: '%s', from running config '%d'\n", params[0], bk->uuid);
- }
- else if (check_id && !check_name) {
- ha_warning("backend name mismatch: from server state file: '%s', from running config '%s'\n", params[1], bk->id);
- send_log(bk, LOG_NOTICE, "backend name mismatch: from server state file: '%s', from running config '%s'\n", params[1], bk->id);
- /* if name doesn't match, we still want to update curproxy if the backend id
- * was forced in previous the previous configuration */
- if (!bk_f_forced_id)
+ if (!srv) {
+ /* if no server found, then warning and continue with next line */
+ ha_warning("can't find server '%s' in backend '%s'\n",
+ params[3], params[1]);
+ send_log(bk, LOG_NOTICE, "can't find server '%s' in backend '%s'\n",
+ params[3], params[1]);
continue;
- }
-
- /* look for the server by its name: param[3] */
- srv = server_find_best_match(bk, params[3], 0, NULL);
+ }
- if (!srv) {
- /* if no server found, then warning and continue with next line */
- ha_warning("can't find server '%s' in backend '%s'\n",
- params[3], params[1]);
- send_log(bk, LOG_NOTICE, "can't find server '%s' in backend '%s'\n",
- params[3], params[1]);
- continue;
+ /* now we can proceed with server's state update */
+ srv_update_state(srv, version, srv_params);
}
-
- /* now we can proceed with server's state update */
- srv_update_state(srv, version, srv_params);
}
fileclose:
fclose(f);
}
+
+ /* now free memory allocated for the tree */
+ for (node = ebmb_first(&state_file), next_node = node ? ebmb_next(node) : NULL;
+ node;
+ node = next_node, next_node = node ? ebmb_next(node) : NULL) {
+ st = container_of(node, struct state_line, name_name);
+ ebmb_delete(&st->name_name);
+ free(st->line);
+ free(st);
+ }
+
}
/*
--
2.17.1
From cd3d3a7b731bc598f2ef2d18bc5d0036a652b5c2 Mon Sep 17 00:00:00 2001
From: Baptiste Assmann <bed...@gmail.com>
Date: Tue, 11 Jun 2019 14:51:49 +0200
Subject: [PATCH 1/2] MEDIUM: server: server-state only rely on server name
Since h7da71293e431b5ebb3d6289a55b0102331788ee6as has been added, the
server name (srv->id in the code) is now unique per backend, which
means it can reliabely be used to identify a server recovered from the
server-state file.
This patch cleans up the parsing of server-state file and ensure we use
only the server name as a reliable key.
---
src/server.c | 27 +++++++--------------------
1 file changed, 7 insertions(+), 20 deletions(-)
diff --git a/src/server.c b/src/server.c
index 0cbba7866..66fba992d 100644
--- a/src/server.c
+++ b/src/server.c
@@ -3384,7 +3384,7 @@ void apply_server_state(void)
int mybuflen;
char *params[SRV_STATE_FILE_MAX_FIELDS] = {0};
char *srv_params[SRV_STATE_FILE_MAX_FIELDS] = {0};
- int arg, srv_arg, version, diff;
+ int arg, srv_arg, version;
FILE *f;
char *filepath;
char globalfilepath[MAXPATHLEN + 1];
@@ -3629,7 +3629,6 @@ void apply_server_state(void)
break;
}
- diff = 0;
bk = curproxy;
/* if backend can't be found, let's continue */
@@ -3648,27 +3647,15 @@ void apply_server_state(void)
continue;
}
- /* look for the server by its id: param[2] */
- /* else look for the server by its name: param[3] */
- diff = 0;
- srv = server_find_best_match(bk, params[3], atoi(params[2]), &diff);
+ /* look for the server by its name: param[3] */
+ srv = server_find_best_match(bk, params[3], 0, NULL);
if (!srv) {
/* if no server found, then warning and continue with next line */
- ha_warning("can't find server '%s' with id '%s' in backend with id '%s' or name '%s'\n",
- params[3], params[2], params[0], params[1]);
- send_log(bk, LOG_NOTICE, "can't find server '%s' with id '%s' in backend with id '%s' or name '%s'\n",
- params[3], params[2], params[0], params[1]);
- continue;
- }
- else if (diff & PR_FBM_MISMATCH_ID) {
- ha_warning("In backend '%s' (id: '%d'): server ID mismatch: from server state file: '%s', from running config %d\n", bk->id, bk->uuid, params[2], srv->puid);
- send_log(bk, LOG_NOTICE, "In backend '%s' (id: %d): server ID mismatch: from server state file: '%s', from running config %d\n", bk->id, bk->uuid, params[2], srv->puid);
- continue;
- }
- else if (diff & PR_FBM_MISMATCH_NAME) {
- ha_warning("In backend '%s' (id: %d): server name mismatch: from server state file: '%s', from running config '%s'\n", bk->id, bk->uuid, params[3], srv->id);
- send_log(bk, LOG_NOTICE, "In backend '%s' (id: %d): server name mismatch: from server state file: '%s', from running config '%s'\n", bk->id, bk->uuid, params[3], srv->id);
+ ha_warning("can't find server '%s' in backend '%s'\n",
+ params[3], params[1]);
+ send_log(bk, LOG_NOTICE, "can't find server '%s' in backend '%s'\n",
+ params[3], params[1]);
continue;
}
--
2.17.1
