On Mon, Dec 16, 2013 at 04:46:55PM +0100, Sander Klein wrote:
> Hmmmm, dev20 does a slowstart when haproxy starts. Dev19 (and before) 
> doesn't do that.
> 
> It even does a slowstart when I reload the config file. That doesn't 
> seem right to me.

But you're perfectly right! I introduced this regression when fixing
another an issue with the agent checks (they prevented one from starting
a server after boot).

And I was happy to see the slowstart ramp up... I didn't realize it was
not normal during boot :-/

I now fixed it and pushed it after having tested on your config that it's
OK now. I'm attaching the fix for your convenience.

Thanks for reporting this!

Willy

>From 02541e8be22c212493ceeb88a54ccd32190db36d Mon Sep 17 00:00:00 2001
From: Willy Tarreau <[email protected]>
Date: Mon, 16 Dec 2013 18:08:36 +0100
Subject: BUG/MEDIUM: checks: servers must not start in slowstart mode

In 1.5-dev20, commit bb9665e (BUG/MEDIUM: checks: ensure we can enable
a server after boot) tried to fix a side effect of having both regular
checks and agent checks condition the up state propagation to servers.

Unfortunately it was still not fine because after this fix, servers
which make use of slowstart start in this mode. We must not check
the agent's health if agent checks are not enabled, and likewise,
we must not check the regular check's health if they are not enabled.

Reading the code, it seems like we could avoid entering this function
at all if (s->state & SRV_RUNNING) is not satisfied. Let's reserve
this for a later patch if needed.

Thanks to Sander Klein for reporting this abnormal situation.
---
 src/checks.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/checks.c b/src/checks.c
index 8014a66..115cc85 100644
--- a/src/checks.c
+++ b/src/checks.c
@@ -480,8 +480,10 @@ void set_server_up(struct check *check) {
        }
 
        if (s->track ||
-           (s->check.health == s->check.rise && (s->agent.health >= 
s->agent.rise || !(s->agent.state & CHK_ST_ENABLED))) ||
-           (s->agent.health == s->agent.rise && (s->check.health >= 
s->check.rise || !(s->check.state & CHK_ST_ENABLED)))) {
+           ((s->check.state & CHK_ST_ENABLED) && (s->check.health == 
s->check.rise) &&
+            (s->agent.health >= s->agent.rise || !(s->agent.state & 
CHK_ST_ENABLED))) ||
+           ((s->agent.state & CHK_ST_ENABLED) && (s->agent.health == 
s->agent.rise) &&
+            (s->check.health >= s->check.rise || !(s->check.state & 
CHK_ST_ENABLED)))) {
                if (s->proxy->srv_bck == 0 && s->proxy->srv_act == 0) {
                        if (s->proxy->last_change < now.tv_sec)         // 
ignore negative times
                                s->proxy->down_time += now.tv_sec - 
s->proxy->last_change;
-- 
1.7.12.1

Reply via email to