Hi Aleks,
On Fri, Feb 06, 2026 at 08:48:06AM +0100, Aleksandar Lazic wrote:
Hi.
On 2026-02-06 (Fr.) 01:02, Aleksandar Lazic wrote:
Hi.
here my first attempt to add the (Peak) EWMA to HAProxy.
In the first post was the doc and reg-test missing this patches are now
added in that mail.
Regards
Aleks
From 7db40d8b38865f0bb065aae09b7f3f12d36e582c Mon Sep 17 00:00:00 2001
From: Aleksandar Lazic <[email protected]>
Date: Fri, 6 Feb 2026 08:45:32 +0100
Subject: [PATCH 10/10] MEDIUM/backend: Add Peak EWMA load balancing algorithm
refer to https://github.com/haproxy/haproxy/issues/1570
Signed-off-by: Aleksandar Lazic <[email protected]>
(...)
Well, it's impossible to review like this honestly, all commit messages
are exactly the same and there is zero info in them. The principles of
commit messages are:
- they must be descriptive of the patch itself: what does the patch
bring, what were the architectural choices and principles, why is
it considered a good approach, whether or not other approaches were
considered and evicted, what tradeoffs were made, whether it can
bring improvements as well as degradations in some specific cases,
etc. As you know, I'm used to saying that these are all the arguments
to "sell" the patch and ask someone to merge it and maintain it
forever.
- only include external links for complementary information (e.g. a
link to a paper). External links to figure the purpose of the
patch are definitely not acceptable because 1) you can't review
nor debug offline, and 2) you're at the mercy of the site hosting
the contents.
- subject lines *should* be unique in a project. Note that I'm not
saying "must" because accidents happen, but they should be
specific enough so that the risk of conflict remains almost zero.
The one above for example could be "DOC: config: document the new
peak-ewma load-balancing algorithm".
- when relevant, please also indicate the conditions in which this
was tested (and if possible what was observed, i.e. measures etc).
This is particularly important when someone starts to face issues
in uncommon environments and it's unknown whether this is getting
out of the initial scope.
I'll try to review the rest later (maybe start next week, but really,
without any intent in the commit messages to know what you're reviewing,
it's extremely difficult).
I'm seeing a few things below upon a quick glance though:
diff --git a/src/lb_pewma.c b/src/lb_pewma.c
new file mode 100644
index 0000000000..69ac79fbe9
--- /dev/null
+++ b/src/lb_pewma.c
@@ -0,0 +1,938 @@
(...)
+/* We reuse the same tree element layout as FWLC. The struct fwlc_tree_elt
+ * type is forward-declared in server-t.h via pointer members. We provide
+ * the full definition here so that we can access its fields.
+ */
+struct fwlc_tree_elt {
+ struct mt_list srv_list[PEWMA_LISTS_NB];
+ struct mt_list free_list;
+ struct eb32_node lb_node;
+ unsigned int elements;
+};
Please never ever redefine a struct somewhere else. It's granted to
break at the very first change that the other definition will experience.
Either you need it and you include the relevant include files, or you
define your own. Note that I strongly doubt that you'd need to reuse
the same though, and if it is really connection oriented, then it must
absolutely be tested under load, because leastconn used to perform
extremely badly in terms of CPU usage on modern systems, forcing us
to invest a lot of thinking into how to redesign it to strongly reduce
the amount of sharing between CPUs.
And forcefully sharing types between different things can just condemn
both algorithms by making it too hard to make one evolve without breaking
the other one.
@@ -58,6 +59,7 @@
/* BE_LB_CB_* is used with BE_LB_KIND_CB */
#define BE_LB_CB_LC 0x00000000 /* least-connections */
#define BE_LB_CB_FAS 0x00000001 /* first available server (opposite of
leastconn) */
+#define BE_LB_CB_PE 0x00000002 /* peak ewma */
/* BE_LB_SA_* is used with BE_LB_KIND_SA */
#define BE_LB_SA_SS 0x00000000 /* stick to server as long as it is
available */
@@ -88,6 +90,7 @@
#define BE_LB_ALGO_RND (BE_LB_KIND_RR | BE_LB_NEED_NONE | BE_LB_RR_RANDOM)
/* random value */
#define BE_LB_ALGO_LC (BE_LB_KIND_CB | BE_LB_NEED_NONE | BE_LB_CB_LC) /*
least connections */
#define BE_LB_ALGO_FAS (BE_LB_KIND_CB | BE_LB_NEED_NONE | BE_LB_CB_FAS) /*
first available server */
+#define BE_LB_ALGO_PE (BE_LB_KIND_CB | BE_LB_NEED_NONE | BE_LB_CB_PE) /*
peak ewma */
#define BE_LB_ALGO_SS (BE_LB_KIND_SA | BE_LB_NEED_NONE | BE_LB_SA_SS) /*
sticky */
#define BE_LB_ALGO_SRR (BE_LB_KIND_RR | BE_LB_NEED_NONE | BE_LB_RR_STATIC)
/* static round robin */
#define BE_LB_ALGO_SH (BE_LB_KIND_HI | BE_LB_NEED_ADDR | BE_LB_HASH_SRC) /*
hash: source IP */
(...)
So if it's in _CB_ it's connection-based (like leastconn thus highly
sensitive to CPU count). Otherwise if it's like random or round robin,
it'd rather be set as KIND_RR (round robin, i.e. no dependency on the
connection count).
Thanks,
Willy
