Hello Willy,

> Unfortunately I couldn't get it to work even a single time :-( I tried
> hard to fiddle with barriers as well to try to improve the serialization
> but didn't manage to force it to proceed like we want. Pretty frustrating.
> What I tried was to make sure that c2 starts only once the two c1 were
> sent, and that the s1 server only starts to respond after c2 gets their
> response from s0. But for now I failed miserably :-(

I gave it another try along these lines, with a few simplifications. I think
it is still a good test for the feature, and at least it appears to work 100%
of the time 'on my machine' -- so I am hopeful in might on yours/CI as well.

I have dropped the second `c2b` client to simplify the test.

I initially also tried to serve the request for `c1b` (the one that is initially
queued) and check that it is successful, by using something along the lines of:

```
s1 {
  rxresp   # Serve request for c1a
  ...      # Barriers
  txresp

  rxresp   # Serve request for c1b
  txtesp
}
```

With `http-reuse always` in the haproxy config, this seemed to work... about 90%
of the time. In about 10% of cases, the second `s1 rxresp` would just hang and
the test would time out. Not sure why from the logs. To avoid those occurrences,
I now let the request from `c1b` fail silently, and this request only serves the
purpose of keeping the queue full for when `c2` comes in.

> If I don't manage to get them to work, I propose you to merge everything
> but tag the VTC as "broken" (we already have a few such) so that they
> don't run by default. It's too bad to delay the inclusion of a feature
> just because we're unable to express it in vtc :-(

That is fine by me as well. Thank you for taking a detailed look at these tests!


>From e359317ebe3184e904432fdbdb93487a9ec6a271 Mon Sep 17 00:00:00 2001
From: Pierre-Andre Savalle <psaval...@protonmail.com>
Date: Fri, 21 Mar 2025 11:27:21 +0100
Subject: [PATCH] MEDIUM: lb-chash: add directive hash-preserve-affinity

When using hash-based load balancing, requests are always assigned to the 
server corresponding
to the hash bucket for the balancing key, without taking maxconn or maxqueue 
into account, unlike
in other load balancing methods like 'first'. This adds a new backend directive 
that can be used
to take maxconn and possibly maxqueue in that context. This can be used when 
hashing is desired
to achieve cache locality, but sending requests to a different server is 
preferable to queuing
for a long time or failing requests when the initial server is saturated.

By default, affinity is preserved as was the case previously. When 
'hash-preserve-affinity' is
set to 'maxqueue', servers are considered successively in the order of the hash 
ring until a
server that does not have a full queue is found.

When 'maxconn' is set on a server, queueing cannot be disabled, as 'maxqueue=0' 
means unlimited.
To support picking a different server when a server is at 'maxconn' 
irrespective of the queue,
'hash-preserve-affinity' can be set to 'maxconn'.
---
 doc/configuration.txt                       | 33 +++++++-
 include/haproxy/proxy-t.h                   |  8 +-
 reg-tests/balance/balance-hash-maxconn.vtc  | 59 ++++++++++++++
 reg-tests/balance/balance-hash-maxqueue.vtc | 89 +++++++++++++++++++++
 src/lb_chash.c                              | 13 ++-
 src/proxy.c                                 | 32 ++++++++
 tests/conf/test-hash-preseve-affinity.cfg   | 52 ++++++++++++
 7 files changed, 282 insertions(+), 4 deletions(-)
 create mode 100644 reg-tests/balance/balance-hash-maxconn.vtc
 create mode 100644 reg-tests/balance/balance-hash-maxqueue.vtc
 create mode 100644 tests/conf/test-hash-preseve-affinity.cfg

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 8eb8db06f..132d32b2d 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -5969,6 +5969,7 @@ filter                                    -          X    
     X         X
 fullconn                                  X          -         X         X
 guid                                      -          X         X         X
 hash-balance-factor                       X          -         X         X
+hash-preserve-affinity                    X          -         X         X
 hash-type                                 X          -         X         X
 http-after-response                       X (!)      X         X         X
 http-check comment                        X          -         X         X
@@ -7857,6 +7858,35 @@ hash-balance-factor <factor>
 
   See also : "balance" and "hash-type".
 
+hash-preserve-affinity { always | maxconn | maxqueue }
+  Specify a method for assigning streams to servers with hash load balancing 
when
+  servers are satured or have a full queue.
+
+  May be used in the following contexts: http
+
+  May be used in sections:   defaults | frontend | listen | backend
+                               yes    |    no    |   yes  |   yes
+
+  The following values can be specified:
+
+    - "always"   : this is the default stategy. A stream is assigned to a 
server
+                   based on hashing irrespective of whether the server is 
currently
+                   saturated.
+
+    - "maxconn"  : when selected, servers that have "maxconn" set and are 
currently
+                   saturated will be skipped. Another server will be picked by
+                   following the hashing ring. This has no effect on servers 
that do
+                   not set "maxconn". If all servers are saturated, the 
request is
+                   enqueued to the last server in the hash ring before the 
initially
+                   selected server.
+
+    - "maxqueue" : when selected, servers that have "maxconn" set, "maxqueue" 
set
+                   to a non-zero value (limited queue size) and currently have 
a
+                   full queue will be skipped. Another server will be picked by
+                   following the hashing ring. This has no effect on servers 
that
+                   do not set both "maxconn" and "maxqueue".
+
+  See also : "maxconn", "maxqueue", "hash-balance-factor"
 
 hash-type <method> <function> <modifier>
   Specify a method to use for mapping hashes to servers
@@ -7950,8 +7980,7 @@ hash-type <method> <function> <modifier>
   default function is "sdbm", the selection of a function should be based on
   the range of the values being hashed.
 
-  See also : "balance", "hash-balance-factor", "server"
-
+  See also : "balance", "hash-balance-factor", "hash-preserve-affinity", 
"server"
 
 http-after-response <action> <options...> [ { if | unless } <condition> ]
   Access control for all Layer 7 responses (server, applet/service and internal
diff --git a/include/haproxy/proxy-t.h b/include/haproxy/proxy-t.h
index 9762ab166..5ad420129 100644
--- a/include/haproxy/proxy-t.h
+++ b/include/haproxy/proxy-t.h
@@ -180,7 +180,13 @@ enum PR_SRV_STATE_FILE {
 #define PR_O3_LOGF_HOST_APPEND   0x00000080
 #define PR_O3_LOGF_HOST          0x000000F0
 
-/* unused: 0x00000100 to  0x80000000 */
+/* bits for hash-preserve-affinity */
+#define PR_O3_HASHAFNTY_ALWS     0x00000000 /* always preserve hash affinity */
+#define PR_O3_HASHAFNTY_MAXCONN  0x00000100 /* preserve hash affinity until 
maxconn is reached */
+#define PR_O3_HASHAFNTY_MAXQUEUE 0x00000200 /* preserve hash affinity until 
maxqueue is reached */
+#define PR_O3_HASHAFNTY_MASK     0x00000300 /* mask for hash-preserve-affinity 
*/
+
+/* unused: 0x00000400 to  0x80000000 */
 /* end of proxy->options3 */
 
 /* Cookie settings for pr->ck_opts */
diff --git a/reg-tests/balance/balance-hash-maxconn.vtc 
b/reg-tests/balance/balance-hash-maxconn.vtc
new file mode 100644
index 000000000..2edd797c0
--- /dev/null
+++ b/reg-tests/balance/balance-hash-maxconn.vtc
@@ -0,0 +1,59 @@
+vtest "Test for balance URI with hash-preserve-affinity maxconn"
+feature ignore_unknown_macro
+
+# Ensure c1 doesn't finish before c2
+barrier b1 cond 2
+
+# Ensure c2 only starts once c1's request is already in flight
+barrier b2 cond 2
+
+server s0 {
+    rxreq
+    barrier b1 sync
+    txresp -hdr "Server: s0"
+} -start
+
+server s1 {
+    rxreq
+    barrier b2 sync
+    barrier b1 sync
+    txresp -hdr "Server: s1"
+} -start
+
+haproxy h1 -arg "-L A" -conf {
+    defaults
+        mode http
+        timeout server "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout connect "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout client "${HAPROXY_TEST_TIMEOUT-5s}"
+
+    listen px
+        bind "fd@${px}"
+        balance uri
+        hash-preserve-affinity maxconn
+        hash-type consistent
+
+        server srv0 ${s0_addr}:${s0_port} maxconn 1
+        server srv1 ${s1_addr}:${s1_port} maxconn 1
+
+} -start
+
+client c1 -connect ${h1_px_sock} {
+    txreq -url "/test-url"
+    rxresp
+    expect resp.status == 200
+    expect resp.http.Server ~ s1
+}  -start
+
+barrier b2 sync
+
+# s1 is saturated, request should be assigned to s0
+client c2 -connect ${h1_px_sock} {
+    txreq -url "/test-url"
+    rxresp
+    expect resp.status == 200
+    expect resp.http.Server ~ s0
+} -start
+
+client c1 -wait
+client c2 -wait
diff --git a/reg-tests/balance/balance-hash-maxqueue.vtc 
b/reg-tests/balance/balance-hash-maxqueue.vtc
new file mode 100644
index 000000000..7bbcaef20
--- /dev/null
+++ b/reg-tests/balance/balance-hash-maxqueue.vtc
@@ -0,0 +1,89 @@
+
+vtest "Test for balance URI with hash-preserve-affinity maxqueue"
+feature ignore_unknown_macro
+
+# The test proceeds as follows:
+#
+# - `c1a` sends a request, which should be routed to `s1`.
+#
+# - Once `s1` receives the request, we unblock `b_s1_has_rxed_c1a`, which 
allows `c1b` to send
+#   a request, which should also be routed to `s1`. Since `s1` is saturated, 
the request from
+#   `c1b` is put in the queue for `s1`.
+#
+# - After the request from `c1b` has been transmitted, we unblock 
`b_has_txed_c1b`, which allows
+#   `c2` to send a request. Since `s1` is at maxconn and maxqueue, it should 
be sent to `s0` and
+#   complete right away.
+#
+# - Once the request from `c2` has been served successfully from `s0`, we 
unblock `b_c2_is_done`
+#   which allows `s1` to serve the requests from `c1a` and `c1b`.
+
+barrier b_s1_has_rxed_c1a cond 2
+barrier b_has_txed_c1b cond 2
+barrier b_c2_is_done cond 2
+barrier b_c1_is_done cond 3
+
+server s0 {
+    rxreq
+    txresp
+} -start
+
+server s1 {
+    rxreq
+
+    # Indicates that c1a's request has been received
+    barrier b_s1_has_rxed_c1a sync
+
+    # Wait until c2 is done
+    barrier b_c2_is_done sync
+
+    txresp
+} -start
+
+haproxy h1 -arg "-L A" -conf {
+    defaults
+        mode http
+        timeout server "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout connect "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout client "${HAPROXY_TEST_TIMEOUT-5s}"
+
+    listen px
+        bind "fd@${px}"
+        balance uri
+        hash-preserve-affinity maxqueue
+        hash-type consistent
+
+        http-response set-header Server %s
+
+        server s0 ${s0_addr}:${s0_port} maxconn 1
+        server s1 ${s1_addr}:${s1_port} maxconn 1 maxqueue 1
+} -start
+
+# c1a sends a request, it should go to s1 and wait
+client c1a -connect ${h1_px_sock} {
+    txreq -url "/test-url"
+    rxresp
+    expect resp.status == 200
+    expect resp.http.Server ~ s1
+}  -start
+
+barrier b_s1_has_rxed_c1a sync
+
+# c1b sends a request, it should go to s1 and wait in queue
+client c1b -connect ${h1_px_sock} {
+    txreq -url "/test-url"
+    barrier b_has_txed_c1b sync
+    rxresp
+}  -start
+
+barrier b_has_txed_c1b sync
+
+# s1 is saturated, requests should be assigned to s0
+client c2 -connect ${h1_px_sock} {
+    txreq -url "/test-url"
+    rxresp
+    barrier b_c2_is_done sync
+    expect resp.status == 200
+    expect resp.http.Server ~ s0
+} -run
+
+client c1a -wait
\ No newline at end of file
diff --git a/src/lb_chash.c b/src/lb_chash.c
index 784a27af1..f5b075ab3 100644
--- a/src/lb_chash.c
+++ b/src/lb_chash.c
@@ -404,6 +404,7 @@ struct server *chash_get_server_hash(struct proxy *p, 
unsigned int hash, const s
        struct eb_root *root;
        unsigned int dn, dp;
        int loop;
+       int hashafnty;
 
        HA_RWLOCK_RDLOCK(LBPRM_LOCK, &p->lbprm.lock);
 
@@ -449,7 +450,17 @@ struct server *chash_get_server_hash(struct proxy *p, 
unsigned int hash, const s
        }
 
        loop = 0;
-       while (nsrv == avoid || (p->lbprm.hash_balance_factor && 
!chash_server_is_eligible(nsrv))) {
+       hashafnty = p->options3 & PR_O3_HASHAFNTY_MASK;
+
+       while (nsrv == avoid ||
+                       (p->lbprm.hash_balance_factor && 
!chash_server_is_eligible(nsrv)) ||
+                       (hashafnty == PR_O3_HASHAFNTY_MAXCONN &&
+                               nsrv->maxconn &&
+                               nsrv->served >= srv_dynamic_maxconn(nsrv)) ||
+                       (hashafnty == PR_O3_HASHAFNTY_MAXQUEUE &&
+                               nsrv->maxconn &&
+                               nsrv->maxqueue &&
+                               nsrv->served + nsrv->queueslength >= 
srv_dynamic_maxconn(nsrv) + nsrv->maxqueue)) {
                next = eb32_next(next);
                if (!next) {
                        next = eb32_first(root);
diff --git a/src/proxy.c b/src/proxy.c
index 3725d364a..b7b6bf192 100644
--- a/src/proxy.c
+++ b/src/proxy.c
@@ -927,6 +927,37 @@ proxy_parse_retry_on(char **args, int section, struct 
proxy *curpx,
        return 0;
 }
 
+/* This function parses a "hash-preserve-affinity" statement */
+static int
+proxy_parse_hash_preserve_affinity(char **args, int section, struct proxy 
*curpx,
+                                                                       const 
struct proxy *defpx, const char *file, int line,
+                                                                       char 
**err)
+{
+       if (!(*args[1])) {
+               memprintf(err, "'%s' needs a keyword to specify when to 
preserve hash affinity", args[0]);
+               return -1;
+       }
+       if (!(curpx->cap & PR_CAP_BE)) {
+               memprintf(err, "'%s' only available in backend or listen 
section", args[0]);
+               return -1;
+       }
+
+       curpx->options3 &= ~PR_O3_HASHAFNTY_MASK;
+
+       if (strcmp(args[1], "always") == 0)
+               curpx->options3 |= PR_O3_HASHAFNTY_ALWS;
+       else if (strcmp(args[1], "maxconn") == 0)
+               curpx->options3 |= PR_O3_HASHAFNTY_MAXCONN;
+       else if (strcmp(args[1], "maxqueue") == 0)
+               curpx->options3 |= PR_O3_HASHAFNTY_MAXQUEUE;
+       else {
+               memprintf(err, "'%s': unknown keyword '%s'", args[0], args[1]);
+               return -1;
+       }
+
+       return 0;
+}
+
 #ifdef TCP_KEEPCNT
 /* This function parses "{cli|srv}tcpka-cnt" statements */
 static int proxy_parse_tcpka_cnt(char **args, int section, struct proxy *proxy,
@@ -2698,6 +2729,7 @@ static struct cfg_kw_list cfg_kws = {ILH, {
        { CFG_LISTEN, "max-keep-alive-queue", proxy_parse_max_ka_queue },
        { CFG_LISTEN, "declare", proxy_parse_declare },
        { CFG_LISTEN, "retry-on", proxy_parse_retry_on },
+       { CFG_LISTEN, "hash-preserve-affinity", 
proxy_parse_hash_preserve_affinity },
 #ifdef TCP_KEEPCNT
        { CFG_LISTEN, "clitcpka-cnt", proxy_parse_tcpka_cnt },
        { CFG_LISTEN, "srvtcpka-cnt", proxy_parse_tcpka_cnt },
diff --git a/tests/conf/test-hash-preseve-affinity.cfg 
b/tests/conf/test-hash-preseve-affinity.cfg
new file mode 100644
index 000000000..1aa0e2c80
--- /dev/null
+++ b/tests/conf/test-hash-preseve-affinity.cfg
@@ -0,0 +1,52 @@
+# This is a test configuration for "hash-preserve-affinity" parameter
+global
+       log             127.0.0.1 local0
+
+defaults
+       mode http
+       timeout client 10s
+       timeout server 10s
+       timeout connect 10s
+
+listen  vip1
+       log             global
+       option          httplog
+       bind            :8001
+       mode            http
+       maxconn         100
+       balance         url_param foo
+       server          srv1 127.0.0.1:80
+       server          srv2 127.0.0.1:80
+
+listen  vip2
+       log             global
+       option          httplog
+       bind            :8002
+       mode            http
+       maxconn         100
+       balance         url_param foo check_post
+       server          srv1 127.0.0.1:80
+       server          srv2 127.0.0.1:80
+       hash-preserve-affinity always
+
+listen  vip3
+       log             global
+       option          httplog
+       bind            :8003
+       mode            http
+       maxconn         100
+       balance         url_param foo check_post
+       server          srv1 127.0.0.1:80
+       server          srv2 127.0.0.1:80
+       hash-preserve-affinity maxconn
+
+listen  vip4
+       log             global
+       option          httplog
+       bind            :8004
+       mode            http
+       maxconn         100
+       balance         url_param foo check_post
+       server          srv1 127.0.0.1:80
+       server          srv2 127.0.0.1:80
+       hash-preserve-affinity maxqueue
-- 
2.39.5 (Apple Git-154)




Reply via email to