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



Heath check responds up even when server is down

2020-10-14 Thread Wesley Lukehart
Hello fine people. Short time lurker, first time poster.

Was on version 2.0.5 with CentOS 7.6 and everything was working fine with 
Exchange 2019.
Upgraded to 2.2.3 and now when we put Exchange into maintenance mode HAProxy 
does not change status - it reports that all services are still up (L7OK/200).

Example backend:
backend be_ex2019_oab
  mode http
  balance roundrobin
  option httpchk GET /oab/healthcheck.htm
  option log-health-checks
  http-check expect status 200
  server  :443 check ssl inter 15s verify required 
ca-file 
  server  :443 check ssl inter 15s verify required 
ca-file 

If I stop the app pool for a service in IIS, or stop all of IIS, HAProxy will 
properly show the service/services as down - as it gets a non 200 response (503 
or 404).

When putting the Exchange server into maintenance mode, there is no http 
response.
When I check with a browser I get "ERR_HTTP2_PROTOCOL_ERROR" or "Secure 
Connection Failed". Basically no response.
When I check with wget from the haproxy server I get "HTTP request sent, 
awaiting response... Read error (Connection reset by peer) in headers."
Yet HAProxy is happy and continues to try to send mail to the down server - not 
good.

Any Ideas?

I just tried 2.2.4 and no joy.

Thanks,

-Luke


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: Unable to get pack file - HTTP 416

2020-10-14 Thread Tim Düsterhus
Hi List,

Am 14.10.20 um 00:34 schrieb Tim Düsterhus:
> Am 14.10.20 um 00:31 schrieb Willy Tarreau:
>> I wouldn't have thought about such a client-side issue, indeed!
>>
>> You may be interested in discussing it on the git mailing list:
>>
>>  g...@vger.kernel.org
>>
>> (not subscription required, just like here).
> 
> Yes, thank you. I'll definitely report this in a spare minute.
> 

For the record: I was able to find a somewhat recent email about this
issue in the git list archives and I replied to that thread.

My reply can be found here if you'd like to follow this further:

https://lore.kernel.org/git/419fd4ad-f9c2-927c-7ae0-c6083f70d...@bastelstu.be/

Best regards
Tim Düsterhus



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



set-path impact on ACL eval and on set-var

2020-10-14 Thread Adis Nezirovic

Hi guys,

Here is a funny config where I've encountered something unexpected.

The theme is URL rewriting (let's say for some API),
after getting the original URI, extracting fields from path, we want to 
construct new path and save extracted fields into query args (we use 
haproxy vars as storage)


1)
If you run the attached haproxy config as is, everything works as expected

curl -v localhost:8080/a/1/b/2 -o /dev/null 2>&1 |grep x-field

< x-field: /a/1/b/2?a=1=2


curl -v localhost:8080/a/1/d/2 -o /dev/null 2>&1 |grep x-field

< x-field: /a/1/d/2?a=1


2) However, since want to replace path as well, let's
uncomment first set-path from the config file
curl -v localhost:8080/a/1/b/2 -o /dev/null 2>&1 |grep x-field



< x-field: /a/1/b/2?a=1

Not expected!

If we move that set-path statement after set-query (Case 2) it starts 
working fine again.



What's happening? Looks like set-var was lazily evaluated only after 
we've used it in set-query lines guarded by acl.


3) If we replace both set-query lines with

 http-request set-query "a=%[var(txn.a)]=%[var(txn.b)]"

It starts working again, but we'll always see "empty" b in the output:

curl -v localhost:8080/a/1/b2/2 -o /dev/null 2>&1 |grep x-field

< x-field: /x/y/z?a=1=



P.S.
For reference, here is the relevant config part:

  http-request set-var(txn.a) path,field(3,/)

  acl is_b path,field(4,/) -m str "b"

  http-request set-var(txn.b) path,field(5,/) if is_b

  # Case 1, we can't see b value if we set-path here

  # http-request set-path /x/y/z

  http-request set-query "a=%[var(txn.a)]" if !is_b

  http-request set-query "a=%[var(txn.a)]=%[var(txn.b)]" if is_b

  # Case 2, but we can see b value now

  # http-request set-path /x/y/z


