Re: Removal / obsolescence of keywords in 2.3 and future - replacing 'monitor-uri' w/ Lua

2020-10-20 Thread Robin H. Johnson
On Wed, Oct 14, 2020 at 03:35:30PM +0200, Tim Düsterhus wrote:
> I believe I already said it somewhere: The most valuable thing about
> monitor-uri is that it does not create entries within the access log. I
> don't think that can be replicated with http-request return as of now,
> but I am happy to learn otherwise.
I recently had to implement some health-of-HAProxy work, and went with
lua-services since HAProxy 1.8 didn't have 'http-request return', but the other
parts of the solution are still relevant.

'http-request set-log-level silent if ...' was the key to NOT logging the
health-checks of HAProxy.

The Lua service used in the below is fairly trivial, but it's in Lua rather
than errorfiles or something else, as we had some dynamic data built into it,
and that worked will with existing code.

Rest of what I had built:

# Fill these in with something meaningful for your use-case:
acl haproxy_maint ... # map or dynamic-acl
acl haproxy_drain ... # map or dynamic-acl
acl haproxy_drain stopping eq 1
acl health_permitted_src src ... # what IPs are allowed to health-check

# All of the paths:
acl health_ANY path -m beg /haproxy-health/
# Usual check point:
acl health_check path -m str /haproxy-health/check
# Special cases if you want to force an output:
acl health_force_up path -m str /haproxy-health/force-up
acl health_force_ready path -m str /haproxy-health/force-ready
acl health_force_down path -m str /haproxy-health/force-down
acl health_force_drain path -m str /haproxy-health/force-drain
acl health_force_maint path -m str /haproxy-health/force-maint

# Deny unauthorized access to healthcheck
# This will be logged!
http-request set-log-level warning if health_ANY !health_permitted_src
http-request deny if health_ANY !health_permitted_src

# Make normal access to healthcheck silent, so it does not spam logs
http-request set-log-level silent if health_ANY health_permitted_src

# Forced states
# all implicit health_permitted_src in these:
http-request use-service lua.health_up if health_force_up
http-request use-service lua.health_ready if health_force_ready
http-request use-service lua.health_down if health_force_down
http-request use-service lua.health_drain if health_force_drain
http-request use-service lua.health_maint if health_force_maint

# Actual check:
# all implicit health_permitted_src in these:
# 1. If the sysadmin created a maint flag, return that
http-request use-service lua.health_maint if health_check haproxy_maint
# 2. If HAProxy is stopping, return a DRAIN state
http-request use-service lua.health_drain if health_check haproxy_drain
# 3. If the backend is up, return up
http-request use-service lua.health_up if health_check
# 4. If the backend is down, return down
http-request use-service lua.health_down if !health_check


-- 
Robin Hugh Johnson
E-Mail : robb...@orbis-terrarum.net
Home Page  : http://www.orbis-terrarum.net/?l=people.robbat2
GnuPG FP   : 11ACBA4F 4778E3F6 E4EDF38E B27B944E 34884E85


signature.asc
Description: PGP signature


Re: Removal / obsolescence of keywords in 2.3 and future

2020-10-14 Thread Willy Tarreau
Hi Lukas!

On Thu, Oct 15, 2020 at 12:53:33AM +0200, Lukas Tribus wrote:
> Hello,
> 
> On Wed, 14 Oct 2020 at 15:29, Willy Tarreau  wrote:
> > For "nbproc", given that I had no response in the previous question and
> > I anticipate some surprises if I play games with it, I'll probably apply
> > William's suggestion, consisting in starting to emit a warning about it,
> > and asking users to either remove it, or explicitly mention "nbthread 1"
> > to get rid of the warning, and to report their use case.
> 
> What about multi-threading performance across NUMA nodes?

There is no reason to share the exact same conf across NUMA nodes. If
you see them as independent machines (their own CPU, RAM and NIC) then
it remains perfectly possible (and desirable) to have completely separate
process, handed as independent services so that you can maintain them
separately.

> On discourse someone asked why nbproc and nbthread can't be combined:
> 
> https://discourse.haproxy.org/t/cpu-affinity-nbproc-1-and-nbthread-1/5675

Thanks for the link!

We had that in 1.8 and it was a total mess. I wanted to stop that very
quickly seeing that it was impossible when reading a config to know what
was coherent and what was not. When you realize that in your config, only
*some* parts will be synchronized via peers and others not, that some
backend servers will be seen as up in some threads belonging to a process
and down in others, stats will differ, CLI will only affect a process at
a time etc, and all this each time with multiple threads together. I'd
say that it combines all the defects of each mode!

> I'm not sure if this is a real use-case or simply a case of overengineering 
> ...

There will always be one "valid" use case here and there, but quite frankly
if you don't purposely share data, frontends or backends between CPU groups,
there will be little difference because nowadays a *lot* of stuff is thread
local. The scheduler is both local and global, fdtab is lockless, etc. I'd
rather get reports from those experiencing limitations when this happens,
we maintain LTS versions long enough to allow them to run something that
suits their needs for the time needed to address such issues. Plus given
that these people will not be QUIC users, they'll have limited interest in
upgrading to the latest versions.

