mirrors/haproxy
1
0
Fork 0
mirror of https://git.haproxy.org/git/haproxy.git/ synced 2025-09-21 22:01:31 +02:00
Code Issues Projects Releases Wiki Activity

MEDIUM: server: simplify snr_set_srv_down() to prevent confusions

Browse Source 
snr_set_srv_down() (was formely known as snr_update_srv_status()), is
still too ambiguous because it's not clear whether we will be putting
the server under maintenance or not. This is mainly due to the fact that
the function behaves differently if has_no_ip is set or not.

By reviewing the function callers, it has now become clear that
snr_resolution_cb() is always calling the function with a valid resolution
so we only want to put the server under maintenance if we don't have a
valid IP address. On the other hand snr_resolution_error_cb() always
calls the function on error, with either no resolution (for SRV requests)
or with failing resolution (all cases except RSLV_STATUS_VALID), so in
this case we decide whether to put the server under maintenance case by
case (ie: expired? timeout?)

As a result, let's simplify snr_set_srv_down() so that it is only called
when the caller really thinks that the server should be put under
maintenance, which means always for snr_resolution_error_cb(), and only
if the resolution didn't yield usable ip for snr_resolution_cb().
This commit is contained in:
Aurelien DARRAGON 2023-12-22 14:17:41 +01:00
parent 689784ed91
commit bdecff511c
1 changed files with 19 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

39
src/server.c
View File