Best regards,
Adis
--
Adis Nezirovic
Software Engineer
HAProxy Technologies - Powering your uptime!
375 Totten Pond Road, Suite 302 | Waltham, MA 02451, US
+1 (844) 222-4340 | https://www.haproxy.com
global
log /dev/log local1

defaults
log global
timeout connect 5s
timeout client 10s
timeout server 10s
timeout http-request 1s
mode http

listen blah
bind 127.0.0.1:8080
http-request set-var(txn.a) path,field(3,/)
acl is_b path,field(4,/) -m str "b"
http-request set-var(txn.b) path,field(5,/) if is_b

# Case 1, we can't see b value if we set-path here
http-request set-path /x/y/z
#http-request set-query "a=%[var(txn.a)]" if !is_b
#http-request set-query "a=%[var(txn.a)]=%[var(txn.b)]" if is_b
http-request set-query "a=%[var(txn.a)]=%[var(txn.b)]"
# Case 2, but we can see b value now
#http-request set-path /x/y/z

http-request set-var(txn.url) url
http-response set-header X-Field %[var(txn.url)]

server srv1 haproxy.org:80


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



Too many response errors

2020-10-14 Thread Seena Fallah
Hi.

I'm facing many response errors from my backends and I have checked the
logs but there were no 5xx errors for these response errors! It seems I'm
in this section of code and because I use http-server-close it will count
failed_resp!
https://github.com/haproxy/haproxy/blob/master/src/http_ana.c#L1648-L1667
Can you please explain why keep-alive connections won't count this and what
actually is this?

Using haproxy 2.2.4 on docker

Thanks.


Re: [PATCH v2 0/4] add set server ssl command

2020-10-14 Thread Willy Tarreau
Hi William,

On Tue, Oct 13, 2020 at 04:10:35PM +0200, William Dauchy wrote:
> Hi William, Willy,
> 
> On Wed, Oct 7, 2020 at 5:02 PM William Dauchy  wrote:
> > adding some more thoughts we discussed internally with Pierre:
> > - we started to use `show servers state`, as it was the only way to
> > get the current config for the parameters we wanted to change with the
> > existing runtime API.
> > - even if it was not necessarily designed for that purpose, we need a
> > way to get the current config.
> > - we understood `show servers state` was more likely designed for
> > internal usage, especially for `load-server-state-from-file`, so that
> > my patch set has some implications.
> >
> > That being said, would it more acceptable to add a new API which which
> > reflects all the `set` commands such as:
> > - show server [/]
> > would display all the config of a given server or all of them
> > - show server / agent
> > - show server / state
> > - show server / weight
> > etc
> > and for my first proposition:
> > - show server ssl [/]
> > - set server ssl [/] on/off
> >
> > for our immediate usage around ssl we are also thinking about ca_file,
> > crt, crt-file.
> >
> > This also would be a good answer to simplify how it is managed through
> > the current dataplane API
> > https://www.haproxy.com/documentation/dataplaneapi/latest/#operation/createServer
> >
> > What are your thoughts about it?
> 
> any thoughts about this proposition?

We've discussed about some of these points last week with William.
I think at some point it would be useful to have a discussion on
these. I'm personally in favor of being able to configure more
things dynamically, I also know that server-state files tend to be
abused to store configuration parameters instead of just pure server
states, but overall based on what is needed, we could possibly
rethink the whole thing, to have for example:
  - servers fed from a dedicated, easily parsable/updatable file
  - ssl optionally pre-configured to allow easy on/off at run time
  - better detection of changed parameters across reloads (currently
the server-state file doesn't hold the config values, so that there
are quite a number of ambigous cases on reload).

And clearly I'd rather go back to the white board based on a clearly
defined set of requirements and use cases than add more hacks on top
of something that was not initially designed for this. For sure some
parts are still extensible (e.g. I don't see anything preventing from
preloading SSL contexts into servers at boot time), but that still
leaves us with server-state, templates etc and maybe we can do better,
simpler, with less impact for everyone.

We should probably have a live discussion between at least the 3 of us,
maybe a few more if that helps, and define how to go into that direction.

What do you think ?

Thanks,
Willy