]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
BUG/MEDIUM: listener/proxy: fix listeners notify for proxy resume
authorAurelien DARRAGON <adarragon@haproxy.com>
Tue, 7 Feb 2023 11:36:27 +0000 (12:36 +0100)
committerChristopher Faulet <cfaulet@haproxy.com>
Thu, 23 Feb 2023 14:05:05 +0000 (15:05 +0100)
In 58651b42f ("MEDIUM: listener/proxy: make the listeners notify about
proxy pause/resume") we introduced the logic for pause/resume notify using
li_ready for pause and li_paused for resume.

Unfortunately, relying on li_paused for resume doesn't work reliably if we
resume a listener which is only made of receivers that are completely stopped.
For example, this could happen with receivers that don't support the
LI_PAUSED state like ABNS sockets.

This is especially true since pause_listener() was renamed to
suspend_listener() to better reflect its actual behavior in
("MINOR: listener:  pause_listener() becomes suspend_listener())

To fix this, we now rely on the li_suspended state in resume_listener() to make
sure that suspend_listener() and resume_listener() notify messages are
consistent to each other:

"Proxy pause" is triggered when there are no more ready listeners.
"Proxy resume" is triggered when there are no more suspended listeners.

Also, we make use of the new PR_FL_PAUSED proxy flag to make sure we don't
report the same event twice.

This could be backported up to 2.4 after a reasonable observation
period to make sure that this change doesn't cause unwanted side-effects.

--
Backport notes:

This commit depends on:
 - "MINOR: listener: pause_listener() becomes suspend_listener()"

-> 2.4 only, as "MINOR: proxy/listener: support for additional PAUSED state"
was not backported:

Replace this:

    |+       if (px && !(px->flags & PR_FL_PAUSED) && !px->li_ready) {
    |                /* PROXY_LOCK is required */
    |                proxy_cond_pause(px);
    |                ha_warning("Paused %s %s.\n", proxy_cap_str(px->cap), px->id);

By this:

    |+       if (px && !px->li_ready) {
    |                ha_warning("Paused %s %s.\n", proxy_cap_str(px->cap), px->id);
    |                send_log(px, LOG_WARNING, "Paused %s %s.\n", proxy_cap_str(px->cap), px->id);
    |        }

And this:

    |+       if (px && (px->flags & PR_FL_PAUSED) && !px->li_suspended) {
    |                /* PROXY_LOCK is required */
    |                proxy_cond_resume(px);
    |                ha_warning("Resumed %s %s.\n", proxy_cap_str(px->cap), px->id);

By this:

    |+       if (px && !px->li_suspended) {
    |                ha_warning("Resumed %s %s.\n", proxy_cap_str(px->cap), px->id);
    |                send_log(px, LOG_WARNING, "Resumed %s %s.\n", proxy_cap_str(px->cap), px->id);
    |        }

src/listener.c

index ca13a71968b3cb13541af3d4fc9c8928a441869a..47cf42634f7077ab8e8581cf5fd3e48ab884630b 100644 (file)
@@ -527,7 +527,7 @@ int suspend_listener(struct listener *l, int lpx, int lli)
         */
        ret = 1;
 
-       if (px && !px->li_ready) {
+       if (px && !(px->flags & PR_FL_PAUSED) && !px->li_ready) {
                /* PROXY_LOCK is required */
                proxy_cond_pause(px);
                ha_warning("Paused %s %s.\n", proxy_cap_str(px->cap), px->id);
@@ -560,7 +560,6 @@ int suspend_listener(struct listener *l, int lpx, int lli)
 int resume_listener(struct listener *l, int lpx, int lli)
 {
        struct proxy *px = l->bind_conf->frontend;
-       int was_paused = px && px->li_paused;
        int ret = 1;
 
        if (!lpx && px)
@@ -598,7 +597,7 @@ int resume_listener(struct listener *l, int lpx, int lli)
                px->li_suspended--;
        l->flags &= ~LI_F_SUSPENDED;
 
-       if (was_paused && !px->li_paused) {
+       if (px && (px->flags & PR_FL_PAUSED) && !px->li_suspended) {
                /* PROXY_LOCK is required */
                proxy_cond_resume(px);
                ha_warning("Resumed %s %s.\n", proxy_cap_str(px->cap), px->id);