@ -4267,43 +4267,39 @@ int srvrq_set_srv_down(struct server *s)
* *
* Must be called with the server lock held. * Must be called with the server lock held.
*/ */
int snr_set_srv_down(struct server *s, int has_no_ip) int snr_set_srv_down(struct server *s)
{ {
struct resolvers *resolvers = s->resolvers; struct resolvers *resolvers = s->resolvers;
struct resolv_resolution *resolution = (s->resolv_requester ? s->resolv_requester->resolution : NULL); struct resolv_resolution *resolution = (s->resolv_requester ? s->resolv_requester->resolution : NULL);
int exp; int exp;
/* server already under maintenance */
if (s->next_admin & SRV_ADMF_RMAINT)
goto out;
/* If resolution is NULL we're dealing with SRV records Additional records */ /* If resolution is NULL we're dealing with SRV records Additional records */
if (resolution == NULL && has_no_ip) if (resolution == NULL)
return srvrq_set_srv_down(s); return srvrq_set_srv_down(s);
switch (resolution->status) { switch (resolution->status) {
case RSLV_STATUS_NONE: case RSLV_STATUS_NONE:
/* status when HAProxy has just (re)started. /* status when HAProxy has just (re)started.
* Nothing to do, since the task is already automatically started */ * Nothing to do, since the task is already automatically started */
break; goto out;
case RSLV_STATUS_VALID: case RSLV_STATUS_VALID:
if (has_no_ip) {
/* /*
* valid resolution but no usable server address * valid resolution but no usable server address
*/ */
if (s->next_admin & SRV_ADMF_RMAINT)
return 1;
srv_set_admin_flag(s, SRV_ADMF_RMAINT, SRV_ADM_STCHGC_DNS_NOIP); srv_set_admin_flag(s, SRV_ADMF_RMAINT, SRV_ADM_STCHGC_DNS_NOIP);
return 0; return 0;
}
return 0;
case RSLV_STATUS_NX: case RSLV_STATUS_NX:
/* stop server if resolution is NX for a long enough period */ /* stop server if resolution is NX for a long enough period */
exp = tick_add(resolution->last_valid, resolvers->hold.nx); exp = tick_add(resolution->last_valid, resolvers->hold.nx);
if (!tick_is_expired(exp, now_ms)) if (!tick_is_expired(exp, now_ms))
break; goto out; // not yet expired
if (s->next_admin & SRV_ADMF_RMAINT)
return 1;
srv_set_admin_flag(s, SRV_ADMF_RMAINT, SRV_ADM_STCHGC_DNS_NX); srv_set_admin_flag(s, SRV_ADMF_RMAINT, SRV_ADM_STCHGC_DNS_NX);
return 0; return 0;
@ -4311,10 +4307,8 @@ int snr_set_srv_down(struct server *s, int has_no_ip)
/* stop server if resolution is TIMEOUT for a long enough period */ /* stop server if resolution is TIMEOUT for a long enough period */
exp = tick_add(resolution->last_valid, resolvers->hold.timeout); exp = tick_add(resolution->last_valid, resolvers->hold.timeout);
if (!tick_is_expired(exp, now_ms)) if (!tick_is_expired(exp, now_ms))
break; goto out; // not yet expired
if (s->next_admin & SRV_ADMF_RMAINT)
return 1;
srv_set_admin_flag(s, SRV_ADMF_RMAINT, SRV_ADM_STCHGC_DNS_TIMEOUT); srv_set_admin_flag(s, SRV_ADMF_RMAINT, SRV_ADM_STCHGC_DNS_TIMEOUT);
return 0; return 0;
@ -4322,10 +4316,8 @@ int snr_set_srv_down(struct server *s, int has_no_ip)
/* stop server if resolution is REFUSED for a long enough period */ /* stop server if resolution is REFUSED for a long enough period */
exp = tick_add(resolution->last_valid, resolvers->hold.refused); exp = tick_add(resolution->last_valid, resolvers->hold.refused);
if (!tick_is_expired(exp, now_ms)) if (!tick_is_expired(exp, now_ms))
break; goto out; // not yet expired
if (s->next_admin & SRV_ADMF_RMAINT)
return 1;
srv_set_admin_flag(s, SRV_ADMF_RMAINT, SRV_ADM_STCHGC_DNS_REFUSED); srv_set_admin_flag(s, SRV_ADMF_RMAINT, SRV_ADM_STCHGC_DNS_REFUSED);
return 0; return 0;
@ -4333,14 +4325,13 @@ int snr_set_srv_down(struct server *s, int has_no_ip)
/* stop server if resolution failed for a long enough period */ /* stop server if resolution failed for a long enough period */
exp = tick_add(resolution->last_valid, resolvers->hold.other); exp = tick_add(resolution->last_valid, resolvers->hold.other);
if (!tick_is_expired(exp, now_ms)) if (!tick_is_expired(exp, now_ms))
break; goto out; // not yet expired
if (s->next_admin & SRV_ADMF_RMAINT)
return 1;
srv_set_admin_flag(s, SRV_ADMF_RMAINT, SRV_ADM_STCHGC_DNS_UNSPEC); srv_set_admin_flag(s, SRV_ADMF_RMAINT, SRV_ADM_STCHGC_DNS_UNSPEC);
return 0; return 0;
} }
out:
return 1; return 1;
} }
@ -4441,7 +4432,7 @@ int snr_resolution_cb(struct resolv_requester *requester, struct dns_counters *c
srv_update_addr(s, firstip, firstip_sin_family, SERVER_INETADDR_UPDATER_DNS_CACHE); srv_update_addr(s, firstip, firstip_sin_family, SERVER_INETADDR_UPDATER_DNS_CACHE);
update_status: update_status:
if (!snr_set_srv_down(s, has_no_ip) && has_no_ip) { if (has_no_ip && !snr_set_srv_down(s)) {
struct server_inetaddr s_addr; struct server_inetaddr s_addr;
memset(&s_addr, 0, sizeof(s_addr)); memset(&s_addr, 0, sizeof(s_addr));
@ -4455,7 +4446,7 @@ int snr_resolution_cb(struct resolv_requester *requester, struct dns_counters *c
counters->app.resolver.invalid++; counters->app.resolver.invalid++;
goto update_status; goto update_status;
} }
if (!snr_set_srv_down(s, has_no_ip) && has_no_ip) { if (has_no_ip && !snr_set_srv_down(s)) {
struct server_inetaddr s_addr; struct server_inetaddr s_addr;
memset(&s_addr, 0, sizeof(s_addr)); memset(&s_addr, 0, sizeof(s_addr));
@ -4541,7 +4532,7 @@ int snr_resolution_error_cb(struct resolv_requester *requester, int error_code)
return 0; return 0;
HA_SPIN_LOCK(SERVER_LOCK, &s->lock); HA_SPIN_LOCK(SERVER_LOCK, &s->lock);
if (!snr_set_srv_down(s, 1)) { if (!snr_set_srv_down(s)) {
struct server_inetaddr s_addr; struct server_inetaddr s_addr;
memset(&s_addr, 0, sizeof(s_addr)); memset(&s_addr, 0, sizeof(s_addr));