[PATCH] : rework checks handling

From: Krzysztof Oledzki <ole#ans.pl>
Date: Mon, 21 Jan 2008 02:00:20 +0100


From ac87ca3883886bd63546d311fe6a5ecd498cc31a Mon Sep 17 00:00:00 2001 From: Krzysztof Piotr Oledzki <ole#ans.pl> Date: Mon, 21 Jan 2008 01:54:06 +0100
Subject: [MEDIUM]: rework checks handling

This patch adds two new variables: fastinter and downinter. When server state is:

It allows to set something like:

        server sr6 127.0.51.61:80 cookie s6 check inter 10000 downinter 20000 fastinter 500 fall 3 weight 40 In the above example haproxy uses 10000ms between checks but as soon as one check fails fastinter (500ms) is used. If server is down downinter (20000) is used or fastinter (500ms) if one check pass. Fastinter is also used when haproxy starts.

New "timeout.check" variable was added, if set haproxy uses it as an additional read timeout, but only after a connection has been already established. I was thinking about using "timeout.server" here but most people set this with an addition reserve but still want checks to kick out laggy servers. Please also note that in most cases check request is much simpler and faster to handle than normal requests so this timeout should be smaller.

I also changed the timeout used for check connections establishing.

Changes from the previous version:

Compile tested only (sorry!) as I'm currently traveling but changes are rather small and trivial.

---
 doc/configuration.txt  |   63 +++++++++++++++++++++++++++++++++++++++--------
 include/proto/server.h |    1 +
 include/types/proxy.h  |    1 +
 include/types/server.h |    2 +-
 src/cfgparse.c         |   25 ++++++++++++++++++-
 src/checks.c           |   54 ++++++++++++++++++++++++++++++----------
 src/proxy.c            |    6 ++++-
 src/server.c           |   13 ++++++++++
 8 files changed, 137 insertions(+), 28 deletions(-)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 4dc529d..d505e9c 100644
--- a/doc/configuration.txt

