From 74ffd0cd0cecaeec3cfa848cbc03beb08999cc72 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Thu, 3 Nov 2016 18:19:49 +0100 Subject: [PATCH 28/31] BUG/MEDIUM: srv-state: properly restore the DRAIN state There were seveal reports about the DRAIN state not being properly restored upon reload. It happens that the condition in the code does exactly the opposite of what the comment says, and the comment is right so the code is wrong. It's worth noting that the conditions are complex here due to the 2 available methods to set the drain state (CLI/agent, and config's weight). To paraphrase the updated comment in the code, there are two possible reasons for FDRAIN to have been present : - previous config weight was zero - "set server b/s drain" was sent to the CLI In the first case, we simply want to drop this drain state if the new weight is not zero anymore, meaning the administrator has intentionally turned the weight back to a positive value to enable the server again after an operation. In the second case, the drain state was forced on the CLI regardless of the config's weight so we don't want a change to the config weight to lose this status. What this means is : - if previous weight was 0 and new one is >0, drop the DRAIN state. - if the previous weight was >0, keep it. This fix must be backported to 1.6. (cherry picked from commit 22cace2f4c3cbeca27c1941c647e7ae38ec8c0c0) --- src/server.c | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/src/server.c b/src/server.c index 79b3cb2..b9e72b7 100644 --- a/src/server.c +++ b/src/server.c @@ -2093,15 +2093,23 @@ static void srv_update_state(struct server *srv, int version, char **params) /* apply drain mode if server is currently enabled */ if (!(srv->admin & SRV_ADMF_FMAINT) && (srv_admin_state & SRV_ADMF_FDRAIN)) { /* The SRV_ADMF_FDRAIN flag is inherited when srv->iweight is 0 - * (srv->iweight is the weight set up in configuration) - * so we don't want to apply it when srv_iweight is 0 and - * srv->iweight is greater than 0. Purpose is to give the - * chance to the admin to re-enable this server from configuration - * file by setting a new weight > 0. + * (srv->iweight is the weight set up in configuration). + * There are two possible reasons for FDRAIN to have been present : + * - previous config weight was zero + * - "set server b/s drain" was sent to the CLI + * + * In the first case, we simply want to drop this drain state + * if the new weight is not zero anymore, meaning the administrator + * has intentionally turned the weight back to a positive value to + * enable the server again after an operation. In the second case, + * the drain state was forced on the CLI regardless of the config's + * weight so we don't want a change to the config weight to lose this + * status. What this means is : + * - if previous weight was 0 and new one is >0, drop the DRAIN state. + * - if the previous weight was >0, keep it. */ - if ((srv_iweight == 0) && (srv->iweight > 0)) { + if (srv_iweight > 0 || srv->iweight == 0) srv_adm_set_drain(srv); - } } srv->last_change = date.tv_sec - srv_last_time_change; -- 2.7.3