BUG/MEDIUM: server: update our local state before propagating changes
Commit 3ff577e ("MAJOR: server: make server state changes synchronous again")
reintroduced synchronous server state changes. However, during the previous
change from synchronous to asynchronous, the server state propagation was
placed at the end of the function to ease the code changes, and the commit
above didn't put it back at its place. This has resulted in propagated
states to be incomplete. For example, making a server leave maintenance
would make it up but would leave its tracking servers down because they
see their tracked server is still down.
Let's just move the status update right to its place. It also adds the
benefit of reporting state changes in the order they appear and not in
reverse.
No backport is needed.
diff --git a/src/server.c b/src/server.c
index 1d7a5a7..ef8deb6 100644
--- a/src/server.c
+++ b/src/server.c
@@ -950,16 +950,16 @@
s->op_st_chg.duration = check->duration;
}
+ /* propagate changes */
+ thread_isolate();
+ srv_update_status(s);
+ thread_release();
+
for (srv = s->trackers; srv; srv = srv->tracknext) {
HA_SPIN_LOCK(SERVER_LOCK, &srv->lock);
srv_set_stopped(srv, NULL, NULL);
HA_SPIN_UNLOCK(SERVER_LOCK, &srv->lock);
}
-
- /* propagate changes */
- thread_isolate();
- srv_update_status(s);
- thread_release();
}
/* Marks server <s> up regardless of its checks' statuses and provided it isn't
@@ -995,16 +995,16 @@
if (s->slowstart <= 0)
s->next_state = SRV_ST_RUNNING;
+ /* propagate changes */
+ thread_isolate();
+ srv_update_status(s);
+ thread_release();
+
for (srv = s->trackers; srv; srv = srv->tracknext) {
HA_SPIN_LOCK(SERVER_LOCK, &srv->lock);
srv_set_running(srv, NULL, NULL);
HA_SPIN_UNLOCK(SERVER_LOCK, &srv->lock);
}
-
- /* propagate changes */
- thread_isolate();
- srv_update_status(s);
- thread_release();
}
/* Marks server <s> stopping regardless of its checks' statuses and provided it
@@ -1039,16 +1039,16 @@
s->op_st_chg.duration = check->duration;
}
+ /* propagate changes */
+ thread_isolate();
+ srv_update_status(s);
+ thread_release();
+
for (srv = s->trackers; srv; srv = srv->tracknext) {
HA_SPIN_LOCK(SERVER_LOCK, &srv->lock);
srv_set_stopping(srv, NULL, NULL);
HA_SPIN_UNLOCK(SERVER_LOCK, &srv->lock);
}
-
- /* propagate changes */
- thread_isolate();
- srv_update_status(s);
- thread_release();
}
/* Enables admin flag <mode> (among SRV_ADMF_*) on server <s>. This is used to
@@ -1074,10 +1074,15 @@
if (cause)
strlcpy2(s->adm_st_chg_cause, cause, sizeof(s->adm_st_chg_cause));
+ /* propagate changes */
+ thread_isolate();
+ srv_update_status(s);
+ thread_release();
+
/* stop going down if the equivalent flag was already present (forced or inherited) */
if (((mode & SRV_ADMF_MAINT) && (s->next_admin & ~mode & SRV_ADMF_MAINT)) ||
((mode & SRV_ADMF_DRAIN) && (s->next_admin & ~mode & SRV_ADMF_DRAIN)))
- goto end;
+ return;
/* compute the inherited flag to propagate */
if (mode & SRV_ADMF_MAINT)
@@ -1090,12 +1095,6 @@
srv_set_admin_flag(srv, mode, cause);
HA_SPIN_UNLOCK(SERVER_LOCK, &srv->lock);
}
-
- end:
- /* propagate changes */
- thread_isolate();
- srv_update_status(s);
- thread_release();
}
/* Disables admin flag <mode> (among SRV_ADMF_*) on server <s>. This is used to
@@ -1117,10 +1116,15 @@
s->next_admin &= ~mode;
+ /* propagate changes */
+ thread_isolate();
+ srv_update_status(s);
+ thread_release();
+
/* stop going down if the equivalent flag is still present (forced or inherited) */
if (((mode & SRV_ADMF_MAINT) && (s->next_admin & SRV_ADMF_MAINT)) ||
((mode & SRV_ADMF_DRAIN) && (s->next_admin & SRV_ADMF_DRAIN)))
- goto end;
+ return;
if (mode & SRV_ADMF_MAINT)
mode = SRV_ADMF_IMAINT;
@@ -1132,12 +1136,6 @@
srv_clr_admin_flag(srv, mode);
HA_SPIN_UNLOCK(SERVER_LOCK, &srv->lock);
}
-
- end:
- /* propagate changes */
- thread_isolate();
- srv_update_status(s);
- thread_release();
}
/* principle: propagate maint and drain to tracking servers. This is useful