From 0a52a75ef7151a187b42cae6d49a75b79380f211 Mon Sep 17 00:00:00 2001 From: Christopher Faulet Date: Mon, 27 Jan 2025 16:17:27 +0100 Subject: [PATCH] BUG/MINOR: stream: Properly handle "on-marked-up shutdown-backup-sessions" shutdown-backup-sessions action for on-marked-up directive does not work anymore since the stream_shutdown() function was modified to be async-safe. When stream_shutdown() was modified to be async-safe, dedicated task events were added to map the reasons to shut a stream down. SF_ERR_DOWN was mapped to TASK_F_EVT1 and SF_ERR_KILLED was mapped to TASK_F_EVT2. The reverse mapping was performed by process_stream() to shut the stream with the appropriate reason. However, SF_ERR_UP reason, used by shutdown-backup-sessions action to shut a stream down because a preferred server became available, was not mapped in the same way. So since commit b8e3b0a18d ("BUG/MEDIUM: stream: make stream_shutdown() async-safe"), this action is ignored and does not work anymore. To fix an issue, and being able to bakcport the fix, a third task event was added. TASK_F_EVT3 is now mapped on SF_ERR_UP. This patch should fix the issue #2848. It must be backported as far as 2.6. --- doc/internals/api/scheduler.txt | 3 +++ include/haproxy/stream.h | 8 +++++--- include/haproxy/task-t.h | 3 ++- src/stream.c | 5 +++-- 4 files changed, 13 insertions(+), 6 deletions(-) diff --git a/doc/internals/api/scheduler.txt b/doc/internals/api/scheduler.txt index 4a1540df5..6598d16b1 100644 --- a/doc/internals/api/scheduler.txt +++ b/doc/internals/api/scheduler.txt @@ -221,6 +221,9 @@ state field before the call to ->process() - TASK_F_UEVT2 one-shot user-defined event type 2. This is application specific, and reset to 0 when the handler is called. + - TASK_F_UEVT3 one-shot user-defined event type 3. This is application + specific, and reset to 0 when the handler is called. + In addition, a few persistent flags may be observed or manipulated by the application, both for tasks and tasklets: diff --git a/include/haproxy/stream.h b/include/haproxy/stream.h index 308fb6904..a14a4b17a 100644 --- a/include/haproxy/stream.h +++ b/include/haproxy/stream.h @@ -389,15 +389,17 @@ static inline int stream_check_conn_timeout(struct stream *s) * locked one way or another so that it cannot leave (i.e. when inspecting * a locked list or under thread isolation). Process_stream() will recognize * the message and complete the job. only supports SF_ERR_DOWN (mapped - * to UEVT1) and SF_ERR_KILLED (mapped to UEVT2). Other values will just - * trigger TASK_WOKEN_OTHER. The stream handler will first call function - * stream_shutdown_self() on wakeup to complete the notification. + * to UEVT1), SF_ERR_KILLED (mapped to UEVT2) and SF_ERR_UP (mapped to UEVT3). + * Other values will just trigger TASK_WOKEN_OTHER. + * The stream handler will first call function stream_shutdown_self() on wakeup + * to complete the notification. */ static inline void stream_shutdown(struct stream *s, int why) { task_wakeup(s->task, TASK_WOKEN_OTHER | ((why == SF_ERR_DOWN) ? TASK_F_UEVT1 : (why == SF_ERR_KILLED) ? TASK_F_UEVT2 : + (why == SF_ERR_UP) ? TASK_F_UEVT3 : 0)); } diff --git a/include/haproxy/task-t.h b/include/haproxy/task-t.h index 2a914c6b2..72f0d137f 100644 --- a/include/haproxy/task-t.h +++ b/include/haproxy/task-t.h @@ -61,7 +61,8 @@ #define TASK_F_UEVT1 0x00020000 /* one-shot user event type 1, application specific, def:0 */ #define TASK_F_UEVT2 0x00040000 /* one-shot user event type 2, application specific, def:0 */ #define TASK_F_WANTS_TIME 0x00080000 /* task/tasklet wants th_ctx->sched_call_date to be set */ -/* unused: 0x100000..0x80000000 */ +#define TASK_F_UEVT3 0x00100000 /* one-shot user event type 3, application specific, def:0 */ +/* unused: 0x200000..0x80000000 */ /* These flags are persistent across scheduler calls */ #define TASK_PERSISTENT (TASK_SELF_WAKING | TASK_KILLED | \ diff --git a/src/stream.c b/src/stream.c index 41a2ffcb9..0961ec9ad 100644 --- a/src/stream.c +++ b/src/stream.c @@ -1707,6 +1707,7 @@ void stream_update_timings(struct task *t, uint64_t lat, uint64_t cpu) * - TASK_WOKEN_MSG forces analysers to be re-evaluated * - TASK_WOKEN_OTHER+TASK_F_UEVT1 shuts the stream down on server down * - TASK_WOKEN_OTHER+TASK_F_UEVT2 shuts the stream down on active kill + * - TASK_WOKEN_OTHER+TASK_F_UEVT3 shuts the stream down because a preferred backend became available * - TASK_WOKEN_OTHER alone has no effect */ struct task *process_stream(struct task *t, void *context, unsigned int state) @@ -1728,9 +1729,9 @@ struct task *process_stream(struct task *t, void *context, unsigned int state) activity[tid].stream_calls++; stream_cond_update_cpu_latency(s); - if ((state & TASK_WOKEN_OTHER) && (state & (TASK_F_UEVT1 | TASK_F_UEVT2))) { + if ((state & TASK_WOKEN_OTHER) && (state & (TASK_F_UEVT1 | TASK_F_UEVT2 | TASK_F_UEVT3))) { /* that an instant kill message, the reason is in _UEVT* */ - stream_shutdown_self(s, (state & TASK_F_UEVT2) ? SF_ERR_KILLED : SF_ERR_DOWN); + stream_shutdown_self(s, (state & TASK_F_UEVT3) ? SF_ERR_UP : (state & TASK_F_UEVT2) ? SF_ERR_KILLED : SF_ERR_DOWN); } req = &s->req;