+++ b/doc/configuration.txt
@@ -598,6 +598,7 @@ stats refresh X - X X stats scope X - X X stats uri X - X X stats hide-version X - X X
+timeout check X - X X
timeout client X X X - timeout clitimeout X X X - (deprecated) timeout connect X - X X @@ -3052,6 +3053,38 @@ stats hide-version See also : "stats auth", "stats enable", "stats realm", "stats uri"
+timeout check <timeout>
+ Set additional check timeout, but only after a connection has been already
+ established.
+
+ May be used in sections: defaults | frontend | listen | backend
+ yes | no | yes | yes
+ Arguments:
+ <timeout> is the timeout value specified in milliseconds by default, but
+ can be in any other unit if the number is suffixed by the unit,
+ as explained at the top of this document.
+
+ If set, haproxy uses min("timeout connect", "inter") as a connect timeout
+ for check and "timeout check" as an additional read timeout. The "min" is
+ used so that people running with *very* long "timeout connect" (eg. those
+ who needed this due to the queue or tarpit) do not slow down their checks.
+ Of course it is better to use "check queue" and "check tarpit" instead of
+ long "timeout connect".
+
+ If "timeout check" is not set haproxy uses "inter" for complete check
+ timeout (connect + read) exactly like all <1.3.15 version.
+
+ In most cases check request is much simpler and faster to handle than normal
+ requests and people may want to kick out laggy servers so this timeout should
+ be smaller than "timeout server".
+
+ This parameter is specific to backends, but can be specified once for all in
+ "defaults" sections. This is in fact one of the easiest solutions not to
+ forget about it.
+
+ See also: "timeout connect", "timeout queue", "timeout server", "timeout tarpit".
+
+
timeout client <timeout> timeout clitimeout <timeout> (deprecated) Set the maximum inactivity time on the client side. @@ -3102,8 +3135,8 @@ timeout contimeout <timeout> (deprecated) immediate (less than a few milliseconds). Anyway, it is a good practise to cover one or several TCP packet losses by specifying timeouts that are slightly above multiples of 3 seconds (eg: 4 or 5 seconds). By default, the - connect timeout also presets the queue timeout to the same value if this one - has not been specified.
+ connect timeout also presets both queue and tarpit timeouts to the same value
+ if these have not been specified.
This parameter is specific to backends, but can be specified once for all in "defaults" sections. This is in fact one of the easiest solutions not to @@ -3116,7 +3149,7 @@ timeout contimeout <timeout> (deprecated) to use it to write new configurations. The form "timeout contimeout" is provided only by backwards compatibility but its use is strongly discouraged. - See also : "timeout queue", "timeout server", "contimeout".
+ See also: "timeout check", "timeout queue", "timeout server", "timeout tarpit" "contimeout".
timeout http-request <timeout> @@ -3739,16 +3772,24 @@ fall <count> unspecified. See also the "check", "inter" and "rise" parameters. inter <delay>
+fastinter <delay>
+downinter <delay>
The "inter" parameter sets the interval between two consecutive health checks to <delay> milliseconds. If left unspecified, the delay defaults to 2000 ms. - Just as with every other time-based parameter, it can be entered in any other - explicit unit among { us, ms, s, m, h, d }. This parameter also serves as a - timeout for health checks sent to servers. In order to reduce "resonance" - effects when multiple servers are hosted on the same hardware, the - health-checks of all servers are started with a small time offset between - them. It is also possible to add some random noise in the health checks - interval using the global "spread-checks" keyword. This makes sense for - instance when a lot of backends use the same servers.
+ It is also possible to use "fastinter" and "downinter" to optimize delays
+ between checks. When server state is:
+ - non-transitionally UP -> haproxy uses inter,
+ - transitionally UP (going down), unchecked or transitionally DOWN (going up)
+ -> haproxy uses "fastinter" if set or "inter" otherwise,
+ - down -> haproxy uses downinter if set or "inter" otherwise.
+ Just as with every other time-based parameter, they can be entered in any other
+ explicit unit among { us, ms, s, m, h, d }. The "inter" parameter also serves
+ as a timeout for health checks sent to servers if "timeout check" is not set.
+ In order to reduce "resonance" effects when multiple servers are hosted on
+ the same hardware, the health-checks of all servers are started with a small
+ time offset between them. It is also possible to add some random noise in the
+ health checks interval using the global "spread-checks" keyword. This makes
+ sense for instance when a lot of backends use the same servers.
maxconn <maxconn> The "maxconn" parameter specifies the maximal number of concurrent diff --git a/include/proto/server.h b/include/proto/server.h index 27e4f36..f3b5e16 100644 --- a/include/proto/server.h
+++ b/include/proto/server.h
@@ -32,6 +32,7 @@ #include <proto/queue.h> int srv_downtime(struct server *s);
+int srv_getinter(struct server *s);
#endif /* _PROTO_SERVER_H */ diff --git a/include/types/proxy.h b/include/types/proxy.h index e0dff09..dd4f00f 100644 --- a/include/types/proxy.h
+++ b/include/types/proxy.h
@@ -178,6 +178,7 @@ struct proxy { struct timeval server; /* server I/O timeout (in milliseconds) */ struct timeval appsession; /* appsession cookie expiration */ struct timeval httpreq; /* maximum time for complete HTTP request */
+ struct timeval check; /* maximum time for complete check */
} timeout; char *id; /* proxy id */ struct list pendconns; /* pending connections with no server assigned yet */ diff --git a/include/types/server.h b/include/types/server.h index 730ed60..cfc4d7d 100644 --- a/include/types/server.h
+++ b/include/types/server.h
@@ -93,7 +93,7 @@ struct server { short check_port; /* the port to use for the health checks */ int health; /* 0->rise-1 = bad; rise->rise+fall-1 = good */ int rise, fall; /* time in iterations */ - int inter; /* time in milliseconds */
+ int inter, fastinter, downinter; /* checks: time in milliseconds */
int slowstart; /* slowstart time in seconds (ms in the conf) */ int result; /* health-check result : SRV_CHK_* */ int curfd; /* file desc used for current test, or -1 if not in test */ diff --git a/src/cfgparse.c b/src/cfgparse.c index ec66933..46b8972 100644 --- a/src/cfgparse.c
+++ b/src/cfgparse.c
@@ -679,6 +679,7 @@ int cfg_parse_listen(const char *file, int linenum, char **args, int inv) if (curproxy->cap & PR_CAP_BE) { curproxy->timeout.connect = defproxy.timeout.connect; curproxy->timeout.server = defproxy.timeout.server;
+ curproxy->timeout.check = defproxy.timeout.check;
curproxy->timeout.queue = defproxy.timeout.queue; curproxy->timeout.tarpit = defproxy.timeout.tarpit; curproxy->source_addr = defproxy.source_addr; @@ -1529,6 +1530,8 @@ int cfg_parse_listen(const char *file, int linenum, char **args, int inv) newsrv->curfd = -1; /* no health-check in progress */ newsrv->inter = DEF_CHKINTR;
+ newsrv->fastinter = 0; /* 0 => use newsrv->inter instead */
+ newsrv->downinter = 0; /* 0 => use newsrv->inter instead */
newsrv->rise = DEF_RISETIME; newsrv->fall = DEF_FALLTIME; newsrv->health = newsrv->rise; /* up, but will fall down at first failure */ @@ -1562,6 +1565,26 @@ int cfg_parse_listen(const char *file, int linenum, char **args, int inv) newsrv->inter = val; cur_arg += 2; }
+ else if (!strcmp(args[cur_arg], "fastinter")) {
+ const char *err = parse_time_err(args[cur_arg + 1], &val, TIME_UNIT_MS);
+ if (err) {
+ Alert("parsing [%s:%d]: unexpected character '%c' in 'fastinter' argument of server %s.\n",
+ file, linenum, *err, newsrv->id);
+ return -1;
+ }
+ newsrv->fastinter = val;
+ cur_arg += 2;
+ }
+ else if (!strcmp(args[cur_arg], "downinter")) {
+ const char *err = parse_time_err(args[cur_arg + 1], &val, TIME_UNIT_MS);
+ if (err) {
+ Alert("parsing [%s:%d]: unexpected character '%c' in 'downinter' argument of server %s.\n",
+ file, linenum, *err, newsrv->id);
+ return -1;
+ }
+ newsrv->downinter = val;
+ cur_arg += 2;
+ }
else if (!strcmp(args[cur_arg], "addr")) { newsrv->check_addr = *str2sa(args[cur_arg + 1]); cur_arg += 2; @@ -1667,7 +1690,7 @@ int cfg_parse_listen(const char *file, int linenum, char **args, int inv) return -1; } else { - Alert("parsing [%s:%d] : server %s only supports options 'backup', 'cookie', 'check', 'inter', 'rise', 'fall', 'addr', 'port', 'source', 'minconn', 'maxconn', 'maxqueue', 'slowstart' and 'weight'.\n",
+ Alert("parsing [%s:%d] : server %s only supports options 'backup', 'cookie', 'check', 'inter', 'fastinter', 'downinter', 'rise', 'fall', 'addr', 'port', 'source', 'minconn', 'maxconn', 'maxqueue', 'slowstart' and 'weight'.\n",
file, linenum, newsrv->id); return -1; } diff --git a/src/checks.c b/src/checks.c index ff02d01..124f40c 100644 --- a/src/checks.c
+++ b/src/checks.c
@@ -171,6 +171,7 @@ static int event_srv_chk_w(int fd) struct task *t = fdtab[fd].owner; struct server *s = t->context;
+ //fprintf(stderr, "event_srv_chk_w, state=%ld\n", unlikely(fdtab[fd].state));
if (unlikely(fdtab[fd].state == FD_STERROR || (fdtab[fd].ev & FD_POLL_ERR))) goto out_error; @@ -198,6 +199,10 @@ static int event_srv_chk_w(int fd) ret = send(fd, s->proxy->check_req, s->proxy->check_len, MSG_DONTWAIT | MSG_NOSIGNAL); #endif if (ret == s->proxy->check_len) {
+ /* we allow up to <timeout.check> if nonzero for a responce */
+ //fprintf(stderr, "event_srv_chk_w, ms=%lu\n", __tv_to_ms(&s->proxy->timeout.check));
+ tv_add_ifset(&t->expire, &now, &s->proxy->timeout.check);
+
EV_FD_SET(fd, DIR_RD); /* prepare for reading reply */ goto out_nowake; } @@ -462,6 +467,8 @@ void process_chk(struct task *t, struct timeval *next) if (s->result == SRV_CHK_UNKNOWN) { if ((connect(fd, (struct sockaddr *)&sa, sizeof(sa)) != -1) || (errno == EINPROGRESS)) {
+ struct timeval tv_con;
+
/* OK, connection in progress or established */ //fprintf(stderr, "process_chk: 4\n"); @@ -480,8 +487,17 @@ void process_chk(struct task *t, struct timeval *next) #ifdef DEBUG_FULL assert (!EV_FD_ISSET(fd, DIR_RD)); #endif - /* FIXME: we allow up to <inter> for a connection to establish, but we should use another parameter */
+ //fprintf(stderr, "process_chk: 4+, %lu\n", __tv_to_ms(&s->proxy->timeout.connect));
+ /* we allow up to min(inter, timeout.connect) for a connection
+ * to establish but only when timeout.check is set
+ * as it may be to short for a full check otherwise
+ */
+ tv_add(&tv_con, &now, &s->proxy->timeout.connect);
tv_ms_add(&t->expire, &now, s->inter);
+
+ if (tv_isset(&s->proxy->timeout.check))
+ tv_bound(&t->expire, &tv_con);
+
task_queue(t); /* restore t to its place in the task list */ *next = t->expire; return; @@ -509,10 +525,20 @@ void process_chk(struct task *t, struct timeval *next) else set_server_down(s); - //fprintf(stderr, "process_chk: 7\n"); - /* FIXME: we allow up to <inter> for a connection to establish, but we should use another parameter */ - while (tv_isle(&t->expire, &now))
+ //fprintf(stderr, "process_chk: 7, %lu\n", __tv_to_ms(&s->proxy->timeout.connect));
+ /* we allow up to min(inter, timeout.connect) for a connection
+ * to establish but only when timeout.check is set
+ * as it may be to short for a full check otherwise
+ */
+ while (tv_isle(&t->expire, &now)) {
+ struct timeval tv_con;
+
+ tv_add(&tv_con, &t->expire, &s->proxy->timeout.connect);
tv_ms_add(&t->expire, &t->expire, s->inter);
+
+ if (tv_isset(&s->proxy->timeout.check))
+ tv_bound(&t->expire, &tv_con);
+ }
goto new_chk; } else { @@ -647,11 +673,11 @@ void process_chk(struct task *t, struct timeval *next) rv = 0; if (global.spread_checks > 0) { - rv = s->inter * global.spread_checks / 100;
+ rv = srv_getinter(s) * global.spread_checks / 100;
rv -= (int) (2 * rv * (rand() / (RAND_MAX + 1.0))); - //fprintf(stderr, "process_chk(%p): (%d+/-%d%%) random=%d\n", s, s->inter, global.spread_checks, rv);
+ //fprintf(stderr, "process_chk(%p): (%d+/-%d%%) random=%d\n", s, srv_getinter(s), global.spread_checks, rv);
} - tv_ms_add(&t->expire, &now, s->inter + rv);
+ tv_ms_add(&t->expire, &now, srv_getinter(s) + rv);
goto new_chk; } else if ((s->result & SRV_CHK_ERROR) || tv_isle(&t->expire, &now)) { @@ -668,11 +694,11 @@ void process_chk(struct task *t, struct timeval *next) rv = 0; if (global.spread_checks > 0) { - rv = s->inter * global.spread_checks / 100;
+ rv = srv_getinter(s) * global.spread_checks / 100;
rv -= (int) (2 * rv * (rand() / (RAND_MAX + 1.0))); - //fprintf(stderr, "process_chk(%p): (%d+/-%d%%) random=%d\n", s, s->inter, global.spread_checks, rv);
+ //fprintf(stderr, "process_chk(%p): (%d+/-%d%%) random=%d\n", s, srv_getinter(s), global.spread_checks, rv);
} - tv_ms_add(&t->expire, &now, s->inter + rv);
+ tv_ms_add(&t->expire, &now, srv_getinter(s) + rv);
goto new_chk; } /* if result is unknown and there's no timeout, we have to wait again */ @@ -708,9 +734,9 @@ int start_checks() { if (!(s->state & SRV_CHECKED)) continue; - if ((s->inter >= SRV_CHK_INTER_THRES) && - (!mininter || mininter > s->inter)) - mininter = s->inter;
+ if ((srv_getinter(s) >= SRV_CHK_INTER_THRES) &&
+ (!mininter || mininter > srv_getinter(s)))
+ mininter = srv_getinter(s);
nbchk++; } @@ -744,7 +770,7 @@ int start_checks() { /* check this every ms */ tv_ms_add(&t->expire, &now, - ((mininter && mininter >= s->inter) ? mininter : s->inter) * srvpos / nbchk);
+ ((mininter && mininter >= srv_getinter(s)) ? mininter : srv_getinter(s)) * srvpos / nbchk);
task_queue(t); srvpos++; diff --git a/src/proxy.c b/src/proxy.c index 7019606..281ee8e 100644 --- a/src/proxy.c
+++ b/src/proxy.c
@@ -118,6 +118,10 @@ int proxy_parse_timeout(const char **args, struct proxy *proxy, tv = &proxy->timeout.connect; td = &defpx->timeout.connect; cap = PR_CAP_BE;
+ } else if (!strcmp(args[0], "check")) {
+ tv = &proxy->timeout.check;
+ td = &defpx->timeout.check;
+ cap = PR_CAP_BE;
} else if (!strcmp(args[0], "appsession")) { tv = &proxy->timeout.appsession; td = &defpx->timeout.appsession; @@ -128,7 +132,7 @@ int proxy_parse_timeout(const char **args, struct proxy *proxy, cap = PR_CAP_BE; } else { snprintf(err, errlen, - "timeout '%s': must be 'client', 'server', 'connect', "
+ "timeout '%s': must be 'client', 'server', 'connect', 'check', "
"'appsession', 'queue', 'http-request' or 'tarpit'", args[0]); return -1; diff --git a/src/server.c b/src/server.c index 8b0fa14..bf29d25 100644 --- a/src/server.c
+++ b/src/server.c
@@ -2,6 +2,7 @@ * Server management functions. * * Copyright 2000-2006 Willy Tarreau <w#1wt.eu>
+ * Copyright 2007-2008 Krzysztof Piotr Oledzki <ole#ans.pl>
* * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License @@ -25,6 +26,18 @@ int srv_downtime(struct server *s) { return now.tv_sec - s->last_change + s->down_time; }
+int srv_getinter(struct server *s) {
+
+ if ((s->state & SRV_CHECKED) && (s->health == s->rise + s->fall - 1))
+ return s->inter;
+
+ if (!(s->state & SRV_RUNNING) && s->health==0)
+ return (s->downinter)?(s->downinter):(s->inter);
+
+ return (s->fastinter)?(s->fastinter):(s->inter);
+}
+
+
/* * Local variables: * c-indent-level: 8 -- 1.5.3.7
Received on 2008/01/21 02:00

This archive was generated by hypermail 2.2.0 : 2008/01/21 02:15 CET