On Tue, Jul 20, 2021 at 12:18:18PM -0300, Joao Morais wrote:
> Subject: no-stop keyword proposal
>
> 
> Hello list, the diff below is a proposal to add a bind keyword used to
> flag LI_O_NOSTOP option in the bind’s listener.
> 
> Regarding the use case: I need the ability to reach a stopping, but
> still running haproxy instance to, at least:
> 1) fairly distribute
> shutdown sessions of long running connections (usually websockets)
> before hard-stop-after timeouts and kicks all the remaining
> connections at the same time[1]; 2) collect some relevant metrics from
> a stopping instance, e.g. current sessions and rps, which would be
> otherwise lost when these metrics are collected only from the current
> instance.
> 

This is already possible using the master CLI, it was done to access all
processes, old and new. It was designed to do that kind of things.


> Regarding the patch: it’s just the changes I needed to make and
> confirm that it works like I was expecting, provided that the
> listening socket is changed before reloading haproxy into a new
> instance. Please let me know if such improvement can be made and also
> if I’m in the right path.
> 
> ~jm
> 
> [1] https://www.mail-archive.com/haproxy@formilux.org/msg40916.html
> 
> diff --git a/src/cli.c b/src/cli.c
> index b3132191d..4285e5a72 100644
> --- a/src/cli.c
> +++ b/src/cli.c
> @@ -1841,6 +1841,19 @@ static int bind_parse_level(char **args, int cur_arg, 
> struct proxy *px, struct b
>       return 0;
>  }
> 
> +/* parse the "no-stop" bind keyword */
> +static int bind_parse_no_stop(char **args, int cur_arg, struct proxy *px, 
> struct bind_conf *conf, char **err)
> +{
> +     struct listener *l;
> +
> +     list_for_each_entry(l, &conf->listeners, by_bind) {
> +             l->options |= LI_O_NOSTOP;
> +             HA_ATOMIC_INC(&unstoppable_jobs);
> +     }
> +
> +     return 0;
> +}
> +
>  static int bind_parse_severity_output(char **args, int cur_arg, struct proxy 
> *px, struct bind_conf *conf, char **err)
>  {
>       if (!*args[cur_arg + 1]) {
> @@ -2984,6 +2997,7 @@ INITCALL1(STG_REGISTER, cfg_register_keywords, 
> &cfg_kws);
>  static struct bind_kw_list bind_kws = { "STAT", { }, {
>       { "level",     bind_parse_level,    1 }, /* set the unix socket admin 
> level */
>       { "expose-fd", bind_parse_expose_fd, 1 }, /* set the unix socket expose 
> fd rights */
> +     { "no-stop",   bind_parse_no_stop, 0 }, /* flag LI_O_NOSTOP in the 
> listener options */
>       { "severity-output", bind_parse_severity_output, 1 }, /* set the 
> severity output format */
>       { NULL, NULL, 0 },
>  }};
> 
> 

That's not a good idea in my opinion, it's an internal state that should
be used only in the case of communication between the master and the
workers, if we expose this to users we will probably have a lot of
corner cases to handle.
This keyword is only meant to say to a worker that it must keep the
communication with the master even if it's trying to exit, so we could
do some maintenance or debugging over the master CLI.

-- 
William Lallemand

Reply via email to