BUG/MINOR: sink: add tempo between 2 connection attempts for sft servers
authorAurelien DARRAGON <adarragon@haproxy.com>
Fri, 21 Feb 2025 09:51:01 +0000 (10:51 +0100)
committerAmaury Denoyelle <adenoyelle@haproxy.com>
Tue, 18 Mar 2025 15:10:31 +0000 (16:10 +0100)
When the connection for sink_forward_{oc}_applet fails or a previous one
is destroyed, the sft->appctx is instantly released.

However process_sink_forward_task(), which may run at any time, iterates
over all known sfts and tries to create sessions for orphan ones.

It means that instantly after sft->appctx is destroyed, a new one will
be created, thus a new connection attempt will be made.

It can be an issue with tcp log-servers or sink servers, because if the
server is unavailable, process_sink_forward() will keep looping without
any temporisation until the applet survives (ie: connection succeeds),
which results in unexpected CPU usage on the threads responsible for
that task.

Instead, we add a tempo logic so that a delay of 1second is applied
between two retries. Of course the initial attempt is not delayed.

This could be backported to all stable versions.

(cherry picked from commit 9561b9fb6964af325a10e7128b563114f144a3cb)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 0164f13cb144fff527b970a6f19175ccd627980c)
[ad: ctx adjustement due to missing commit 09d69eac]
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>

include/haproxy/sink-t.h
src/sink.c

index d5e1cec..3c9ad60 100644 (file)
@@ -39,6 +39,7 @@ enum sink_type {
 struct sink_forward_target {
        struct server *srv;    // used server
        struct appctx *appctx; // appctx of current session
+       uint last_conn;        // copy of now_ms for last session establishment attempt
        size_t ofs;            // ring buffer reader offset
        struct sink *sink;     // the associated sink
        struct sink_forward_target *next;
index 1cae047..20be8ca 100644 (file)
@@ -573,6 +573,7 @@ static struct appctx *sink_forward_session_create(struct sink *sink, struct sink
        if (appctx_init(appctx) == -1)
                goto out_free_appctx;
 
+       sft->last_conn = TICK_ETERNITY;
        return appctx;
 
        /* Error unrolling */
@@ -595,9 +596,21 @@ static struct task *process_sink_forward(struct task * task, void *context, unsi
        if (!stopping) {
                while (sft) {
                        HA_SPIN_LOCK(SFT_LOCK, &sft->lock);
-                       /* if appctx is NULL, start a new session */
-                       if (!sft->appctx)
-                               sft->appctx = sink_forward_session_create(sink, sft);
+                       /* if appctx is NULL, start a new session
+                        *
+                        * We enforce a tempo to ensure we don't perform more than 1 session
+                        * establishment attempt per second.
+                        */
+                       if (!sft->appctx) {
+                               uint tempo = sft->last_conn + MS_TO_TICKS(1000);
+
+                               if (sft->last_conn == TICK_ETERNITY || tick_is_expired(tempo, now_ms))
+                                       sft->appctx = sink_forward_session_create(sink, sft);
+                               else if (task->expire == TICK_ETERNITY)
+                                       task->expire = tempo;
+                               else
+                                       task->expire = tick_first(task->expire, tempo);
+                       }
                        HA_SPIN_UNLOCK(SFT_LOCK, &sft->lock);
                        sft = sft->next;
                }