By the way, I'd be curious to know what makes some people think they *need*
40 CPUs, beyond the common belief "I paid those cores, I need to use them
all".

Thanks!
Willy



Re: Removal / obsolescence of keywords in 2.3 and future

2020-10-14 Thread Lukas Tribus
Hello,

On Wed, 14 Oct 2020 at 15:29, Willy Tarreau  wrote:
> For "nbproc", given that I had no response in the previous question and
> I anticipate some surprises if I play games with it, I'll probably apply
> William's suggestion, consisting in starting to emit a warning about it,
> and asking users to either remove it, or explicitly mention "nbthread 1"
> to get rid of the warning, and to report their use case.

What about multi-threading performance across NUMA nodes?

On discourse someone asked why nbproc and nbthread can't be combined:

https://discourse.haproxy.org/t/cpu-affinity-nbproc-1-and-nbthread-1/5675


I'm not sure if this is a real use-case or simply a case of overengineering ...


lukas



Re: Removal / obsolescence of keywords in 2.3 and future

2020-10-14 Thread Willy Tarreau
On Wed, Oct 14, 2020 at 03:35:30PM +0200, Tim Düsterhus wrote:
> Willy,
> 
> Am 14.10.20 um 15:29 schrieb Willy Tarreau:
> > As previously discussed above, we can probably keep monitor-uri for now
> > as it works. It's not the most elegant thing in the code but replacing
> > it will be at least as annoying for some users as it is to keep it in
> > the code. Probably that we could update the doc to encourage use of
> > http-request return though. If someone has a good proposal that does
> > both monitor-uri and monitor-fail in one line, that would be nice (e.g.
> > maybe with Tim's new "iif").
> 
> I believe I already said it somewhere: The most valuable thing about
> monitor-uri is that it does not create entries within the access log. I
> don't think that can be replicated with http-request return as of now,
> but I am happy to learn otherwise.

That's a good point indeed. At some point we thought about making the
return directive flexible enough to support termination modes which would
update different counters or adjust the log level. That might be something
to refresh for 2.4.

But I'm not against keeping monitor-uri, I know it's easy to use and
convenient. It's just a bit limited and users don't always know how it
interacts with other rule sets. Nothing really harmful. But having the
doc suggest how to do it with http-request rules would be a great way
to educate users.

Willy



Re: Removal / obsolescence of keywords in 2.3 and future

2020-10-14 Thread Tim Düsterhus
Willy,

Am 14.10.20 um 15:29 schrieb Willy Tarreau:
> As previously discussed above, we can probably keep monitor-uri for now
> as it works. It's not the most elegant thing in the code but replacing
> it will be at least as annoying for some users as it is to keep it in
> the code. Probably that we could update the doc to encourage use of
> http-request return though. If someone has a good proposal that does
> both monitor-uri and monitor-fail in one line, that would be nice (e.g.
> maybe with Tim's new "iif").

I believe I already said it somewhere: The most valuable thing about
monitor-uri is that it does not create entries within the access log. I
don't think that can be replicated with http-request return as of now,
but I am happy to learn otherwise.

Best regards
Tim Düsterhus



Removal / obsolescence of keywords in 2.3 and future

2020-10-14 Thread Willy Tarreau
Hi all,

one year ago I proposed to remove support for "mode health",
"monitor-net" and "monitor-uri", then the conversation moved to the fact
that were missing "http-request return" to implement monitor-uri cleanly
and nobody complained about "mode health" nor "monitor-net" which are
already quite useless now since incompatible with SSL for example:

  https://www.mail-archive.com/haproxy@formilux.org/msg35204.html

And since then we forgot to kill them :-/ I rediscovered they were still
present while trying to make progress on the code changes required to
integrate with QUIC. Seeing two nice 'send(cfd, "OK")'  directly in the
code didn't make me laugh a lot to be honest.

So I'm going to kill them right now in 2.3 (a year late and with no more
prior warning than the past discussion above), otherwise we're stuck for
yet another round. I'm fine with adding suggestions about http-return in
the error messages though.

As previously discussed above, we can probably keep monitor-uri for now
as it works. It's not the most elegant thing in the code but replacing
it will be at least as annoying for some users as it is to keep it in
the code. Probably that we could update the doc to encourage use of
http-request return though. If someone has a good proposal that does
both monitor-uri and monitor-fail in one line, that would be nice (e.g.
maybe with Tim's new "iif").

Regarding "http-tunnel" which is ignored and was marked as deprecated
since 2.1-dev2, I'll kill it as well for 2.3.

For "nbproc", given that I had no response in the previous question and
I anticipate some surprises if I play games with it, I'll probably apply
William's suggestion, consisting in starting to emit a warning about it,
and asking users to either remove it, or explicitly mention "nbthread 1"
to get rid of the warning, and to report their use case.

I think we'll really have to spend some time discussing about future
cleanups in the language during 2.4-dev, so that we can start adding
other warnings in 2.4. Some of these could cover Tim's request to try
to make the naming a bit more consistent between sample fetches and
converters.

Regards,
Willy