Hello!
On Fri, Mar 21, 2025 at 11:05:13AM +0000, psavalle wrote:
> Hello everyone,
>
> This patch implements a new backend directive to control hash-based load
> balancing when servers are at the 'maxconn' limit or have a full queue. See
> https://github.com/haproxy/haproxy/issues/2893 for context.
>
> Thank you!
Thank you, pretty good work here! I have two requests below:
> From e518ee79a56319c9781e62005da13f2c0064399b Mon Sep 17 00:00:00 2001
> From: psavalle <psaval...@protonmail.com>
Here, we'll need to have something that looks like a real person
name. If for whatever reason you don't want to leave your real
identity, maybe have someone accept to post the patch for you.
> --- a/src/cfgparse-listen.c
> +++ b/src/cfgparse-listen.c
> @@ -2533,6 +2533,34 @@ int cfg_parse_listen(const char *file, int linenum,
> char **args, int kwm)
> goto out;
> }
> }
> + else if (strcmp(args[0], "hash-preserve-affinity") == 0) {
We're trying to get rid of all those strcmp() for first-level keywords
because they prevent the keywords from being enumerated. Instead it's
possible to write the same code in a small parsing function that's
registered for the keyword. Could you please have a look at the
"retry-on" keyword which is parsed by proxy_parse_retry_on(), which
does essentially the same ? You'll have to reference it in the same
file in the cfg_kws[] array (just search for proxy_parse_retry_on,
you'll find it).
Thanks to this, you can then verify using "haproxy -dKcfg -f /dev/null"
that your new keyword is listed. This is useful for people who implement
parsers, APIs etc to detect new keywords addition.
No other comment about this, your code looks clean and the doc and tests
look fine, so I'm definitely willing to merge it once this is addressed.
Thank you!
Willy
