Re: How to wait some time before retry?

[Patrick Hemmer]

*From:* Marco Colli [mailto:collimarc...@gmail.com]
*Sent:* Friday, September 27, 2019, 07:21 EDT
*To:* HAProxy 
*Subject:* How to wait some time before retry?

Still have this issue and I cannot find a solution. It would be great 
to have an option "wait time before retry" in the next versions of 
HAProxy (instead of the fixed value of 1 sec).


On Mon, Sep 16, 2019 at 2:03 PM Marco Colli wrote:


Hello!

I have a question about HAProxy configuration. Maybe someone has a
solution ;)

I have a HAProxy (v2.0) load balancer in front of many web servers.

When I restart the web servers the TCP socket remains closed for a
few seconds (~10s). For this reason I would like to retry failed
attempts to connect after some seconds.

I already use |option redispatch|, however it seems that does not
solve my issue. The problem is that the request is retried
immediately (after 1s), thus causing all the retries to fail. From
the HAProxy docs:

In order to avoid immediate reconnections to a server which is
restarting, a turn-around timer of min("timeout connect", one
second) is applied before a retry occurs.

Is there any option to wait some more time (e.g. 10s) before
retrying? Or do you have any other solution?




Not the cleanest solution, but something that could work would be to add 
a lua action to sleep for 10 seconds on the response when you have a 
connect error, and then override the response to a 307 (or perhaps 302) 
redirect back to the same location. This will then cause the browser to 
retry the request.



-Patrick

Re: [PATCH] improving github experience, kindly ask people to reproduce bugs on latest haproxy

[Patrick Hemmer]

*From:* Илья Шипицин [mailto:chipits...@gmail.com]
*Sent:* Thursday, September 19, 2019, 15:10 EDT
*To:* HAProxy 
*Subject:* [PATCH] improving github experience, kindly ask people to 
reproduce bugs on latest haproxy



hello,

please find attached patch

Ilya Shipitsin
I dunno, I've personally never been fond of it when bug reporters are 
blindly asked to upgrade to the latest version. Sometimes the request is 
justified, such as when the project maintainers have reason to believe 
the bug is fixed, or if the version is years old. But otherwise it can 
causes difficulties for the reporter. In corporate environments, it can 
be difficult to perform such upgrades. Sometimes these issues are only 
reproducible in production environments. So by asking them to upgrade, 
you're making them go through the difficulty, and potentially cause 
impact to their clients. And because that process can take a while, it's 
possible that by the time they do complete the upgrade, another version 
has been released.


I personally also find the use of heavy bug templates, with nuanced 
little checkboxes to be annoying. In this case more so because we 
already ask them to provide the version information, which answers the 
question the checkbox is for. And the checkbox "yes i'm on the latest" 
might be accurate at the time of submission, but can become out of date.


Now all that said, I'm not against encouraging people to try a later 
version if available. Just that I don't think it should be the expectation.


-Patrick

[RFC] MINOR: attempt to insert into priority queue when full

[Patrick Hemmer]

This is a draft patch which essentially allows a high priority request 
to replace a low priority request when the queue is full.


Currently if a high priority request comes along, and the destination 
queue is full, the request gets rejected (or redispatched if possible). 
This change allows inserting into the queue if there are lower priority 
requests present. When doing so, the lowest priority request is evicted 
to make room.


The intent is for that evicted request to be handled as if it just came 
in. Meaning if there were a brand new request coming in, and the 
configuration were such that the request should be rejected, then the 
evicted request should be rejected. If redispatch is enabled a new 
request would be sent to a different server, and so the evicted request 
should be as well.



Now as for why this is tagged RFC: The implementation for the handling 
of the evicted request is bad. If you notice inside pendconn_displace(), 
the section of code that handles this (lines 498-505) has no business 
being there. Those operations should be handled higher up. It also 
doesn't work as intended, as instead of responding to the client with a 
503, it just kills the connection.


However I'm having difficulty finding the appropriate way to do this, 
and could use some guidance. I've tried several different things, and 
none of the behaviors are quite right.
The simplest solution of just performing __pendconn_unlink() and then 
waking the task results in the request being redispatched. The higher 
level code assumes that if the request was in a queue, and is now no 
longer in a queue, then redispatch is the appropriate action.


Thanks

-Patrick
From a3c8ba92a05ec877662359f963ece0cfa82051f8 Mon Sep 17 00:00:00 2001
From: Patrick Hemmer 
Date: Thu, 12 Sep 2019 22:56:51 -0400
Subject: [PATCH] MINOR: attempt to insert into priority queue when full

This makes it so that when a queue (server or proxy) is full, that we try to
insert into the queue and evict a request of lower priority.

The evicted request will either be redispatched if `option redispatch` is
enabled, or rejected if not.
---
 include/proto/queue.h |   1 +
 src/backend.c |   3 +-
 src/queue.c   | 119 ++
 3 files changed, 122 insertions(+), 1 deletion(-)

diff --git a/include/proto/queue.h b/include/proto/queue.h
index a7ab63b35..156bf3964 100644
--- a/include/proto/queue.h
+++ b/include/proto/queue.h
@@ -37,6 +37,7 @@
 extern struct pool_head *pool_head_pendconn;
 
 struct pendconn *pendconn_add(struct stream *strm);
+int pendconn_displace(struct stream *strm);
 int pendconn_dequeue(struct stream *strm);
 void process_srv_queue(struct server *s);
 unsigned int srv_dynamic_maxconn(const struct server *s);
diff --git a/src/backend.c b/src/backend.c
index 1b01536c1..56340d2de 100644
--- a/src/backend.c
+++ b/src/backend.c
@@ -976,7 +976,8 @@ int assign_server_and_queue(struct stream *s)
(srv->nbpend || srv->served >= srv_dynamic_maxconn(srv))) {
 
if (srv->maxqueue > 0 && srv->nbpend >= srv->maxqueue)
-   return SRV_STATUS_FULL;
+   // queue is full. see if priority allows us to 
insert
+   return pendconn_displace(s);
 
p = pendconn_add(s);
if (p)
diff --git a/src/queue.c b/src/queue.c
index 30b7ef056..ea015272a 100644
--- a/src/queue.c
+++ b/src/queue.c
@@ -185,6 +185,33 @@ void pendconn_unlink(struct pendconn *p)
pendconn_queue_unlock(p);
 }
 
+/* Comapres 2 pendconn queue priority keys.
+ *
+ * Returns :
+ *   -1 if k1 < k2
+ *   0  if k1 == k2
+ *   1  if k1 > k2
+ */
+static int key_cmp(u32 k1, u32 k2)
+{
+   if (KEY_CLASS(k1) < KEY_CLASS(k2))
+   return -1;
+   if (KEY_CLASS(k1) > KEY_CLASS(k2))
+   return 1;
+
+   if (k1 < NOW_OFFSET_BOUNDARY())
+   k1 += 0x10; // key in the future
+   if (k2 < NOW_OFFSET_BOUNDARY())
+   k2 += 0x10; // key in the future
+
+   if (k1 < k2)
+   return -1;
+   if (k1 > k2)
+   return 1;
+
+   return 0;
+}
+
 /* Retrieve the first pendconn from tree . Classes are always
  * considered first, then the time offset. The time does wrap, so the
  * lookup is performed twice, one to retrieve the first class and a second
@@ -212,6 +239,31 @@ static struct pendconn *pendconn_first(struct eb_root 
*pendconns)
return eb32_entry(node2, struct pendconn, node);
 }
 
+/* Retrieve the last pendconn from tree .
+ * Follows the same semantics as pendconn_first.
+ */
+static struct pendconn *pendconn_last(struct eb_root *pendconns)
+{
+   struct eb32_node *node, *node2 = NULL;
+   u32 key;
+
+   node = eb32_last(pendconns);
+   if (!node)
+   return NULL;
+
+   key = KEY_CL

Re: fullconn not working

[Patrick Hemmer]

*From:* Jerome Magnin [mailto:jmag...@haproxy.com]
*Sent:* Tuesday, July 16, 2019, 10:19 EDT
*To:* Patrick Hemmer 
*Cc:* Pavlos Parissis , haproxy@formilux.org
*Subject:* fullconn not working


Hi Patrick,

On Tue, Jul 16, 2019 at 09:40:31AM -0400, Patrick Hemmer wrote:


*From:* Pavlos Parissis [mailto:pavlos.paris...@gmail.com]
*Sent:* Tuesday, July 16, 2019, 09:32 EDT
*To:* haproxy@formilux.org
*Cc:* Patrick Hemmer 
*Subject:* fullconn not working


On Παρασκευή, 28 Ιουνίου 2019 5:50:48 Μ.Μ. CEST Patrick Hemmer wrote:

I'm trying to get fullconn working, and can't seem to do so. I dunno if
it's a bug, or if it's my understanding that's wrong.
Basically my goal is to prevent the cumulative total of all connections
to all servers in a pool from exceeding a certain value.
For example I might have 10 servers, each with a maxconn of 10. But I
want to configure haproxy with a pool-wide limit of 50, so that even if
the connections are well distributed and no one server is maxed out,
after 50 connections to all servers, haproxy will still start to queue
instead.

fullconn seems like the right way to accomplish this, however I cannot
get it to work. I've tried a simple setup of 2 servers, each with
`maxconn 3`, and then a backend `fullconn 2`, which should result in
queuing after 2 simultaneous connections, however it doesn't. If I send
4 connections, all 4 are simultaneously sent to the backend servers.

Here's my test config:
defaults
   log 127.0.0.1:1234 daemon
   mode http
   option httplog
   timeout queue 5s
frontend f1
   bind :8001
   default_backend b1
backend b1
   fullconn 2
   server s1 127.0.0.1:8081 minconn 1 maxconn 3
   server s2 127.0.0.1:8081 minconn 1 maxconn 3

Here's how I test:
for i in {1..4}; do curl "http://localhost:8001/?sleep=2=$i; & done

And here's the logs:
<30>Jun 28 11:37:47 haproxy[75322]: 127.0.0.1:55119
[28/Jun/2019:11:37:45.658] f1 b1/s2 0/0/0/2003/2003 200 75 - - 
4/4/3/2/0 0/0 "GET /?sleep=2=3 HTTP/1.1"
<30>Jun 28 11:37:47 haproxy[75322]: 127.0.0.1:55117
[28/Jun/2019:11:37:45.658] f1 b1/s2 0/0/0/2003/2003 200 75 - - 
4/4/2/1/0 0/0 "GET /?sleep=2=4 HTTP/1.1"
<30>Jun 28 11:37:47 haproxy[75322]: 127.0.0.1:55118
[28/Jun/2019:11:37:45.658] f1 b1/s1 0/0/0/2003/2003 200 75 - - 
4/4/1/2/0 0/0 "GET /?sleep=2=1 HTTP/1.1"
<30>Jun 28 11:37:47 haproxy[75322]: 127.0.0.1:55120
[28/Jun/2019:11:37:45.658] f1 b1/s1 0/0/0/2003/2003 200 75 - - 
4/4/0/1/0 0/0 "GET /?sleep=2=2 HTTP/1.1"



Your e-mail client mangled above log lines and as a result they are bit 
unreadable.
The 4th field from `4/4/3/2/0`  is srv_conn  *2* which is below the maxconn of 
*3*, so haproxy did
the right thing as it didn't allow more than *full_conn* connections to be 
concurrently opened
against the server.

Cheers,
Pavlos

maxconn and fullconn are different settings.
maxconn: 
https://cbonte.github.io/haproxy-dconv/1.9/configuration.html#maxconn%20(Server%20and%20default-server%20options)
fullconn:
https://cbonte.github.io/haproxy-dconv/1.9/configuration.html#fullconn

-Patrick

fullconn is not used to set a limit on the amount of connections handled by the
backend. It is used to have a 'dynamic' maxconn value on server lines.

this dymanic maxconn value will never be below minconn, and never higher than
maxconn. When the amount of connections handled by backend is between 0 and
fullconn, this dynamic maxconn value is somewhere between minconn and maxconn,
proportionnate to where we are between 0 and fullconn. When fullconn is reached,
dynamic maxconn equals the maxconn you set on server line.

With the values you set, when you reach 2 connections at backend level, servers
will allow 3 at most, each.

Jérôme

Thanks, I think I'm following now. My understanding was backwards.
I'm guessing the use case for this is so that you have maxconn at a 
value where servers have a good response time. But then if you start 
queueing too much, then maxconn is raised, trading slower response time 
for higher throughput.



So my next question, is there any way to set a backend connection limit 
(after which connections get queued)? I want to limit the number of 
active connections across the whole pool, regardless of the number of 
servers, or what the limit on each server is.


The reason is that there are shared resources (e.g. a database, NAS 
filesystem, etc) behind a pool, and that shared resource can only handle 
so much load. We can divide the cumulative limit across the number of 
servers in the pool, but if some of the servers are out of the pool, 
then that limit shrinks and we're not utilizing the full capacity of the 
shared resources.



-Patrick

Re: fullconn not working

[Patrick Hemmer]

*From:* Pavlos Parissis [mailto:pavlos.paris...@gmail.com]
*Sent:* Tuesday, July 16, 2019, 09:32 EDT
*To:* haproxy@formilux.org
*Cc:* Patrick Hemmer 
*Subject:* fullconn not working


On Παρασκευή, 28 Ιουνίου 2019 5:50:48 Μ.Μ. CEST Patrick Hemmer wrote:

I'm trying to get fullconn working, and can't seem to do so. I dunno if
it's a bug, or if it's my understanding that's wrong.
Basically my goal is to prevent the cumulative total of all connections
to all servers in a pool from exceeding a certain value.
For example I might have 10 servers, each with a maxconn of 10. But I
want to configure haproxy with a pool-wide limit of 50, so that even if
the connections are well distributed and no one server is maxed out,
after 50 connections to all servers, haproxy will still start to queue
instead.

fullconn seems like the right way to accomplish this, however I cannot
get it to work. I've tried a simple setup of 2 servers, each with
`maxconn 3`, and then a backend `fullconn 2`, which should result in
queuing after 2 simultaneous connections, however it doesn't. If I send
4 connections, all 4 are simultaneously sent to the backend servers.

Here's my test config:
defaults
  log 127.0.0.1:1234 daemon
  mode http
  option httplog
  timeout queue 5s
frontend f1
  bind :8001
  default_backend b1
backend b1
  fullconn 2
  server s1 127.0.0.1:8081 minconn 1 maxconn 3
  server s2 127.0.0.1:8081 minconn 1 maxconn 3

Here's how I test:
for i in {1..4}; do curl "http://localhost:8001/?sleep=2=$i; & done

And here's the logs:
<30>Jun 28 11:37:47 haproxy[75322]: 127.0.0.1:55119
[28/Jun/2019:11:37:45.658] f1 b1/s2 0/0/0/2003/2003 200 75 - - 
4/4/3/2/0 0/0 "GET /?sleep=2=3 HTTP/1.1"
<30>Jun 28 11:37:47 haproxy[75322]: 127.0.0.1:55117
[28/Jun/2019:11:37:45.658] f1 b1/s2 0/0/0/2003/2003 200 75 - - 
4/4/2/1/0 0/0 "GET /?sleep=2=4 HTTP/1.1"
<30>Jun 28 11:37:47 haproxy[75322]: 127.0.0.1:55118
[28/Jun/2019:11:37:45.658] f1 b1/s1 0/0/0/2003/2003 200 75 - - 
4/4/1/2/0 0/0 "GET /?sleep=2=1 HTTP/1.1"
<30>Jun 28 11:37:47 haproxy[75322]: 127.0.0.1:55120
[28/Jun/2019:11:37:45.658] f1 b1/s1 0/0/0/2003/2003 200 75 - - 
4/4/0/1/0 0/0 "GET /?sleep=2=2 HTTP/1.1"



Your e-mail client mangled above log lines and as a result they are bit 
unreadable.
The 4th field from `4/4/3/2/0`  is srv_conn  *2* which is below the maxconn of 
*3*, so haproxy did
the right thing as it didn't allow more than *full_conn* connections to be 
concurrently opened
against the server.

Cheers,
Pavlos

maxconn and fullconn are different settings.
maxconn: 
https://cbonte.github.io/haproxy-dconv/1.9/configuration.html#maxconn%20(Server%20and%20default-server%20options)
fullconn: 
https://cbonte.github.io/haproxy-dconv/1.9/configuration.html#fullconn


-Patrick

Re: fullconn not working

[Patrick Hemmer]

*From:* Patrick Hemmer [mailto:hapr...@stormcloud9.net]
*Sent:* Friday, June 28, 2019, 11:50 EDT
*To:* HAProxy 
*Subject:* fullconn not working

I'm trying to get fullconn working, and can't seem to do so. I dunno 
if it's a bug, or if it's my understanding that's wrong.
Basically my goal is to prevent the cumulative total of all 
connections to all servers in a pool from exceeding a certain value.
For example I might have 10 servers, each with a maxconn of 10. But I 
want to configure haproxy with a pool-wide limit of 50, so that even 
if the connections are well distributed and no one server is maxed 
out, after 50 connections to all servers, haproxy will still start to 
queue instead.


fullconn seems like the right way to accomplish this, however I cannot 
get it to work. I've tried a simple setup of 2 servers, each with 
`maxconn 3`, and then a backend `fullconn 2`, which should result in 
queuing after 2 simultaneous connections, however it doesn't. If I 
send 4 connections, all 4 are simultaneously sent to the backend servers.


Here's my test config:
defaults
    log 127.0.0.1:1234 daemon
    mode http
    option httplog
    timeout queue 5s
frontend f1
    bind :8001
    default_backend b1
backend b1
    fullconn 2
    server s1 127.0.0.1:8081 minconn 1 maxconn 3
    server s2 127.0.0.1:8081 minconn 1 maxconn 3

Here's how I test:
for i in {1..4}; do curl "http://localhost:8001/?sleep=2=$i; & done

And here's the logs:
<30>Jun 28 11:37:47 haproxy[75322]: 127.0.0.1:55119 
[28/Jun/2019:11:37:45.658] f1 b1/s2 0/0/0/2003/2003 200 75 - -  
4/4/3/2/0 0/0 "GET /?sleep=2=3 HTTP/1.1"
<30>Jun 28 11:37:47 haproxy[75322]: 127.0.0.1:55117 
[28/Jun/2019:11:37:45.658] f1 b1/s2 0/0/0/2003/2003 200 75 - -  
4/4/2/1/0 0/0 "GET /?sleep=2=4 HTTP/1.1"
<30>Jun 28 11:37:47 haproxy[75322]: 127.0.0.1:55118 
[28/Jun/2019:11:37:45.658] f1 b1/s1 0/0/0/2003/2003 200 75 - -  
4/4/1/2/0 0/0 "GET /?sleep=2=1 HTTP/1.1"
<30>Jun 28 11:37:47 haproxy[75322]: 127.0.0.1:55120 
[28/Jun/2019:11:37:45.658] f1 b1/s1 0/0/0/2003/2003 200 75 - -  
4/4/0/1/0 0/0 "GET /?sleep=2=2 HTTP/1.1"



So am I misunderstanding how fullconn works? Or is there a bug?
I've tested with 2.0.1, 1.9.8, and 1.8.13.

-Patrick


Anyone with any thoughts on this? Anyone using fullconn successfully?

As an additional item of note that makes me suspect this is a bug even 
more is that when viewing the stats page, it shows the number of 
sessions is greater than the limit.



-Patrick

Re: haproxy inappropriately sending rst_stream on http/2

[Patrick Hemmer]

*From:* Patrick Hemmer [mailto:hapr...@stormcloud9.net]
*Sent:* Wednesday, June 26, 2019, 08:06 EDT
*To:* haproxy@formilux.org 
*Subject:* haproxy inappropriately sending rst_stream on http/2

I'm running haproxy 1.9.8 and am having an issue where haproxy is 
sending a http/2 rst_stream message to the client for some reason.


When I look in the haproxy logs, the state termination flags are 
"CD--". However I know the client didn't abort the request as I have a 
packet capture showing the client did no such thing. Additionally 
there are other requests on the same connection, both before & after 
the one that gets reset, which go through fine.


Willy, I'm going to send both the logs and packet capture off-list.

-Patrick

So I sent the information to Willy, but I'm guessing he's occupied with 
other things as I haven't heard back (which is perfectly fine, this is 
an open source project with no support contract after all). But if there 
is someone else in the HAProxy org I can send the debug info to, I can 
do that. I just don't want to post it publicly.


Thanks

-Patrick

Re: DOC: Suggest to replace the netstat commands

[Patrick Hemmer]

*From:* Alain Belkadi [mailto:xigu...@linuxbeach.be]
*Sent:* Monday, July 8, 2019, 10:51 EDT
*To:* haproxy@formilux.org
*Subject:* DOC: Suggest to replace the netstat commands



Hello,

As the "netstat" command is deprecated since a long time (1), I 
suggest to replace it with other commands like ss and ip.


I've made a first patch for this. As the number of columns is higher 
than the 80 standard, I've made a second patch with less long lines 
... but that don't fit inside 80 chars and if I remove more spaces 
that don't looks good.


(1) https://en.wikipedia.org/wiki/Netstat

Regards,

This raises a potential issue. Netstat is deprecated on Linux yes, but 
not other OSs (which HAProxy runs on). However the example usage that 
was provided ("netstat -ltnp") is the Linux compatible flags, so the doc 
was already somewhat Linux specific. However other references, such as 
"netstat -i", are cross-platform.


So where do we draw the line on being Linux-specific in our 
documentation, vs. OS agnostic?


-Patrick

fullconn not working

[Patrick Hemmer]

I'm trying to get fullconn working, and can't seem to do so. I dunno if 
it's a bug, or if it's my understanding that's wrong.
Basically my goal is to prevent the cumulative total of all connections 
to all servers in a pool from exceeding a certain value.
For example I might have 10 servers, each with a maxconn of 10. But I 
want to configure haproxy with a pool-wide limit of 50, so that even if 
the connections are well distributed and no one server is maxed out, 
after 50 connections to all servers, haproxy will still start to queue 
instead.


fullconn seems like the right way to accomplish this, however I cannot 
get it to work. I've tried a simple setup of 2 servers, each with 
`maxconn 3`, and then a backend `fullconn 2`, which should result in 
queuing after 2 simultaneous connections, however it doesn't. If I send 
4 connections, all 4 are simultaneously sent to the backend servers.


Here's my test config:
defaults
    log 127.0.0.1:1234 daemon
    mode http
    option httplog
    timeout queue 5s
frontend f1
    bind :8001
    default_backend b1
backend b1
    fullconn 2
    server s1 127.0.0.1:8081 minconn 1 maxconn 3
    server s2 127.0.0.1:8081 minconn 1 maxconn 3

Here's how I test:
for i in {1..4}; do curl "http://localhost:8001/?sleep=2=$i; & done

And here's the logs:
<30>Jun 28 11:37:47 haproxy[75322]: 127.0.0.1:55119 
[28/Jun/2019:11:37:45.658] f1 b1/s2 0/0/0/2003/2003 200 75 - -  
4/4/3/2/0 0/0 "GET /?sleep=2=3 HTTP/1.1"
<30>Jun 28 11:37:47 haproxy[75322]: 127.0.0.1:55117 
[28/Jun/2019:11:37:45.658] f1 b1/s2 0/0/0/2003/2003 200 75 - -  
4/4/2/1/0 0/0 "GET /?sleep=2=4 HTTP/1.1"
<30>Jun 28 11:37:47 haproxy[75322]: 127.0.0.1:55118 
[28/Jun/2019:11:37:45.658] f1 b1/s1 0/0/0/2003/2003 200 75 - -  
4/4/1/2/0 0/0 "GET /?sleep=2=1 HTTP/1.1"
<30>Jun 28 11:37:47 haproxy[75322]: 127.0.0.1:55120 
[28/Jun/2019:11:37:45.658] f1 b1/s1 0/0/0/2003/2003 200 75 - -  
4/4/0/1/0 0/0 "GET /?sleep=2=2 HTTP/1.1"



So am I misunderstanding how fullconn works? Or is there a bug?
I've tested with 2.0.1, 1.9.8, and 1.8.13.

-Patrick

Re: Case Sensitive Headers

[Patrick Hemmer]

*From:* Luke Seelenbinder [mailto:l...@sermonaudio.com]
*Sent:* Wednesday, June 26, 2019, 10:07 EDT
*To:* HAProxy 
*Subject:* Case Sensitive Headers


Hello List,

I have a painful case of noncompliance to report and figure out how to 
fix.


When HTX is enabled, all headers are returned in lower case (e.g., 
content-length, date, etc.). This is obviously fine and within spec. 
Unfortunately, I'm using a rather frustrating piece of software 
(Wowza) that talks to an haproxy instance and Wowza requires that the 
content-length header is always camel case, i.e., Content-Length, 
otherwise requests fail.


I tried using http-response set-header Content-Length 
%[res.hdr(content-length)] if { res.hdr(content-length) -m found } to 
force the value to upper case, but that didn't help.


This is very obviously a case of badly behaving software and not a 
problem with HAProxy, but I'm wondering if there's any other way to 
force that header to Content-Length without turning HTX off.


Thanks for any ideas!

Best,
Luke

—
*Luke Seelenbinder*
SermonAudio.com  | Senior Software Engineer


This is just a stab in the dark, but try deleting the header, then 
adding it back. For example


http-response set-var(res.conlen) res.hdr(content-length)
http-response del-header content-length
http-response set-header Content-Length %[var(res.conlen)] if { 
var(res.conlen) -m found }


-Patrick

haproxy inappropriately sending rst_stream on http/2

[Patrick Hemmer]

I'm running haproxy 1.9.8 and am having an issue where haproxy is 
sending a http/2 rst_stream message to the client for some reason.


When I look in the haproxy logs, the state termination flags are "CD--". 
However I know the client didn't abort the request as I have a packet 
capture showing the client did no such thing. Additionally there are 
other requests on the same connection, both before & after the one that 
gets reset, which go through fine.


Willy, I'm going to send both the logs and packet capture off-list.

-Patrick

Re: [PATCH] MINOR: SSL: add client/server random sample fetches

[Patrick Hemmer]

*From:* Patrick Hemmer [mailto:hapr...@stormcloud9.net]
*Sent:* Tuesday, June 4, 2019, 16:38 EDT
*To:* haproxy@formilux.org
*Subject:* [PATCH] MINOR: SSL: add client/server random sample fetches

Re-send of earlier patch due to formatting issues (upgraded 
thunderbird and lost a bunch of stuff :-( ).

As an attachment this time, so should be safe.

-Patrick
The updated patch fixes the documentation to be in alphabetical order. 
Got carried away with putting the doc in the same order the code is in.


-Patrick
From 39238b4840d409b5dcf198f2e03b5a58bb718d4a Mon Sep 17 00:00:00 2001
From: Patrick Hemmer 
Date: Tue, 4 Jun 2019 08:13:03 -0400
Subject: [PATCH] MINOR: SSL: add client/server random sample fetches

This adds 4 sample fetches:
- ssl_fc_client_random
- ssl_fc_server_random
- ssl_bc_client_random
- ssl_bc_server_random

These fetches retrieve the client or server random value sent during the
handshake.

Their use is to be able to decrypt traffic sent using ephemeral ciphers. Tools
like wireshark expect a TLS log file with lines in a few known formats
(https://code.wireshark.org/review/gitweb?p=wireshark.git;a=blob;f=epan/dissectors/packet-tls-utils.c;h=28a51fb1fb029eae5cea52d37ff5b67d9b11950f;hb=HEAD#l5209).
Previously the only format supported using data retrievable from HAProxy state
was the one utilizing the Session-ID. However an SSL/TLS session ID is
optional, and thus cannot be relied upon for this purpose.

This change introduces the ability to extract the client random instead which
can be used for one of the other formats. The change also adds the ability to
extract the server random, just in case it might have some other use, as the
code change to support this was trivial.
---
 doc/configuration.txt | 20 
 src/ssl_sock.c| 35 +++
 2 files changed, 55 insertions(+)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 074a7fffe..e6e6285a6 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -15430,6 +15430,11 @@ ssl_bc_cipher : string
   Returns the name of the used cipher when the outgoing connection was made
   over an SSL/TLS transport layer.
 
+ssl_bc_client_random : binary
+  Returns the client random of the back connection when the incoming connection
+  was made over an SSL/TLS transport layer. It is useful to to decrypt traffic
+  sent using ephemeral ciphers. This requires OpenSSL >= 1.1.0, or BoringSSL.
+
 ssl_bc_is_resumed : boolean
   Returns true when the back connection was made over an SSL/TLS transport
   layer and the newly created SSL session was resumed using a cached
@@ -15454,6 +15459,11 @@ ssl_bc_unique_id : binary
   returns the TLS unique ID as defined in RFC5929 section 3. The unique id
   can be encoded to base64 using the converter: "ssl_bc_unique_id,base64".
 
+ssl_bc_server_random : binary
+  Returns the server random of the back connection when the incoming connection
+  was made over an SSL/TLS transport layer. It is useful to to decrypt traffic
+  sent using ephemeral ciphers. This requires OpenSSL >= 1.1.0, or BoringSSL.
+
 ssl_bc_session_id : binary
   Returns the SSL ID of the back connection when the outgoing connection was
   made over an SSL/TLS transport layer. It is useful to log if we want to know
@@ -15675,6 +15685,11 @@ ssl_fc_cipherlist_xxh : integer
   "tune.ssl.capture-cipherlist-size" is set greater than 0, however the hash
   take in account all the data of the cipher list.
 
+ssl_fc_client_random : binary
+  Returns the client random of the front connection when the incoming 
connection
+  was made over an SSL/TLS transport layer. It is useful to to decrypt traffic
+  sent using ephemeral ciphers. This requires OpenSSL >= 1.1.0, or BoringSSL.
+
 ssl_fc_has_crt : boolean
   Returns true if a client certificate is present in an incoming connection 
over
   SSL/TLS transport layer. Useful if 'verify' statement is set to 'optional'.
@@ -15719,6 +15734,11 @@ ssl_fc_unique_id : binary
   returns the TLS unique ID as defined in RFC5929 section 3. The unique id
   can be encoded to base64 using the converter: "ssl_bc_unique_id,base64".
 
+ssl_fc_server_random : binary
+  Returns the server random of the front connection when the incoming 
connection
+  was made over an SSL/TLS transport layer. It is useful to to decrypt traffic
+  sent using ephemeral ciphers. This requires OpenSSL >= 1.1.0, or BoringSSL.
+
 ssl_fc_session_id : binary
   Returns the SSL ID of the front connection when the incoming connection was
   made over an SSL/TLS transport layer. It is useful to stick a given client to
diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index 2eb344dfa..fb7e96bf9 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -7195,6 +7195,37 @@ smp_fetch_ssl_fc_session_id(const struct arg *args, 
struct sample *smp, const ch
 
 
 #if HA_OPENSSL_VERSION_NUMBER >

[PATCH] MINOR: SSL: add client/server random sample fetches

[Patrick Hemmer]

Re-send of earlier patch due to formatting issues (upgraded thunderbird 
and lost a bunch of stuff :-( ).

As an attachment this time, so should be safe.

-Patrick
From 0947dc1faf7a0a90631adcebc2e65fc191da8473 Mon Sep 17 00:00:00 2001
From: Patrick Hemmer 
Date: Tue, 4 Jun 2019 08:13:03 -0400
Subject: [PATCH] MINOR: SSL: add client/server random sample fetches

This adds 4 sample fetches:
- ssl_fc_client_random
- ssl_fc_server_random
- ssl_bc_client_random
- ssl_bc_server_random

These fetches retrieve the client or server random value sent during the
handshake.

Their use is to be able to decrypt traffic sent using ephemeral ciphers. Tools
like wireshark expect a TLS log file with lines in a few known formats
(https://code.wireshark.org/review/gitweb?p=wireshark.git;a=blob;f=epan/dissectors/packet-tls-utils.c;h=28a51fb1fb029eae5cea52d37ff5b67d9b11950f;hb=HEAD#l5209).
Previously the only format supported using data retrievable from HAProxy state
was the one utilizing the Session-ID. However an SSL/TLS session ID is
optional, and thus cannot be relied upon for this purpose.

This change introduces the ability to extract the client random instead which
can be used for one of the other formats. The change also adds the ability to
extract the server random, just in case it might have some other use, as the
code change to support this was trivial.
---
 doc/configuration.txt | 20 
 src/ssl_sock.c| 35 +++
 2 files changed, 55 insertions(+)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 074a7fffe..f1325ea3f 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -15459,6 +15459,16 @@ ssl_bc_session_id : binary
   made over an SSL/TLS transport layer. It is useful to log if we want to know
   if session was reused or not.
 
+ssl_bc_client_random : binary
+  Returns the client random of the back connection when the incoming connection
+  was made over an SSL/TLS transport layer. It is useful to to decrypt traffic
+  sent using ephemeral ciphers. This requires OpenSSL >= 1.1.0, or BoringSSL.
+
+ssl_bc_server_random : binary
+  Returns the server random of the back connection when the incoming connection
+  was made over an SSL/TLS transport layer. It is useful to to decrypt traffic
+  sent using ephemeral ciphers. This requires OpenSSL >= 1.1.0, or BoringSSL.
+
 ssl_bc_session_key : binary
   Returns the SSL session master key of the back connection when the outgoing
   connection was made over an SSL/TLS transport layer. It is useful to decrypt
@@ -15725,6 +15735,16 @@ ssl_fc_session_id : binary
   a server. It is important to note that some browsers refresh their session ID
   every few minutes.
 
+ssl_fc_client_random : binary
+  Returns the client random of the front connection when the incoming 
connection
+  was made over an SSL/TLS transport layer. It is useful to to decrypt traffic
+  sent using ephemeral ciphers. This requires OpenSSL >= 1.1.0, or BoringSSL.
+
+ssl_fc_server_random : binary
+  Returns the server random of the front connection when the incoming 
connection
+  was made over an SSL/TLS transport layer. It is useful to to decrypt traffic
+  sent using ephemeral ciphers. This requires OpenSSL >= 1.1.0, or BoringSSL.
+
 ssl_fc_session_key : binary
   Returns the SSL session master key of the front connection when the incoming
   connection was made over an SSL/TLS transport layer. It is useful to decrypt
diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index 2eb344dfa..fb7e96bf9 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -7195,6 +7195,37 @@ smp_fetch_ssl_fc_session_id(const struct arg *args, 
struct sample *smp, const ch
 
 
 #if HA_OPENSSL_VERSION_NUMBER >= 0x1010L || defined(OPENSSL_IS_BORINGSSL)
+static int
+smp_fetch_ssl_fc_random(const struct arg *args, struct sample *smp, const char 
*kw, void *private)
+{
+   struct connection *conn = (kw[4] != 'b') ? objt_conn(smp->sess->origin) 
:
+   smp->strm ? 
cs_conn(objt_cs(smp->strm->si[1].end)) : NULL;
+   struct buffer *data;
+   struct ssl_sock_ctx *ctx;
+
+   if (!conn || !conn->xprt_ctx || conn->xprt != _sock)
+   return 0;
+   ctx = conn->xprt_ctx;
+
+   data = get_trash_chunk();
+   if (kw[7] == 'c')
+   data->data = SSL_get_client_random(ctx->ssl,
+   
(unsigned char *) data->area,
+   
data->size);
+   else
+   data->data = SSL_get_server_random(ctx->ssl,
+   
(unsigned char *) data->area,
+   

Re: http_first_req not working with http2

[Patrick Hemmer]

*From:* Willy Tarreau [mailto:w...@1wt.eu]
*Sent:* Tuesday, June 4, 2019, 10:08 EDT
*To:* Patrick Hemmer 
*Cc:* haproxy@formilux.org
*Subject:* http_first_req not working with http2


Hi Patrick,

On Mon, Jun 03, 2019 at 05:21:26PM -0400, Patrick Hemmer wrote:

As subject says, it appears that the http_first_req sample is not working
with http2.

Indeed, the first request in H1 is special in that it is not
automatically retryable. In H2 it's different, all requests are
totally independant and there is not even any real ordering. Since
it is possible to use GOAWAY to cleanly close a connection, no H2
request suffers from the limitations of the first H1 request, so
none of them is marked first. At the very least this should be
mentioned in the doc.

Do you have a particular use case that relies on this and which would
need any form of emulation of this older behaviour ? I couldn't find
any but maybe there are.

Thanks,
Willy


The use case was that I was trying to log SSL data (via a Lua script) 
when the connection is first established. If the frontend is operating 
in http mode (as opposed to tcp), we can only apply actions on receipt 
of a request, and not after connection is established. Thus I was trying 
to use http_first_req to trigger the action only on the first request.


I suppose an alternate that might work would be to add support for using 
Lua actions with `tcp-request session`.


-Patrick

[PATCH] MINOR: SSL: add client/server random sample fetches

[Patrick Hemmer]

This adds 4 sample fetches:
- ssl_fc_client_random
- ssl_fc_server_random
- ssl_bc_client_random
- ssl_bc_server_random

These fetches retrieve the client or server random value sent during the
handshake.

Their use is to be able to decrypt traffic sent using ephemeral ciphers. 
Tools
like wireshark expect a TLS log file with lines in a few known formats
(https://code.wireshark.org/review/gitweb?p=wireshark.git;a=blob;f=epan/dissectors/packet-tls-utils.c;h=28a51fb1fb029eae5cea52d37ff5b67d9b11950f;hb=HEAD#l5209).
Previously the only format supported using data retrievable from HAProxy 
state
was the one utilizing the Session-ID. However an SSL/TLS session ID is
optional, and thus cannot be relied upon for this purpose.

This change introduces the ability to extract the client random instead 
which
can be used for one of the other formats. The change also adds the 
ability to
extract the server random, just in case it might have some other use, as the
code change to support this was trivial.
---
  doc/configuration.txt | 20 
  src/ssl_sock.c| 35 +++
  2 files changed, 55 insertions(+)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 074a7fffe..f1325ea3f 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -15459,6 +15459,16 @@ ssl_bc_session_id : binary
made over an SSL/TLS transport layer. It is useful to log if we want 
to know
if session was reused or not.
  +ssl_bc_client_random : binary
+  Returns the client random of the back connection when the incoming 
connection
+  was made over an SSL/TLS transport layer. It is useful to to decrypt 
traffic
+  sent using ephemeral ciphers. This requires OpenSSL >= 1.1.0, or 
BoringSSL.
+
+ssl_bc_server_random : binary
+  Returns the server random of the back connection when the incoming 
connection
+  was made over an SSL/TLS transport layer. It is useful to to decrypt 
traffic
+  sent using ephemeral ciphers. This requires OpenSSL >= 1.1.0, or 
BoringSSL.
+
  ssl_bc_session_key : binary
Returns the SSL session master key of the back connection when the 
outgoing
connection was made over an SSL/TLS transport layer. It is useful to 
decrypt
@@ -15725,6 +15735,16 @@ ssl_fc_session_id : binary
a server. It is important to note that some browsers refresh their 
session ID
every few minutes.
  +ssl_fc_client_random : binary
+  Returns the client random of the front connection when the incoming 
connection
+  was made over an SSL/TLS transport layer. It is useful to to decrypt 
traffic
+  sent using ephemeral ciphers. This requires OpenSSL >= 1.1.0, or 
BoringSSL.
+
+ssl_fc_server_random : binary
+  Returns the server random of the front connection when the incoming 
connection
+  was made over an SSL/TLS transport layer. It is useful to to decrypt 
traffic
+  sent using ephemeral ciphers. This requires OpenSSL >= 1.1.0, or 
BoringSSL.
+
  ssl_fc_session_key : binary
Returns the SSL session master key of the front connection when the 
incoming
connection was made over an SSL/TLS transport layer. It is useful to 
decrypt
diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index 2eb344dfa..fb7e96bf9 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -7195,6 +7195,37 @@ smp_fetch_ssl_fc_session_id(const struct arg 
*args, struct sample *smp, const ch
#if HA_OPENSSL_VERSION_NUMBER >= 0x1010L || 
defined(OPENSSL_IS_BORINGSSL)
+static int
+smp_fetch_ssl_fc_random(const struct arg *args, struct sample *smp, 
const char *kw, void *private)
+{
+   struct connection *conn = (kw[4] != 'b') ? objt_conn(smp->sess->origin) 
:
+   smp->strm ? 
cs_conn(objt_cs(smp->strm->si[1].end)) : NULL;
+   struct buffer *data;
+   struct ssl_sock_ctx *ctx;
+
+   if (!conn || !conn->xprt_ctx || conn->xprt != _sock)
+   return 0;
+   ctx = conn->xprt_ctx;
+
+   data = get_trash_chunk();
+   if (kw[7] == 'c')
+   data->data = SSL_get_client_random(ctx->ssl,
+   
(unsigned char *) data->area,
+   
data->size);
+   else
+   data->data = SSL_get_server_random(ctx->ssl,
+   
(unsigned char *) data->area,
+   
data->size);
+   if (!data->data)
+   return 0;
+
+   smp->flags = 0;
+   smp->data.type = SMP_T_BIN;
+   smp->data.u.str = *data;
+
+   return 1;
+}
+
  static int
  smp_fetch_ssl_fc_session_key(const struct arg *args, struct sample 
*smp, const char *kw, void *private)
  {
@@ -9395,6 +9426,8 @@ static struct sample_fetch_kw_list 

unset-var doesn't support conditions

[Patrick Hemmer]

It appears that all usages of unset-var don't support conditions. 
Meaning none of the following work:


http-request unset-var(txn.foo) if { always_true }
tcp-request content unset-var(txn.foo) if { always_true }
etc.

The error presented is:
[ALERT] 153/175307 (58641) : parsing [haproxy-minimal.cfg:12] : error 
detected in frontend 'f1' while parsing 'http-request 
unset-var(txn.foo)' rule :

 fetch method not supported.
[ALERT] 153/175307 (58641) : Error(s) found in configuration file : 
haproxy-minimal.cfg



The documentation indicates this should be possible:

http-request unset-var() [ { if | unless }  ]

This is experienced with version 1.9.8

-Patrick

segfault in tcp-request session set-var

[Patrick Hemmer]

In haproxy 1.9.8, if you do `tcp-request session set-var()` with a 
variable in any scope other than sess, it segfaults.

For example:
tcp-request session set-var(txn.foo) ...
tcp-request session set-var(req.foo) ...

* thread #1, queue = 'com.apple.main-thread', stop reason = 
EXC_BAD_ACCESS (code=1, address=0x268)
    frame #0: 0x00010019c6a7 
haproxy`sample_store_stream(name="foo", scope=SCOPE_TXN, 
smp=0x7ffeefbfef48) at vars.c:442

   439         case SCOPE_RES:
   440         default: vars = >strm->vars_reqres; break;
   441         }
-> 442         if (vars->scope != scope)
   443             return 0;
   444
   445         HA_RWLOCK_WRLOCK(VARS_LOCK, >rwlock);
Target 0: (haproxy) stopped.

-Patrick

Re: http_first_req not working with http2

[Patrick Hemmer]

*From:* Patrick Hemmer [mailto:hapr...@stormcloud9.net]
*Sent:* Monday, June 3, 2019, 17:21 EDT
*To:* haproxy@formilux.org
*Subject:* http_first_req not working with http2

As subject says, it appears that the http_first_req sample is not 
working with http2.


Config:
  frontend f1
    bind :8000
    option http-use-htx
    log-format http_first_req=%[http_first_req]

With `curl http://localhost:8000`
Outputs:
  <30>Jun  3 17:16:36 haproxy[47767]: http_first_req=1


Where as,
Config:
  frontend f1
    bind :8000 proto h2
    option http-use-htx
    log-format http_first_req=%[http_first_req]

With `curl --http2-prior-knowledge http://localhost:8000`
Outputs:
  <30>Jun  3 17:16:51 haproxy[47829]: http_first_req=0


-Patrick


Oh, forgot the version. 1.9.8

-Patrick

http_first_req not working with http2

[Patrick Hemmer]

As subject says, it appears that the http_first_req sample is not 
working with http2.


Config:
  frontend f1
    bind :8000
    option http-use-htx
    log-format http_first_req=%[http_first_req]

With `curl http://localhost:8000`
Outputs:
  <30>Jun  3 17:16:36 haproxy[47767]: http_first_req=1


Where as,
Config:
  frontend f1
    bind :8000 proto h2
    option http-use-htx
    log-format http_first_req=%[http_first_req]

With `curl --http2-prior-knowledge http://localhost:8000`
Outputs:
  <30>Jun  3 17:16:51 haproxy[47829]: http_first_req=0


-Patrick

Lua logging to syslog & not stderr

[Patrick Hemmer]

Is there a way to have Lua log to syslog only, and not to stderr? When I 
call `TXN.log(...)`, the message shows up in syslog AND stderr.


The Lua documentation implies this is possible as it has this statement 
(http://www.arpalert.org/src/haproxy-lua-api/1.9/index.html):


> The log is sent, according with the HAProxy configuration file, on 
the default syslog server if it is configured and on the stderr if it is 
allowed.


So how do I make stderr not allowed?

In my config, I have the following log related settings in defaults
  log 127.0.0.1:514 daemon
  option httplog

-Patrick

Re: Capturing headers from http/2 trailers?

[Patrick Hemmer]

*From:* Willy Tarreau [mailto:w...@1wt.eu]
*Sent:* Saturday, May 25, 2019, 01:42 EDT
*To:* Patrick Hemmer 
*Cc:* Haproxy 
*Subject:* Capturing headers from http/2 trailers?


Hi Patrick,

On Fri, May 24, 2019 at 09:00:25AM -0400, Patrick Hemmer wrote:

Is there a way to capture (and log) headers from http/2 response trailers?
The documentation doesn't mention trailers, and when I try to reference
headers which are present in the trailers (e.g. "res.fhdr(grpc-status)"), it
doesn't get captured.

At the end of the day I'm trying to log the grpc-status and grpc-message
headers from gRPC responses.

For now you can't.

Bummer, but thanks for answering.


In fact, in legacy mode, trailers are simply dropped,
and in HTX they're currently encoded in H1 format. Some work has begun
to encode them the same way as regular headers, hoping that in a near
future they can have a more official existence. In this case we could
imagine adding a new analyser which passes after the data forwarding
ones to deal with those, and possibly capture them. Depending on the
complexity this represents we may even possibly imagine backporting
this to 2.0 (e.g. if it's just one analyser triggered in this case).

Based on your experience with gRPC, do you think it would be better to
have different sample fetch functions to look up trailers or to use the
same as the header ones ? My reading of the gRPC spec told me that the
header fields could appear either in headers or trailers sections, which
tends to make me think a unified set would be desirable. But there may
be very valid reasons for prefering to separate them.
The H2 RFC says trailers should be handled as described in RFC7230 
chunked trailers section, which goes on to say:
> When a chunked message containing a non-empty trailer is received, 
the recipient may process the fields (aside from those forbidden above) 
as if they were appended to the message's header section.


So this seems like treating them as one set is acceptable. And with this 
in mind, any producer of headers/trailers must be prepared for the 
consumer to treat them this way.


And yes, from my experience on gRPC, the grpc-status & grpc-message 
headers can appear in both (they appear in headers when there is no 
body). But these 2 headers have a set purpose and meaning. It doesn't 
matter which section the header/value came from, it still means the same 
thing.



Thanks

Re: Capturing headers from http/2 trailers?

[Patrick Hemmer]

*From:* Aleksandar Lazic [mailto:al-mob...@none.at]
*Sent:* Friday, May 24, 2019, 20:30 EDT
*To:* Patrick Hemmer 
*Cc:* Haproxy 
*Subject:* Capturing headers from http/2 trailers?


Hi.

Fri May 24 15:00:55 GMT+02:00 2019 Patrick Hemmer :


Is there a way to capture (and log) headers from http/2 response

  > trailers? The documentation doesn't mention trailers, and when I try to
  > reference headers which are present in the trailers (e.g.
  > "res.fhdr(grpc-status)"), it doesn't get captured.
  >
  > At the end of the day I'm trying to log the grpc-status and grpc-message
  > headers from gRPC responses.

In the upcoming 2.0 is ungrpc an protobuf available.

http://git.haproxy.org/?p=haproxy.git;a=blob;f=doc/configuration.txt;h=40424073e58048100f79d08963e49d614a6e7dcb;hb=HEAD

Maybe this will do what you want

The `grpc-status` and `grpc-message` headers are passed as h2 trailers. 
They're not in the protobuf body.


-Patrick

Capturing headers from http/2 trailers?

[Patrick Hemmer]

Is there a way to capture (and log) headers from http/2 response 
trailers? The documentation doesn't mention trailers, and when I try to 
reference headers which are present in the trailers (e.g. 
"res.fhdr(grpc-status)"), it doesn't get captured.


At the end of the day I'm trying to log the grpc-status and grpc-message 
headers from gRPC responses.


Thanks

-Patrick

haproxy 1.9.6 segfault in srv_update_status

[Patrick Hemmer]

We haven't had a chance to update to 1.9.8 yet, so we're still running 
1.9.6 (Linux) in production, and just had 2 segfaults happen a little 
over an hour apart. When I look at the core dumps from them, the stack 
trace is the same. I'm not sure if this is an issue already fixed, so 
providing just in case.


There was one oddity going on at the time these segfaults occurred. We 
had maxed out the Linux kernel's conntrack table. So haproxy would have 
been experiencing timeouts when attempting new connections, with health 
checks failing all over the place.



(gdb) bt full
#0  task_schedule (when=, task=0x0) at 
include/proto/task.h:439

No locals.
#1  srv_update_status (s=0x7f6ea12e2a80) at src/server.c:4872
    next_admin = 0
    check = 0x7f6ea12e2f20
    xferred = 
    px = 0x7f6ea12d8300
    prev_srv_count = 6
    srv_was_stopping = 0
    log_level = 
    tmptrash = 0x0
#2  0x7f6ea0a7bd22 in server_recalc_eweight 
(sv=sv@entry=0x7f6ea12e2a80, must_update=must_update@entry=1) at 
src/server.c:1310

    px = 
    w = 
#3  0x7f6ea0a81ce8 in srv_update_state (params=0x7ffc51186bf0, 
version=1, srv=0x7f6ea12e2a80) at src/server.c:3112

    p = 0x7ffc51186ccf ""
    srv_op_state = 
    bk_f_forced_id = 
    port = 8080
    srv_admin_state = 0
    srv_last_time_change = 6
    srv_check_state = 6
    srv_agent_state = 0
    srv_check_result = CHK_RES_PASSED
    fqdn_set_by_cli = 0
    srv_check_status = 15
    port_str = 0x7ffc51186cd2 "8080"
    srvrecord = 0x0
    msg = 0x7f6ea08d7fe0
    srv_uweight = 1
    srv_iweight = 1
    srv_check_health = 
    srv_f_forced_id = 
    fqdn = 0x0
#4  apply_server_state () at src/server.c:3514
    bk_f_forced_id = 
    check_id = 
    check_name = 
    cur = 
    end = 
    mybuf = 
"36\000backoffice\000\062\000iad1gbow02\000\061\060.3.66.169\000\061\000\060\000\061\000\061\000\066\000\061\065\000\063\000\062\000\066\000\060\000\060\000\060\000-\000\070\060\070\060\000-\000\000\000\000\061\000\060\000\062\000\060\000\060\000\060\000\060\000-\000\070\060\070\060\000-\000\000me_since_last_change 
srv_check_status srv_check_result srv_check_health srv_check_state 
srv_agent_st"...

    mybuflen = 
    params = {0x7ffc51186c90 "36", 0x7ffc51186c93 "backoffice", 
0x7ffc51186c9e "2", 0x7ffc51186ca0 "iad1gbow02", 0x7ffc51186cab 
"10.3.66.169",
  0x7ffc51186cb7 "1", 0x7ffc51186cb9 "0", 0x7ffc51186cbb "1", 
0x7ffc51186cbd "1", 0x7ffc51186cbf "6", 0x7ffc51186cc1 "15",
  0x7ffc51186cc4 "3", 0x7ffc51186cc6 "2", 0x7ffc51186cc8 "6", 
0x7ffc51186cca "0", 0x7ffc51186ccc "0", 0x7ffc51186cce "0",

  0x7ffc51186cd0 "-", 0x7ffc51186cd2 "8080", 0x7ffc51186cd7 "-"}
    srv_params = {0x7ffc51186cab "10.3.66.169", 0x7ffc51186cb7 "1", 
0x7ffc51186cb9 "0", 0x7ffc51186cbb "1", 0x7ffc51186cbd "1",
  0x7ffc51186cbf "6", 0x7ffc51186cc1 "15", 0x7ffc51186cc4 "3", 
0x7ffc51186cc6 "2", 0x7ffc51186cc8 "6", 0x7ffc51186cca "0",
  0x7ffc51186ccc "0", 0x7ffc51186cce "0", 0x7ffc51186cd0 "-", 
0x7ffc51186cd2 "8080", 0x7ffc51186cd7 "-", 0x0, 0x0, 0x0, 0x0}

    arg = 
    srv_arg = 
    version = 
    diff = 0
    f = 0x7f6ea16ad080
    filepath = 
    globalfilepath = "/var/lib/haproxy/state", '\000' times>...
    localfilepath = 
"d\000\000\000\000\000\000\000f\222Ҡn\177\000\000b\222Ҡn\177\000\000\000\177\030Q\374\177\000\000\020\000\000\000\000\000\000\000\265\221Ҡn\177\000\000\261\221Ҡn\177\000\000\003\000\000\000\000\000\000\000\250\221Ҡn\177\000\000\b\000\000\000\000\000\000\000\003\000\000\000\000\000\000\000\250\221Ҡn\177\000\000\001\222Ҡn\177\000\000\f\000\000\000\000\000\000\000\060\200\030Q\374\177\000\000\t\000\000\000\000\000\000\000\363\221Ҡn\177\000\000\344t\262\240n\177\000\000\265\221Ҡn\177\000\000\240\231m\241n\177\000\000\\\000\000\000\000\000\000\000x\331m\241n\177\000\000\016\222Ҡn\177\000\000\b\000\000\000\000\000\000\000\250\221Ҡn\177\000\000\302\000\000\000\000\000\000\000\060\200\030"...

    len = 
    fileopenerr = 
    globalfilepathlen = 
    localfilepathlen = 
    curproxy = 0x7f6ea12d8300
    bk = 0x7f6ea12d8300
    srv = 0x7f6ea12e2a80
#5  0x7f6ea0a8f48f in init (argc=, argc@entry=13, 
argv=, argv@entry=0x7ffc51189488) at src/haproxy.c:1843

    arg_mode = 
    tmp = 
    cfg_pidfile = 
    err_code = 9
    err_msg = 0x0
    wl = 
    progname = 0x7ffc5118acf6 "haproxy"
    change_dir = 
    px = 
    pcf = 
#6  0x7f6ea09e41a7 in main (argc=13, argv=0x7ffc51189488) at 
src/haproxy.c:2774

    err = 
    retry = 
    limit = {rlim_cur = 131072, rlim_max = 131072}
    errmsg = 

Re: HAProxy 1.9.6 unresponsive

[Patrick Hemmer]

*From:* Willy Tarreau [mailto:w...@1wt.eu]
*Sent:* Saturday, May 11, 2019, 06:10 EDT
*To:* Patrick Hemmer 
*Cc:* haproxy@formilux.org
*Subject:* HAProxy 1.9.6 unresponsive


Hi Patrick,

On Fri, May 10, 2019 at 09:17:25AM -0400, Patrick Hemmer wrote:

So I see a few updates on some of the other 100% CPU usage threads, and that
some fixes have been pushed. Are any of those in relation to this issue? Or
is this one still outstanding?

Apparently we've pulled a long piece of string and uncovered a series of
such bugs. It's likely that different persons have been affected by
different bugs. We still have the issue Maciej is experiencing that I'd
really like to nail down, given the last occurrence doesn't seem to make
sense as the code looks right after Olivier's fix.

Thanks,
Willy


Thanks, but I'm unsure if that means the issue I reported is fixed, or 
if other related issues are fixed and this one is still outstanding.
There's been mention of releasing 1.9.8. Will that release contain a fix 
for the issue reported in this thread?


-Patrick

Re: HAProxy 1.9.6 unresponsive

[Patrick Hemmer]

*From:* Willy Tarreau [mailto:w...@1wt.eu]
*Sent:* Tuesday, May 7, 2019, 14:46 EDT
*To:* Patrick Hemmer 
*Cc:* haproxy@formilux.org
*Subject:* HAProxy 1.9.6 unresponsive


Hi Patrick,

On Tue, May 07, 2019 at 02:01:33PM -0400, Patrick Hemmer wrote:

Just in case it's useful, we had the issue recur today. However I gleaned a
little more information from this recurrence. Provided below are several
outputs from a gdb `bt full`. The important bit is that in the captures, the
last frame which doesn't change between each capture is the `si_cs_send`
function. The last stack capture provided has the shortest stack depth of
all the captures, and is inside `h2_snd_buf`.

Thank you. At first glance this remains similar. Christopher and I have
been studying these issues intensely these days because they have deep
roots into some design choices and tradeoffs we've had to make and that
we're relying on, and we've come to conclusions about some long term
changes to address the causes, and some fixes for 1.9 that now appear
valid. We're still carefully reviewing our changes before pushing them.
Then I think we'll emit 1.9.8 anyway since it will already fix quite a
number of issues addressed since 1.9.7, so for you it will probably be
easier to try again.
  
So I see a few updates on some of the other 100% CPU usage threads, and 
that some fixes have been pushed. Are any of those in relation to this 
issue? Or is this one still outstanding?


Thanks

-Patrick

systemd watchdog support?

[Patrick Hemmer]

So with the prevalence of the issues lately where haproxy is going 
unresponsive and consuming 100% CPU, I wanted to see what thoughts were 
on implementing systemd watchdog functionality.


In our case, haproxy going unresponsive is extremely problematic as our 
clustering software (pacemaker+systemd) sees the service still running, 
and doesn't realize it needs to restart the service or fail over.
We could look into implementing some sort of custom check resource in 
pacemaker, but before going down that route I wanted to explore the 
systemd watchdog functionality.



The watchdog is implemented by periodically sending "WATCHDOG=1" on the 
systemd notification socket. However there are a few different ways I 
can see this being implemented.


We could put this in the master control process, but this only tells us 
if the master is functioning, not the workers, which are what really matter.


So the next thought would be for all of the workers to listen on a 
shared socket. The master would periodically send a request to that 
socket, and as long as it gets a response, it pings the watchdog. This 
tells us that there is at least one worker able to accept traffic.


However if a frontend is bound to a specific worker, then that would 
frontend would be non-responsive, and the watchdog wouldn't restart the 
service. For that the worker would have to send a request to each worker 
separately, and require a response from all of them before it pings the 
watchdog. This would be better able to detect issues, but for some 
people who aren't using any bound-to-process frontends, they would be 
able to handle failure of a single worker and potentially schedule a 
restart/reload at a less impactful time.


The last idea would be to have the watchdog watch the master only, and 
the master watches the workers in turn. If a worker stops responding, 
the master would restart just that one worker.



Any thoughts on the matter, or do we not want to do this, and rely on a 
custom check in the cluster management software?


-Patrick

Re: HAProxy 1.9.6 unresponsive

[Patrick Hemmer]

*From:* Willy Tarreau [mailto:w...@1wt.eu]
*Sent:* Monday, May 6, 2019, 08:42 EDT
*To:* Patrick Hemmer 
*Cc:* haproxy@formilux.org
*Subject:* HAProxy 1.9.6 unresponsive


On Sun, May 05, 2019 at 09:40:02AM +0200, Willy Tarreau wrote:

With this said, after studying the code a little bit more, I'm seeing a
potential case where if we'd have a trailers entry in the HTX buffer but
no end of message, we could loop forever there not consuming this block.
I have no idea if this is possible in an HTX message, I'll ask Christopher
tomorrow. In any case we need to address this one way or another, possibly
reporting an error instead if required. Thus I'm postponing 1.9.8 for
tomorrow.

So the case is indeed possible and at the moment all we can do is try to
minimize the probability to produce it :-(  The issue is caused by the
moment we've received the end of trailsers but not the end of the mesage.
 From the H2 protocol perspective if we've sent the END_STREAM flag, the
stream is closed, and a closed stream gets detached and cannot receive
new traffic, so at best we'll occasionally close too early and report
client failures at the upper layers while everything went OK. We cannot
send trailers without the END_STREAM flag since no frame may follow.
Abusing CONTINUATION is out of question here as this would require to
completely freeze the whole connection (including control frames) for
the time it takes to get this final EOM block. I thought about simply
reporting an error when we're in this situation between trailers and EOM
but it will mean that occasionally some chunked responses of sizes close
to 16N kB with trailers may err out, which is not acceptable either.

For 2.0 we approximately see what needs to be modified to address this
situation, but that will not be trivial and not backportable.

For 1.9 I'm still trying to figure what the "best" solution is. I may
finally end up marking the stream as closed as soon as we see the
trailers pushed down. I'm just unsure right now about all the possible
consequences and need to study the edge cases. Also I fear that this
will be something hard to unroll later, so I'm still studying.

Willy


Just in case it's useful, we had the issue recur today. However I 
gleaned a little more information from this recurrence. Provided below 
are several outputs from a gdb `bt full`. The important bit is that in 
the captures, the last frame which doesn't change between each capture 
is the `si_cs_send` function. The last stack capture provided has the 
shortest stack depth of all the captures, and is inside `h2_snd_buf`.


Otherwise it's still the behavior is the same as last time with `strace` 
showing absolutely nothing, so it's still looping.





#0  h1_headers_to_hdr_list (start=0x7f5a4ea6b5fb "grpco\243?", 
stop=0x7f5a4ea6b5ff "o\243?", hdr=hdr@entry=0x7ffdc58f6400, 
hdr_num=hdr_num@entry=101, h1m=h1m@entry=0x7ffdc58f63d0, 
slp=slp@entry=0x0) at src/h1.c:793

    ret = 
    state = 
    ptr = 
    end = 
    hdr_count = 
    skip = 0
    sol = 
    col = 
    eol = 
    sov = 
    sl = 
    skip_update = 
    restarting = 
    n = 
    v = {ptr = 0x7f5a4eb51453 "LZ\177", len = 140025825685243}
#1  0x7f5a4d862539 in h2s_htx_make_trailers 
(h2s=h2s@entry=0x7f5a4ecc7860, htx=htx@entry=0x7f5a4ea67630) at 
src/mux_h2.c:4996
    list = {{n = {ptr = 0x0, len = 0}, v = {ptr = 0x0, len = 0}} 
}

    h2c = 0x7f5a4ec56610
    blk = 
    blk_end = 0x0
    outbuf = {size = 140025844274259, area = 0x7f5a4d996efb 
 
"\205\300~\aHc\320H\001SXH\205\355t\026Lc\310E1\300D\211\351L\211⾃", 
data = 16472, head = 140025845781936}
    h1m = {state = H1_MSG_HDR_NAME, flags = 2056, curr_len = 0, 
body_len = 0, next = 4, err_pos = 0, err_state = 1320431563}

    type = 
    ret = 
    hdr = 0
    idx = 5
    start = 
#2  0x7f5a4d866ef5 in h2_snd_buf (cs=0x7f5a4e9a8980, 
buf=0x7f5a4e777d78, count=4, flags=) at src/mux_h2.c:5372

    h2s = 
    orig_count = 
    total = 16291
    ret = 
    htx = 0x7f5a4ea67630
    blk = 
    btype = 
    idx = 
#3  0x7f5a4d8f4be4 in si_cs_send (cs=cs@entry=0x7f5a4e9a8980) at 
src/stream_interface.c:691

    send_flag = 
    conn = 0x7f5a4e86f4c0
    si = 0x7f5a4e777f98
    oc = 0x7f5a4e777d70
    ret = 
    did_send = 0
#4  0x7f5a4d8f6305 in si_cs_io_cb (t=, 
ctx=0x7f5a4e777f98, state=) at src/stream_interface.c:737

    si = 0x7f5a4e777f98
    cs = 0x7f5a4e9a8980
    ret = 0
#5  0x7f5a4d925f02 in process_runnable_tasks () at src/task.c:437
    t = 
    state = 
    ctx = 
    process = 
    t = 
    max_processed = 
#6  0x7f5a4d89f6ff in run_poll_loop () at src/haproxy.c:2642
    next = 
    exp = 
#7  run_thread_poll_loop 

Re: [PATCH v2 1/2] MINOR: systemd: Use the variables from /etc/default/haproxy

[Patrick Hemmer]

*From:* Tim Duesterhus [mailto:t...@bastelstu.be]
*Sent:* Monday, May 6, 2019, 07:00 EDT
*To:* haproxy@formilux.org
*Cc:* Apollon Oikonomopoulos , 
wlallem...@haproxy.com, w...@1wt.eu, ber...@debian.org, Tim Duesterhus 

*Subject:* [PATCH v2 1/2] MINOR: systemd: Use the variables from 
/etc/default/haproxy



From: Apollon Oikonomopoulos 

This will allow seamless upgrades from the sysvinit system while respecting
any changes the users may have made. It will also make local configuration
easier than overriding the systemd unit file.

Note by Tim:

This GPL-2 licensed patch was taken from the Debian project at [1].

It was slightly modified to cleanly apply, because HAProxy's default unit
file does not include rsyslog.service as an 'After' dependency. Also the
subject line was modified to include the proper subsystem and severity.

This patch may be backported to 1.9.

[1] 
https://salsa.debian.org/haproxy-team/haproxy/blob/master/debian/patches/haproxy.service-use-environment-variables.patch

Co-authored-by: Tim Duesterhus 
---
  contrib/systemd/haproxy.service.in | 7 ---
  1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/contrib/systemd/haproxy.service.in 
b/contrib/systemd/haproxy.service.in
index 74e66e302..138701223 100644
--- a/contrib/systemd/haproxy.service.in
+++ b/contrib/systemd/haproxy.service.in
@@ -3,10 +3,11 @@ Description=HAProxy Load Balancer
  After=network.target
  
  [Service]

+EnvironmentFile=-/etc/default/haproxy
  Environment="CONFIG=/etc/haproxy/haproxy.cfg" "PIDFILE=/run/haproxy.pid"
-ExecStartPre=@SBINDIR@/haproxy -f $CONFIG -c -q
-ExecStart=@SBINDIR@/haproxy -Ws -f $CONFIG -p $PIDFILE
-ExecReload=@SBINDIR@/haproxy -f $CONFIG -c -q
+ExecStartPre=@SBINDIR@/haproxy -f $CONFIG -c -q $EXTRAOPTS
+ExecStart=@SBINDIR@/haproxy -Ws -f $CONFIG -p $PIDFILE $EXTRAOPTS
+ExecReload=@SBINDIR@/haproxy -f $CONFIG -c -q $EXTRAOPTS
  ExecReload=/bin/kill -USR2 $MAINPID
  KillMode=mixed
  Restart=always


/etc/default is a debianism. Other distros use different directories, 
such as RedHat which uses /etc/sysconfig


-Patrick

HAProxy 1.9.6 unresponsive

[Patrick Hemmer]

We are running HAProxy 1.9.6 and managed to get into a state where 
HAProxy was completely unresponsive. It was pegged at 100% like many of 
the other experiences here on the mailing list lately. But in addition 
it wouldn't respond to anything. The stats socket wasn't even responsive.


When I attached an strace, it sat there with no activity. When I 
attached GDB I got the following stack:


        (gdb) bt full
        #0  htx_get_head (htx=0x7fbeb666eba0) at include/common/htx.h:357
        No locals.
        #1  h2s_htx_make_trailers (h2s=h2s@entry=0x7fbeb625f9f0, 
htx=htx@entry=0x7fbeb666eba0) at src/mux_h2.c:4975
                        list = {{n = {ptr = 0x0, len = 0}, v = {ptr = 
0x0, len = 0}} }

                        h2c = 0x7fbeb6372320
                        blk = 
                        blk_end = 0x0
                        outbuf = {size = 140722044755807, area = 0x0, 
data = 140457080712096, head = 140457060939041}
                        h1m = {state = H1_MSG_HDR_NAME, flags = 2056, 
curr_len = 140457077580664, body_len = 16384, next = 2, err_pos = 0, 
err_state = -1237668736}

                        type = 
                        ret = 0
                        hdr = 0
                        idx = 
                        start = 
        #2  0x7fbeb50f2ef5 in h2_snd_buf (cs=0x7fbeb63ea9a0, 
buf=0x7fbeb6127048, count=2, flags=) at src/mux_h2.c:5372

                        h2s = 
                        orig_count = 
                        total = 15302
                        ret = 
                        htx = 0x7fbeb666eba0
                        blk = 
                        btype = 
                        idx = 
        #3  0x7fbeb5180be4 in si_cs_send (cs=0x7fbeb63ea9a0) at 
src/stream_interface.c:691

                        send_flag = 
                        conn = 0x7fbeb6051a70
                        si = 0x7fbeb6127268
                        oc = 0x7fbeb6127040
                        ret = 
                        did_send = 0
        #4  0x7fbeb51817c8 in si_update_both 
(si_f=si_f@entry=0x7fbeb6127268, si_b=si_b@entry=0x7fbeb61272a8) at 
src/stream_interface.c:850

                        req = 0x7fbeb6126fe0
                        res = 
                        cs = 
        #5  0x7fbeb50ea2e1 in process_stream (t=, 
context=0x7fbeb6126fd0, state=) at src/stream.c:2502

                        srv = 
                        s = 0x7fbeb6126fd0
                        sess = 
                        rqf_last = 
                        rpf_last = 3255042562
                        rq_prod_last = 
                        rq_cons_last = 
                        rp_cons_last = 7
                        rp_prod_last = 7
                        req_ana_back = 
                        req = 0x7fbeb6126fe0
                        res = 0x7fbeb6127040
                        si_f = 0x7fbeb6127268
                        si_b = 0x7fbeb61272a8
        #6  0x7fbeb51b20a8 in process_runnable_tasks () at 
src/task.c:434

                        t = 
                        state = 
                        ctx = 
                        process = 
                        t = 
                        max_processed = 
        #7  0x7fbeb512b6ff in run_poll_loop () at src/haproxy.c:2642
                        next = 
                        exp = 
        #8  run_thread_poll_loop (data=data@entry=0x7fbeb5d84620) at 
src/haproxy.c:2707

                        ptif = 
                        ptdf = 
                        start_lock = 0
        #9  0x7fbeb507d2b5 in main (argc=, 
argv=0x7ffc677d73b8) at src/haproxy.c:3343

                        tids = 0x7fbeb5d84620
                        threads = 0x7fbeb5eb6d90
                        i = 
                        old_sig = {__val = {68097, 0, 511101108338, 0, 
140722044760335, 140457059422467, 140722044760392, 140454020513805, 124, 
140457064304960, 390842023936, 140457064395072, 48, 140457035994976, 
18446603351664791121, 140454020513794}}

        ---Type  to continue, or q  to quit---
                        blocked_sig = {__val = {1844674406710583, 
18446744073709551615 }}

                        err = 
                        retry = 
                        limit = {rlim_cur = 131300, rlim_max = 131300}
                        errmsg = 
"\000@\000\000\000\000\000\000\002\366\210\263\276\177\000\000\300\364m\265\276\177\000\000`\227\274\263\276\177\000\000\030\000\000\000\000\000\000\000>\001\000\024\000\000\000\000p$o\265\276\177\000\000@>k\265\276\177\000\000\000\320$\265\276\177\000\000\274\276\177\000\000 
t}g\374\177\000\000\000\000\000\000\000\000\000\000P\367m\265"

                        pidfd = 

Our config is big and complex, and not something I want to post here (I 
may be able to provide directly if required). However I think the 
important bit is that we we have a frontend and backend which are used 
for load balancing gRPC traffic (thus h2). The backend servers are 

Re: [PATCH] MINOR: systemd: Make use of master socket in systemd unit

[Patrick Hemmer]

*From:* Willy Tarreau [mailto:w...@1wt.eu]
*Sent:* Monday, April 29, 2019, 23:55 EDT
*To:* William Lallemand 
*Cc:* Tim Düsterhus , Patrick Hemmer 
, haproxy@formilux.org

*Subject:* [PATCH] MINOR: systemd: Make use of master socket in systemd unit


On Tue, Apr 30, 2019 at 01:47:13AM +0200, William Lallemand wrote:

the systemd API
is limited and it's not possible to report a fail during a reload with 
sd_notify.

Anyway a service manager which requires daemons to stay in foreground,
does not allow processes to be replaced, and relies on asynchronous and
unidirectional signal delivery to perform critical actions such as config
updates is obviously misdesigned and can only result from the lack of
experience of production servers and people making them work every day.

I tend to think that the only longterm solution to work around all the
systemd imposed mess is to have haproxy become its own service manager.
It is a shame as it significantly complicates haproxy management on Linux
compared to sane operating systems, but I'm having a hard time accepting
that haproxy has become less reliable on Linux due to systemd. We've
already made progress in this direction with the master-worker model,
but we need to go further and make haproxy be able to serve as a client
to talk to an existing master and return relevant status codes. This
way we'll be able to emulate the start/reload/restart operations that
are performed on sane systems and gain reliability again, at the
expense of using a total of 3 processes talking to each other during
a reload instead of just one. The advantage is that it should work on
other operating systems too if needed.

In my opinion this is something we seriously need to work on for 2.1,
and address remaining limitations (doubled memory usage and CLI being
closed on reload).

By the way, does systemd support passing actions on stdin and getting
the result on stdout ? Given that the master process is forced to
stay in foreground, at least we could have the equivalent of the CLI
on stdio used as a "console" and not require a third process to talk
to it.

Willy
Not to start a systemd flame war, but you can always tell systemd to 
stop tracking the process, and you can then do whatever you want. Of 
course you won't get automatic restarts if it dies, but that's how all 
the other "sane operating systems" behave.

Just add to the service file:
Type=oneshot
RemainAfterExit=yes

Assuming we don't do that, and going back to the original issue at hand 
of putting the master socket in the systemd unit: do we want to do this? 
From all the discussion, whether or not we add a `haproxy reload` 
command to the haproxy binary, it still seems like we will need master 
socket support to make it work. So should this patch be adopted?


-Patrick

Re: [PATCH] MINOR: systemd: Make use of master socket in systemd unit

[Patrick Hemmer]

*From:* Tim Düsterhus [mailto:t...@bastelstu.be]
*Sent:* Friday, April 26, 2019, 15:03 EDT
*To:* Patrick Hemmer , William Lallemand 


*Cc:* haproxy@formilux.org, w...@1wt.eu
*Subject:* [PATCH] MINOR: systemd: Make use of master socket in systemd unit


Patrick,

Am 26.04.19 um 20:55 schrieb Patrick Hemmer:

One other consideration is a problem I've been looking to address, and
that's better reloads. Right now the systemd unit sends a signal for the
reload, but sending a signal gets no feedback on whether the reload
operation was successful or not.

Have you checked that you are using Type=notify in your unit file? It
uses systemd's sd_notify API to communicate the reload to systemd which
should do what you are searching for.

See this mailing list thread:
https://www.mail-archive.com/haproxy@formilux.org/msg27874.html

Best regards
Tim Düsterhus


Yes, we use Type=notify. The problem though is that systemd is reporting 
reload success even if the reload fails.


Take the following example config:

global
stats socket /tmp/foo

With the current systemd file provided by haproxy, I copied it as 
"haproxy-test.service" and modified one line to the following:


Environment="CONFIG=/tmp/haproxy.cfg" "PIDFILE=/run/haproxy-test.pid"

And started haproxy:

# systemctl start haproxy-test.service
-- Logs begin at Thu 2019-04-11 15:13:39 EDT. --
Apr 29 14:24:14 fll2albs01qa2 systemd[1]: Starting HAProxy Load Balancer...
Apr 29 14:24:14 fll2albs01qa2 haproxy[27702]: [NOTICE] 118/142414 
(27702) : New worker #1 (27704) forked

Apr 29 14:24:14 fll2albs01qa2 systemd[1]: Started HAProxy Load Balancer.

I then edited the config and changed the stats socket line to:

stats socket /notmp/foo

And then tried a reload:
# systemctl reload haproxy-test.service
-- Logs begin at Thu 2019-04-11 15:13:39 EDT. --
Apr 29 14:24:47 fll2albs01qa2 haproxy[27702]: [WARNING] 118/142447 
(27702) : Reexecuting Master process
Apr 29 14:24:47 fll2albs01qa2 haproxy[27702]: [ALERT] 118/142447 (27702) 
: Starting frontend GLOBAL: cannot bind UNIX socket [/notmp/foo]
Apr 29 14:24:47 fll2albs01qa2 haproxy[27702]: [WARNING] 118/142447 
(27702) : Reexecuting Master process in waitpid mode
Apr 29 14:24:47 fll2albs01qa2 haproxy[27702]: [WARNING] 118/142447 
(27702) : Reexecuting Master process

Apr 29 14:24:47 fll2albs01qa2 systemd[1]: Reloaded HAProxy Load Balancer.

# systemctl status haproxy-test.service
● haproxy-test.service - HAProxy Load Balancer
   Loaded: loaded (/etc/systemd/system/haproxy-test.service; disabled; 
vendor preset: disabled)

   Active: active (running) since Mon 2019-04-29 14:24:14 EDT; 45s ago
  Process: 28335 ExecReload=/bin/kill -USR2 $MAINPID (code=exited, 
status=0/SUCCESS)
  Process: 28334 ExecReload=/usr/sbin/haproxy -f $CONFIG -c -q 
(code=exited, status=0/SUCCESS)
  Process: 27700 ExecStartPre=/usr/sbin/haproxy -f $CONFIG -c -q 
(code=exited, status=0/SUCCESS)

 Main PID: 27702 (haproxy)
   Memory: 2.9M
   CGroup: /system.slice/haproxy-test.service
   ├─27702 /usr/sbin/haproxy -Ws -f /tmp/haproxy.cfg -p 
/run/haproxy-test.pid -sf 27704
   └─27704 /usr/sbin/haproxy -Ws -f /tmp/haproxy.cfg -p 
/run/haproxy-test.pid


Note that the reload failed, but systemd reports success.

With older versions of haproxy (<1.8) we had a script which checked the 
PID of the master process and that it changed after sending SIGUSR2. 
This strategy no longer works.

Re: [PATCH] MINOR: systemd: Make use of master socket in systemd unit

[Patrick Hemmer]

*From:* Tim Düsterhus [mailto:t...@bastelstu.be]
*Sent:* Friday, April 26, 2019, 14:30 EDT
*To:* William Lallemand 
*Cc:* haproxy@formilux.org, w...@1wt.eu
*Subject:* [PATCH] MINOR: systemd: Make use of master socket in systemd unit


William,

Am 26.04.19 um 14:56 schrieb William Lallemand:

On Fri, Apr 26, 2019 at 12:15:37AM +0200, Tim Duesterhus wrote:

  [Service]
-Environment="CONFIG=/etc/haproxy/haproxy.cfg" "PIDFILE=/run/haproxy.pid"
+Environment="CONFIG=/etc/haproxy/haproxy.cfg" "PIDFILE=/run/haproxy.pid" 
"MASTER_SOCKET=/run/haproxy-master.sock"
  ExecStartPre=@SBINDIR@/haproxy -f $CONFIG -c -q
-ExecStart=@SBINDIR@/haproxy -Ws -f $CONFIG -p $PIDFILE
+ExecStart=@SBINDIR@/haproxy -Ws -S $MASTER_SOCKET -f $CONFIG -p $PIDFILE
  ExecReload=@SBINDIR@/haproxy -f $CONFIG -c -q

In my opinion that's not a good idea to do it this way, because you can't
disable the master CLI by unsetting the MASTER_SOCKET variable.

It's probably better to have "-S /run/haproxy-master.sock" in an $OPTIONS
variable at the end of the ExecStart line.


I'm not sure whether this is better. Yes you can disable the socket more
easily then, but you have to remember to add it back when editing the
'OPTIONS' variable.

I believe it boils to to this question: Does the user usually want to
run with the socket enabled or not? My guess is that most users want to
have this socket (having is better than needing!), they might just want
to move it to a different location rather than outright disabling it.
During my tests it was only accessible to root, so there does not appear
a security issue in the default configuration either.

On an unrelated note: Debian patches the unit file to add in such a
variable, called 'EXTRAOPTS':
https://salsa.debian.org/haproxy-team/haproxy/blob/master/debian/patches/haproxy.service-use-environment-variables.patch

So if we want to go the 'OPTIONS' variable path we might want to re-use
that variable name. Any change will break their patch anyway so they
definitely notice.

Best regards
Tim Düsterhus

One other consideration is a problem I've been looking to address, and 
that's better reloads. Right now the systemd unit sends a signal for the 
reload, but sending a signal gets no feedback on whether the reload 
operation was successful or not.


I haven't thought about this a whole lot, but I'm thinking the way to 
address it would be some sort of inquiry to the master process, which 
means using the socket. So if the systemd unit file ensured that the 
master socket is available, then ExecReload could be adjusted to use it 
and get success/failure feedback.


-Patrick

Re: `stats bind-process` broken

[Patrick Hemmer]

*From:* Willy Tarreau [mailto:w...@1wt.eu]
*Sent:* Thursday, April 11, 2019, 13:23 EDT
*To:* Patrick Hemmer 
*Cc:* haproxy@formilux.org, wlallem...@haproxy.com
*Subject:* `stats bind-process` broken


On Thu, Apr 11, 2019 at 06:51:59PM +0200, Willy Tarreau wrote:

I'm leaning towards what I'd consider a cleaner and more future-proof
option consisting in deprecating "stats bind-process" in favor of "process"
on the stats line (which binds the listener itself, not the whole frontend)
and emitting a warning at startup time if master-worker + this option are
set together, explaining what to do instead. I think it could be a reasonably
acceptable change for 1.9/2.0 given that the overlap between master-worker
and "stats bind-process" should not be high.

Would you see any issue with this if the warning gives all the indications
on what to do ?

So just to let you know that after discussing this with William, we might
have another solution consisting in moving the master-worker socketpair
to a dedicated frontend instead. It looks more future-proof but we need
to validate a number of points before engaging that route.

Regards,
Willy
In regards to deprecating `stats bind-process`, I think this would be 
acceptable. I can't think of any issues that might arise from that. 
Though I'm not sure what else is part of this frontend, which I'm 
gathering is some sort of hidden frontend used for the stats socket, 
master-worker communication, and may be other things.

Seems like `stats socket ... process` would indeed be safer in that regard.

-Patrick

`stats bind-process` broken

[Patrick Hemmer]

With haproxy 1.9.6 the `stats bind-process` directive is not working. 
Every connection to the socket is going to a random process:


Here's a simple reproduction:
Config:
   global
       nbproc 3
       stats socket /tmp/haproxy.sock level admin
       stats bind-process 1


Testing:
   # for i in {1..5}; do socat - unix:/tmp/haproxy.sock <<< "show info" 
| grep Pid: ; done

   Pid: 33371
   Pid: 33373
   Pid: 33372
   Pid: 33373
   Pid: 33373

-Patrick

Re: Adding Configuration parts via File

[Patrick Hemmer]

On 2019/3/8 08:17, Philipp Kolmann wrote:
> Hi,
>
> I have ACLs for Source-IPs for Admins for several services. These ACLs
> are identical for multiple listener-sections.
>
> Would it be possible to have a file with several acl snipplets and
> source that at the proper section of the config file multiple times?
> I haven't found anything in the docs that would make this possible.
>
> My wished Setup:
>
> admin_acl.conf:
>
> acl is_admin src 10.0.0.1
> acl is_admin src 10.0.0.2
> acl is_admin src 10.0.0.3
> acl is_admin src 10.0.0.4
>
>
> haproxy.cfg:
>
> listen service1
> bind 10.1.0.10:80
> include admin_acl.conf
>
>  more parameters ...
>
>
> listen service2
> bind 10.1.0.20:80
> include admin_acl.conf
>
>  more parameters ...
>
>
> listen service3
> bind 10.1.0.30:80
> include admin_acl.conf
>
>  more parameters ...
>
>
> The admin_acl needs to be maintained only once and can be used
> multiple times.
>
> Is this already possible? Could such an include option be made for the
> config files?
>
> thanks
> Philipp
>

You can use external files in two cases. See the following blog articles:

https://www.haproxy.com/blog/introduction-to-haproxy-acls/ (search for
"acl file")

https://www.haproxy.com/blog/introduction-to-haproxy-maps/

-Patrick

Re: Issue with systemd haproxy.service on RHEL 7.4

[Patrick Hemmer]

On 2019/3/7 11:08, Badari Prasad wrote:
> Hi  
>  RHEL 7.4 comes with haproxy 1.5.18, I wanted use latest version of
> haproxy 1.9.4.  So source code comes with haproxy.service.in
> 
> [https://github.com/haproxy/haproxy/blob/master/contrib/systemd/haproxy.service.in]
> .
> Executing make in the dir contrib/systemd/ creates haproxy.service. I
> tried to copy this generarted file in
> : /usr/lib/systemd/system/haproxy.service .
> With this I see lots of errors : 
> #systemctl status haproxy.service
> ● haproxy.service - HAProxy Load Balancer
>Loaded: loaded (/usr/lib/systemd/system/haproxy.service; disabled;
> vendor preset: disabled)
>Active: inactive (dead)
>
> Mar 07 03:13:47 local systemd[1]:
> [/usr/lib/systemd/system/haproxy.service:9] Executable path is not
> absolute, ignoring: @SBINDIR@/haproxy -f $CONFIG -c -q
> Mar 07 03:13:47 local systemd[1]: haproxy.service lacks both
> ExecStart= and ExecStop= setting. Refusing.
> Mar 07 03:13:47 local systemd[1]:
> [/usr/lib/systemd/system/haproxy.service:3] Failed to add dependency
> on =syslog.target, ignoring: Invalid argument
> Mar 07 03:13:47 local systemd[1]:
> [/usr/lib/systemd/system/haproxy.service:7] Executable path is not
> absolute, ignoring: @SBINDIR@/haproxy -f $CONFIG -c -q
> Mar 07 03:13:47 local systemd[1]:
> [/usr/lib/systemd/system/haproxy.service:8] Executable path is not
> absolute, ignoring: @SBINDIR@/haproxy -Ws -f $CONFIG -p $PIDFILE
> Mar 07 03:13:47 local systemd[1]:
> [/usr/lib/systemd/system/haproxy.service:9] Executable path is not
> absolute, ignoring: @SBINDIR@/haproxy -f $CONFIG -c -q
> Mar 07 03:13:47 local systemd[1]: haproxy.service lacks both
> ExecStart= and ExecStop= setting. Refusing.
> Mar 07 03:29:51 local systemd[1]: Unit haproxy.service cannot be
> reloaded because it is inactive.
> Mar 07 09:28:57 local systemd[1]: Unit haproxy.service cannot be
> reloaded because it is inactive.
> Mar 07 09:35:45 local systemd[1]: Unit haproxy.service cannot be
> reloaded because it is inactive.
>
>
> Have attached haproxy.service for reference. Can I get some pointers
> to resolve this issue.
>
> Thanks
>  badari 
>
>

Differences between the output you have provided and the attached
service file indicates that you had an earlier version of the service
file present on your system, and have not reloaded systemd since
modifying it, so it's using the old file.
You need to run: `systemctl daemon-reload`

-Patrick

http/2 server-push support

[Patrick Hemmer]

Now that we have h2 support on frontends, backends, trailers, etc, I'm
hoping that server side server-push is somewhere on the roadmap. By
"server side" I mean not this middleware based server-push methodology
frequently used where a "Link" header is converted to a server push. But
instead where the server itself can generate the server-push responses.

Is there any plan on this, or when it might be available?

If it helps for prioritization, our use case for this is reduce
processing overhead. A single request might require the client to have
related resources, and those related resources might all involve
computation that is shared. If each request is handled separately (e.g.
Link header to server-push conversion), it would result in a lot of
duplicated work. So instead we want to do the computation once, and push
out the multiple responses separately.

-Patrick

Re: Error parsing commas in str() fetch

[Patrick Hemmer]

On 2019/2/23 18:18, Nick Ramirez wrote:
>
> If I set a variable like this:
>
>
> /http-request set-var(txn.allowed_methods) str("GET,POST,PUT")/
>
>
> Then I get an error:
>
>
> /parsing [/etc/haproxy/haproxy.cfg:20] : error detected in frontend
> 'fe_proxy' while parsing 'http-request set-var(txn.allowed_methods)'
> rule : fetch method 'str' : end of arguments expected at position 2,
> but got ',POST,PUT,OPTIONS'./
>
>
> This seems like HAProxy is parsing the comma as a delimiter between
> arguments to the str() fetch method instead of including it as part of
> the string that's surrounded by double quotes. Is this expected
> behavior? If so, is there a work-around? Some way to escape commas in
> a string passed to str()?
>
>
> Nick Ramirez
>

Yes this is known behavior. In the config documentation, on some of the
keywords, you'll see:
> Note that due to the config parser, it is not possible to use a comma
nor a closing parenthesis as delimitors.

You can get around it by using converters, and some sort of encoded
text, such as URL or base64. For example:

str(GET%2CPOST%2CPUT),url_dec()


-Patrick

Re: Compilation fails on OS-X

[Patrick Hemmer]

On 2019/2/14 12:45, Olivier Houchard wrote:
> Hi Patrick,
>
> On Thu, Feb 14, 2019 at 09:12:18AM -0500, Patrick Hemmer wrote:
>>
>> On 2019/2/14 08:20, Frederic Lecaille wrote:
>>> On 2/14/19 1:32 PM, Frederic Lecaille wrote:
>>>> On 2/13/19 7:30 PM, Patrick Hemmer wrote:
>>>>>
>>>>> On 2019/2/13 10:29, Olivier Houchard wrote:
>>>>>> Hi Patrick,
>>>>>>
>>>>>> On Wed, Feb 13, 2019 at 10:01:01AM -0500, Patrick Hemmer wrote:
>>>>>>> On 2019/2/13 09:40, Aleksandar Lazic wrote:
>>>>>>>> Am 13.02.2019 um 14:45 schrieb Patrick Hemmer:
>>>>>>>>> Trying to compile haproxy on my local machine for testing
>>>>>>>>> purposes and am
>>>>>>>>> running into the following:
>>>>>>>> Which compiler do you use?
>>>>>>> # gcc -v
>>>>>>> Configured with:
>>>>>>> --prefix=/Applications/Xcode.app/Contents/Developer/usr
>>>>>>> --with-gxx-include-dir=/usr/include/c++/4.2.1
>>>>>>> Apple LLVM version 9.0.0 (clang-900.0.39.2)
>>>>>>> Target: x86_64-apple-darwin17.7.0
>>>>>>> Thread model: posix
>>>>>>> InstalledDir:
>>>>>>> /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin
>>>>>>>
>>>>>>>
>>>>>>>>>  # make TARGET=osx
>>>>>>>>>  src/proto_http.c:293:1: error: argument to 'section'
>>>>>>>>> attribute is not
>>>>>>>>> valid for this target: mach-o section specifier requires a
>>>>>>>>> segment and section
>>>>>>>>> separated by a comma
>>>>>>>>>  DECLARE_POOL(pool_head_http_txn, "http_txn",
>>>>>>>>> sizeof(struct http_txn));
>>>>>>>>>  ^
>>>>>>>>>  include/common/memory.h:128:2: note: expanded from
>>>>>>>>> macro 'DECLARE_POOL'
>>>>>>>>>  REGISTER_POOL(, name, size)
>>>>>>>>>  ^
>>>>>>>>>  include/common/memory.h:123:2: note: expanded from
>>>>>>>>> macro 'REGISTER_POOL'
>>>>>>>>>  INITCALL3(STG_POOL,
>>>>>>>>> create_pool_callback, (ptr), (name),
>>>>>>>>> (size))
>>>>>>>>>  ^
>>>>>>>>>  include/common/initcall.h:102:2: note: expanded from
>>>>>>>>> macro 'INITCALL3'
>>>>>>>>>  _DECLARE_INITCALL(stage, __LINE__,
>>>>>>>>> function, arg1, arg2,
>>>>>>>>> arg3)
>>>>>>>>>  ^
>>>>>>>>>  include/common/initcall.h:78:2: note: expanded from macro
>>>>>>>>> '_DECLARE_INITCALL'
>>>>>>>>>  __DECLARE_INITCALL(__VA_ARGS__)
>>>>>>>>>  ^
>>>>>>>>>  include/common/initcall.h:65:42: note: expanded from macro
>>>>>>>>> '__DECLARE_INITCALL'
>>>>>>>>> __attribute__((__used__,__section__("init_"#stg))) =   \
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Issue occurs on master, and the 1.9 branch
>>>>>>>>>
>>>>>>>>> -Patrick
>>>>>> Does the (totally untested, because I have no Mac to test) patch
>>>>>> works for
>>>>>> you ?
>>>>> Unfortunately not. Just introduces a lot of new errors:
>>>>>
>>>>>
>>>>>  In file included from src/ev_poll.c:22:
>>>>>  In file included from include/common/hathreads.h:26:
>>>>>  include/common/initcall.h:134:22: error: expected ')'
>>>>>  DECLARE_INIT_SECTION(STG_PREPARE);
>>>>>   ^
>>>>>  include/common/initcall.h:134:1

Re: Compilation fails on OS-X

[Patrick Hemmer]

On 2019/2/14 08:20, Frederic Lecaille wrote:
> On 2/14/19 1:32 PM, Frederic Lecaille wrote:
>> On 2/13/19 7:30 PM, Patrick Hemmer wrote:
>>>
>>>
>>> On 2019/2/13 10:29, Olivier Houchard wrote:
>>>> Hi Patrick,
>>>>
>>>> On Wed, Feb 13, 2019 at 10:01:01AM -0500, Patrick Hemmer wrote:
>>>>> On 2019/2/13 09:40, Aleksandar Lazic wrote:
>>>>>> Am 13.02.2019 um 14:45 schrieb Patrick Hemmer:
>>>>>>> Trying to compile haproxy on my local machine for testing
>>>>>>> purposes and am
>>>>>>> running into the following:
>>>>>> Which compiler do you use?
>>>>> # gcc -v
>>>>> Configured with:
>>>>> --prefix=/Applications/Xcode.app/Contents/Developer/usr
>>>>> --with-gxx-include-dir=/usr/include/c++/4.2.1
>>>>> Apple LLVM version 9.0.0 (clang-900.0.39.2)
>>>>> Target: x86_64-apple-darwin17.7.0
>>>>> Thread model: posix
>>>>> InstalledDir:
>>>>> /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin
>>>>>
>>>>>
>>>>>>>  # make TARGET=osx
>>>>>>>  src/proto_http.c:293:1: error: argument to 'section'
>>>>>>> attribute is not
>>>>>>> valid for this target: mach-o section specifier requires a
>>>>>>> segment and section
>>>>>>> separated by a comma
>>>>>>>  DECLARE_POOL(pool_head_http_txn, "http_txn",
>>>>>>> sizeof(struct http_txn));
>>>>>>>  ^
>>>>>>>  include/common/memory.h:128:2: note: expanded from
>>>>>>> macro 'DECLARE_POOL'
>>>>>>>  REGISTER_POOL(, name, size)
>>>>>>>  ^
>>>>>>>  include/common/memory.h:123:2: note: expanded from
>>>>>>> macro 'REGISTER_POOL'
>>>>>>>  INITCALL3(STG_POOL,
>>>>>>> create_pool_callback, (ptr), (name),
>>>>>>> (size))
>>>>>>>  ^
>>>>>>>  include/common/initcall.h:102:2: note: expanded from
>>>>>>> macro 'INITCALL3'
>>>>>>>  _DECLARE_INITCALL(stage, __LINE__,
>>>>>>> function, arg1, arg2,
>>>>>>> arg3)
>>>>>>>  ^
>>>>>>>  include/common/initcall.h:78:2: note: expanded from macro
>>>>>>> '_DECLARE_INITCALL'
>>>>>>>  __DECLARE_INITCALL(__VA_ARGS__)
>>>>>>>  ^
>>>>>>>  include/common/initcall.h:65:42: note: expanded from macro
>>>>>>> '__DECLARE_INITCALL'
>>>>>>> __attribute__((__used__,__section__("init_"#stg))) =   \
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Issue occurs on master, and the 1.9 branch
>>>>>>>
>>>>>>> -Patrick
>>>> Does the (totally untested, because I have no Mac to test) patch
>>>> works for
>>>> you ?
>>>
>>> Unfortunately not. Just introduces a lot of new errors:
>>>
>>>
>>>  In file included from src/ev_poll.c:22:
>>>  In file included from include/common/hathreads.h:26:
>>>  include/common/initcall.h:134:22: error: expected ')'
>>>  DECLARE_INIT_SECTION(STG_PREPARE);
>>>   ^
>>>  include/common/initcall.h:134:1: note: to match this '('
>>>  DECLARE_INIT_SECTION(STG_PREPARE);
>>>  ^
>>>  include/common/initcall.h:124:82: note: expanded from macro
>>> 'DECLARE_INIT_SECTION'
>>>  extern __attribute__((__weak__)) const struct
>>> initcall *__start_init_##stg __asm("section$start$__DATA$" stg); \
>>> ^
>>
>> Try to use -E in place of -c option of your compiler to stop after
>> having preprocessed the code. Then have a look to how the code of
>> src/ev_poll.c was preprocessed.
>>
>> This should help.
>>
>> Fred.
>
> As this sounds to be a preprocessing issue, and to have a l

Re: Compilation fails on OS-X

[Patrick Hemmer]

On 2019/2/13 10:29, Olivier Houchard wrote:
> Hi Patrick,
>
> On Wed, Feb 13, 2019 at 10:01:01AM -0500, Patrick Hemmer wrote:
>>
>> On 2019/2/13 09:40, Aleksandar Lazic wrote:
>>> Am 13.02.2019 um 14:45 schrieb Patrick Hemmer:
>>>> Trying to compile haproxy on my local machine for testing purposes and am
>>>> running into the following:
>>> Which compiler do you use?
>>  # gcc -v
>>  Configured with: 
>> --prefix=/Applications/Xcode.app/Contents/Developer/usr 
>> --with-gxx-include-dir=/usr/include/c++/4.2.1
>>  Apple LLVM version 9.0.0 (clang-900.0.39.2)
>>  Target: x86_64-apple-darwin17.7.0
>>  Thread model: posix
>>  InstalledDir: 
>> /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin
>>
>>>> # make TARGET=osx
>>>> src/proto_http.c:293:1: error: argument to 'section' attribute is 
>>>> not
>>>> valid for this target: mach-o section specifier requires a segment and 
>>>> section
>>>> separated by a comma
>>>> DECLARE_POOL(pool_head_http_txn, "http_txn", sizeof(struct 
>>>> http_txn));
>>>> ^
>>>> include/common/memory.h:128:2: note: expanded from macro 
>>>> 'DECLARE_POOL'
>>>> REGISTER_POOL(, name, size)
>>>> ^
>>>> include/common/memory.h:123:2: note: expanded from macro 
>>>> 'REGISTER_POOL'
>>>> INITCALL3(STG_POOL, create_pool_callback, (ptr), 
>>>> (name),
>>>> (size))
>>>> ^
>>>> include/common/initcall.h:102:2: note: expanded from macro 
>>>> 'INITCALL3'
>>>> _DECLARE_INITCALL(stage, __LINE__, function, arg1, 
>>>> arg2,
>>>> arg3)
>>>> ^
>>>> include/common/initcall.h:78:2: note: expanded from macro
>>>> '_DECLARE_INITCALL'
>>>> __DECLARE_INITCALL(__VA_ARGS__)
>>>> ^
>>>> include/common/initcall.h:65:42: note: expanded from macro
>>>> '__DECLARE_INITCALL'
>>>>
>>>> __attribute__((__used__,__section__("init_"#stg))) =   \
>>>>
>>>>
>>>>
>>>> Issue occurs on master, and the 1.9 branch
>>>>
>>>> -Patrick
> Does the (totally untested, because I have no Mac to test) patch works for
> you ?

Unfortunately not. Just introduces a lot of new errors:


In file included from src/ev_poll.c:22:
In file included from include/common/hathreads.h:26:
include/common/initcall.h:134:22: error: expected ')'
DECLARE_INIT_SECTION(STG_PREPARE);
 ^
include/common/initcall.h:134:1: note: to match this '('
DECLARE_INIT_SECTION(STG_PREPARE);
^
include/common/initcall.h:124:82: note: expanded from macro
'DECLARE_INIT_SECTION'
extern __attribute__((__weak__)) const struct
initcall *__start_init_##stg __asm("section$start$__DATA$" stg); \

   
^

-Patrick

Re: Compilation fails on OS-X

[Patrick Hemmer]

On 2019/2/13 09:40, Aleksandar Lazic wrote:
> Am 13.02.2019 um 14:45 schrieb Patrick Hemmer:
>> Trying to compile haproxy on my local machine for testing purposes and am
>> running into the following:
> Which compiler do you use?

# gcc -v
Configured with: 
--prefix=/Applications/Xcode.app/Contents/Developer/usr 
--with-gxx-include-dir=/usr/include/c++/4.2.1
Apple LLVM version 9.0.0 (clang-900.0.39.2)
Target: x86_64-apple-darwin17.7.0
Thread model: posix
InstalledDir: 
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin

>> # make TARGET=osx
>> src/proto_http.c:293:1: error: argument to 'section' attribute is not
>> valid for this target: mach-o section specifier requires a segment and 
>> section
>> separated by a comma
>> DECLARE_POOL(pool_head_http_txn, "http_txn", sizeof(struct 
>> http_txn));
>> ^
>> include/common/memory.h:128:2: note: expanded from macro 
>> 'DECLARE_POOL'
>> REGISTER_POOL(, name, size)
>> ^
>> include/common/memory.h:123:2: note: expanded from macro 
>> 'REGISTER_POOL'
>> INITCALL3(STG_POOL, create_pool_callback, (ptr), 
>> (name),
>> (size))
>> ^
>> include/common/initcall.h:102:2: note: expanded from macro 
>> 'INITCALL3'
>> _DECLARE_INITCALL(stage, __LINE__, function, arg1, 
>> arg2,
>> arg3)
>> ^
>> include/common/initcall.h:78:2: note: expanded from macro
>> '_DECLARE_INITCALL'
>> __DECLARE_INITCALL(__VA_ARGS__)
>> ^
>> include/common/initcall.h:65:42: note: expanded from macro
>> '__DECLARE_INITCALL'
>>
>> __attribute__((__used__,__section__("init_"#stg))) =   \
>>
>>
>>
>> Issue occurs on master, and the 1.9 branch
>>
>> -Patrick

Compilation fails on OS-X

[Patrick Hemmer]

Trying to compile haproxy on my local machine for testing purposes and
am running into the following:

# make TARGET=osx
src/proto_http.c:293:1: error: argument to 'section' attribute
is not valid for this target: mach-o section specifier requires a
segment and section separated by a comma
DECLARE_POOL(pool_head_http_txn, "http_txn", sizeof(struct
http_txn));
^
include/common/memory.h:128:2: note: expanded from macro
'DECLARE_POOL'
REGISTER_POOL(, name, size)
^
include/common/memory.h:123:2: note: expanded from macro
'REGISTER_POOL'
INITCALL3(STG_POOL, create_pool_callback, (ptr),
(name), (size))
^
include/common/initcall.h:102:2: note: expanded from macro
'INITCALL3'
_DECLARE_INITCALL(stage, __LINE__, function,
arg1, arg2, arg3)
^
include/common/initcall.h:78:2: note: expanded from macro
'_DECLARE_INITCALL'
__DECLARE_INITCALL(__VA_ARGS__)
^
include/common/initcall.h:65:42: note: expanded from macro
'__DECLARE_INITCALL'
   
__attribute__((__used__,__section__("init_"#stg))) =   \



Issue occurs on master, and the 1.9 branch

-Patrick

Re: Does anyone *really* use 51d or WURFL ?

[Patrick Hemmer]

On 2019/1/21 09:36, Willy Tarreau wrote:
> Hi all,
>
> recently it was figured that the buffer API changes caused some breakage
> to da.c and 51d.c (both fixed since), I don't know if wurfl builds at all
> by the way since the last update to the module is its introduction more
> than 2 years ago. But more importantly I'm realizing that neither 51d nor
> wurfl will start with threads enabled. This has been the case for about
> 15 months now, while we've had threads enabled on all relevant operating
> systems.
>
> Thus it leads me to the (possibly wrong) conclusion that absolutely
> nobody ever uses these options. Am I wrong ? I'm asking because each
> time we perform some API changes it's always a pain to go through these
> which don't build out of the box without external dependencies, and I
> suspect we're simply wasting our time and we should get rid of them
> if nobody uses them (or at the very least move them to contrib/).
>
> But if anyone is using them and disables threads for this, then it's
> fine, but in this case it would be nice if these two would check the
> thread safety of their code so that the thread restriction can be
> removed for the benefit of their users.
>
> Please advise.
>
> Thanks,
> Willy
>

We do use 51Degrees at my place of employment. However a couple of
caveats in that statement. One is that we're still running on 1.7. We'll
likely be upgrading to 1.9 soon, but still a couple months out.

The other caveat is that we run with threading disabled. Until the
statement on 'nbthread' that "THREADS SUPPORT IN HAPROXY IS HIGHLY
EXPERIMENTAL" goes away, we'll be leaving it off.

-Patrick

lua time tracking

[Patrick Hemmer]

What are the thoughts around putting time tracking stats around LUA
calls? Just really basic info like how long a request spent running LUA
code. Similar to how we already have metrics for time spent in queue,
connecting, waiting on response, etc.

Currently I accomplish this manually by grabbing the timestamp at the
beginning and end of all LUA actions, store the duration in a
transaction variable, and add that variable to the log. But I was
wondering if this should instead have a native solution.
I see we already have `tune.lua.service-timeout`, so it appears some
timing data is already tracked. However one difference between that data
and my custom implementation is that mine includes sleep time. So we
might want to expose the LUA time as 2 different metrics: one as CPU run
time, and another as wall clock time.

The use case is that as we put more code into LUA scripts, we want to be
aware of the impact this code is having on the performance of the
requests, and the response times.

-Patrick

Re: [PATCH 2/2] MINOR: Add srv_conn_free sample fetch

[Patrick Hemmer]

On 2018/8/22 04:05, Willy Tarreau wrote:
> On Thu, Aug 09, 2018 at 06:46:29PM -0400, Patrick Hemmer wrote:
>> This adds the 'srv_conn_free([/])' sample fetch. This fetch
>> provides the number of available connections on the designated server.
> Fine with this as well, though just like with the previous one, I
> disagree with this special case of -1 and would rather only count
> the really available connections (i.e. 0 if it's not possible to
> use the server).
>
> Willy

Adjusted from previous submission to handle dynamic maxconn, maxconn <
currconn, and cleanup documentation note.

-Patrick
From 2e3a908f229a1fcc11381c602aa131284b165a63 Mon Sep 17 00:00:00 2001
From: Patrick Hemmer 
Date: Thu, 14 Jun 2018 18:01:35 -0400
Subject: [PATCH] MINOR: Add srv_conn_free sample fetch

This adds the 'srv_conn_free([/])' sample fetch. This fetch
provides the number of available connections on the designated server.
---
 doc/configuration.txt | 19 ---
 src/backend.c | 28 
 2 files changed, 44 insertions(+), 3 deletions(-)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 6eec8c10b..513ef0c49 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -13690,7 +13690,8 @@ be_conn_free([]) : integer
   servers are also not included, unless all other servers are down. If no
   backend name is specified, the current one is used. But it is also possible
   to check another backend. It can be used to use a specific farm when the
-  nominal one is full. See also the "be_conn" and "connslots" criteria.
+  nominal one is full. See also the "be_conn", "connslots", and "srv_conn_free"
+  criteria.
 
   OTHER CAVEATS AND NOTES: if any of the server maxconn, or maxqueue is 0
   (meaning unlimited), then this fetch clearly does not make sense, in which
@@ -13908,8 +13909,20 @@ srv_conn([/]) : integer
   evaluated. If  is omitted, then the server is looked up in the
   current backend. It can be used to use a specific farm when one server is
   full, or to inform the server about our view of the number of active
-  connections with it. See also the "fe_conn", "be_conn" and "queue" fetch
-  methods.
+  connections with it. See also the "fe_conn", "be_conn", "queue", and
+  "srv_conn_free" fetch methods.
+
+srv_conn_free([/]) : integer
+  Returns an integer value corresponding to the number of available connections
+  on the designated server, possibly including the connection being evaluated.
+  The value does not include queue slots. If  is omitted, then the
+  server is looked up in the current backend. It can be used to use a specific
+  farm when one server is full, or to inform the server about our view of the
+  number of active connections with it. See also the "be_conn_free" and
+  "srv_conn" fetch methods.
+
+  OTHER CAVEATS AND NOTES: If the server maxconn is 0, then this fetch clearly
+  does not make sense, in which case the value returned will be -1.
 
 srv_is_up([/]) : boolean
   Returns true when the designated server is UP, and false when it is either
diff --git a/src/backend.c b/src/backend.c
index 01bd4b161..5a22b0fd0 100644
--- a/src/backend.c
+++ b/src/backend.c
@@ -1886,6 +1886,33 @@ smp_fetch_srv_conn(const struct arg *args, struct sample 
*smp, const char *kw, v
return 1;
 }
 
+/* set temp integer to the number of available connections on the server in 
the backend.
+ * Accepts exactly 1 argument. Argument is a server, other types will lead to
+ * undefined behaviour.
+ */
+static int
+smp_fetch_srv_conn_free(const struct arg *args, struct sample *smp, const char 
*kw, void *private)
+{
+   unsigned int maxconn;
+
+   smp->flags = SMP_F_VOL_TEST;
+   smp->data.type = SMP_T_SINT;
+
+   if (args->data.srv->maxconn == 0) {
+   /* one active server is unlimited, return -1 */
+   smp->data.u.sint = -1;
+   return 1;
+   }
+
+   maxconn = srv_dynamic_maxconn(args->data.srv);
+   if (maxconn > args->data.srv->cur_sess)
+   smp->data.u.sint = maxconn - args->data.srv->cur_sess;
+   else
+   smp->data.u.sint = 0;
+
+   return 1;
+}
+
 /* set temp integer to the number of connections pending in the server's queue.
  * Accepts exactly 1 argument. Argument is a server, other types will lead to
  * undefined behaviour.
@@ -1945,6 +1972,7 @@ static struct sample_fetch_kw_list smp_kws = {ILH, {
{ "nbsrv", smp_fetch_nbsrv,  ARG1(1,BE),  NULL, 
SMP_T_SINT, SMP_USE_INTRN, },
{ "queue", smp_fetch_queue_size, ARG1(1,BE),  NULL, 
SMP_T_SINT, SMP_USE_INTRN, },
{ "srv_conn",  smp_fetch_srv_conn,   ARG1(1,SRV), NULL, 
SMP_T_SINT,

Re: [PATCH 1/2] MINOR: add be_conn_free sample fetch

[Patrick Hemmer]

On 2018/8/25 01:30, Willy Tarreau wrote:
> On Fri, Aug 24, 2018 at 06:18:23PM -0400, Patrick Hemmer wrote:
>>> I disagree with making a special case above for maxconn 0. In fact for me
>>> it just means that such a server cannot accept connections, so it simply
>>> doesn't count in the sum, just as if it were not present.
>> On a server, maxconn=0 means unlimited, not that it can't accept
>> connections. If we return 0, how will the caller differentiate between
>> unlimited, and no free connections?
> Ah OK I didn't remember about this one, the it completely makes sense.
> Thanks for refreshing me on this one. It deserves a comment because it's
> not obvious ;-)  The current comment says the configuration is stupid
> while in fact it's just that this server is not limited. I think I
> still don't agree much with reporting -1 even if I'm the one having
> set it for connslots, which probably means I changed my mind regarding
> this. But I'm not seeing any better value that can easily be checked,
> so that's probably the least bad solution.
>
>> Also made 2 additional changes. One is to handle dynamic maxconn. The
>> other is to handle the case where the maxconn is adjusted (via stats
>> socket) to be less than the number of currently active connections,
>> which would result in the value wrapping.
> Good point. I'll adjust the doc then since it still says that it doesn't
> handle dynamic maxconn. Just let me know if you agree, and I'll do it
> myself to save you from having to respin a patch.
Sure, that's fine with me. Thanks :-)

>> Will update the srv_conn_free fetch with similar changes pending outcome
>> of this one.
> OK, thanks.
>
> Willy
I'll adjust the other sample fetch tomorrow.

-Patrick

Re: [PATCH 1/2] MINOR: add be_conn_free sample fetch

[Patrick Hemmer]

On 2018/8/22 04:04, Willy Tarreau wrote:
> Hi Patrick,
>
> On Thu, Aug 09, 2018 at 06:46:28PM -0400, Patrick Hemmer wrote:
>> This adds the sample fetch 'be_conn_free([])'. This sample fetch
>> provides the total number of unused connections across available servers
>> in the specified backend.
> Thanks for writing this one, I recently figured I needed the same for my
> build farm :-)
>
>> +be_conn_free([]) : integer
>> +  Returns an integer value corresponding to the number of available 
>> connections
>> +  across available servers in the backend. Queue slots are not included. 
>> Backup
>> +  servers are also not included, unless all other servers are down. If no
>> +  backend name is specified, the current one is used. But it is also 
>> possible
>> +  to check another backend. It can be used to use a specific farm when the
>> +  nominal one is full. See also the "be_conn" and "connslots" criteria.
>> +
>> +  OTHER CAVEATS AND NOTES: at this point in time, the code does not take 
>> care
>> +  of dynamic connections. Also, if any of the server maxconn, or maxqueue 
>> is 0,
>> +  then this fetch clearly does not make sense, in which case the value 
>> returned
>> +  will be -1.
> I disagree with making a special case above for maxconn 0. In fact for me
> it just means that such a server cannot accept connections, so it simply
> doesn't count in the sum, just as if it were not present.
On a server, maxconn=0 means unlimited, not that it can't accept
connections. If we return 0, how will the caller differentiate between
unlimited, and no free connections?
This is the same behavior provided by the `connslots` fetch, and is also
where I stole the doc snippet from.

>
>> +px = iterator->proxy;
>> +if (!srv_currently_usable(iterator) || ((iterator->flags & 
>> SRV_F_BACKUP) &&
>> +(px->srv_act ||
>> +(iterator != 
>> px->lbprm.fbck && !(px->options & PR_O_USE_ALL_BK)
>> +continue;
> Please slightly reorder the condition to improve the indent above for
> better readability, for example :
>
>   if (!srv_currently_usable(iterator) ||
>   ((iterator->flags & SRV_F_BACKUP) &&
>(px->srv_act || (iterator != px->lbprm.fbck && 
> !(px->options & PR_O_USE_ALL_BK)
>
> After checking, I'm OK with the condition :-)
>
>> +if (iterator->maxconn == 0) {
>> +/* configuration is stupid */
>> +smp->data.u.sint = -1;  /* FIXME: stupid value! */
>> +return 1;
>> +}
>> +
>> +smp->data.u.sint += (iterator->maxconn - iterator->cur_sess);
> Here I'd simply suggest this to replace the whole block :
>
>   if (iterator->maxconn > iterator->cur_sess)
>   smp->data.u.sint += (iterator->maxconn - 
> iterator->cur_sess);
>
> And then it can properly count available connections through all
> available servers, regardless of their individual configuration.
>
> Otherwise I'm fine with this patch.
>
> Thanks,
> Willy
Also made 2 additional changes. One is to handle dynamic maxconn. The
other is to handle the case where the maxconn is adjusted (via stats
socket) to be less than the number of currently active connections,
which would result in the value wrapping.

Will update the srv_conn_free fetch with similar changes pending outcome
of this one.

-Patrick
From 151d032b9cb396df47435742c03817818878f5af Mon Sep 17 00:00:00 2001
From: Patrick Hemmer 
Date: Thu, 14 Jun 2018 17:10:27 -0400
Subject: [PATCH 1/2] MINOR: add be_conn_free sample fetch

This adds the sample fetch 'be_conn_free([])'. This sample fetch
provides the total number of unused connections across available servers in the
specified backend.
---
 doc/configuration.txt | 15 ++-
 src/backend.c | 41 +
 2 files changed, 55 insertions(+), 1 deletion(-)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 6e33f5994..f65efce95 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -13682,7 +13682,20 @@ be_conn([]) : integer
   possibly including the connection being evaluated. If no backend name is
   specified, the current one is used. But it is also possible to check another
   backend. It can be used to use a specific farm when the nominal one is full.
-  See also the "fe_conn", "queue" and "be_sess_rate" criteria.
+  

[PATCH] MEDIUM: reset lua transaction between http requests

[Patrick Hemmer]

Not sure if this is the right approach, but this addresses the issue for me.
This should be backported to 1.8.

-Patrick
From 9087400de99a3925380cac4128a431cd48a09145 Mon Sep 17 00:00:00 2001
From: Patrick Hemmer 
Date: Wed, 22 Aug 2018 10:02:00 -0400
Subject: [PATCH] MEDIUM: reset lua transaction between http requests

Previously LUA code would maintain the transaction state between http requests,
resulting in things like txn:get_priv() retrieving data from a previous requst.
This addresses the issue by ensuring the LUA state is reset between requests.
---
 src/proto_http.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/proto_http.c b/src/proto_http.c
index a7a8dadd6..93c8857cd 100644
--- a/src/proto_http.c
+++ b/src/proto_http.c
@@ -4460,6 +4460,9 @@ void http_end_txn_clean_session(struct stream *s)
s->flags &= ~(SF_CURR_SESS|SF_REDIRECTABLE|SF_SRV_REUSED);
s->flags &= ~(SF_ERR_MASK|SF_FINST_MASK|SF_REDISP);
 
+   hlua_ctx_destroy(s->hlua);
+   s->hlua = NULL;
+
s->txn->meth = 0;
http_reset_txn(s);
s->txn->flags |= TX_NOT_FIRST | TX_WAIT_NEXT_RQ;
-- 
2.18.0

Re: BUG: LUA txn:get_priv() scoped to connection, not transaction

[Patrick Hemmer]

On 2018/8/22 05:16, Thierry Fournier wrote:
> Hi Patrick,
>
> Could you retry adding the keyword “local” before data. Unfortunately,
> by default, Lua variables are global.
>
Makes no difference, still get the same result. I don't think it would
do anything anyway as the `txn:get_priv()` will still return a value,
even if nil, and overwrite whatever is in a previous definition.

>
>> core.register_action("test", { "http-req" }, function(txn)
>> *local*data = txn:get_priv()
>> if not data then
>> data = 0
>> end
>> data = data + 1
>> print(string.format("set to %d", data))
>>     txn:set_priv(data)
>> end)
>
> BR,
> Thierry
>
>
>> On 22 Aug 2018, at 05:57, Patrick Hemmer > <mailto:hapr...@stormcloud9.net>> wrote:
>>
>> There is a bug in the current stable haproxy (1.8.13) where the LUA
>> function txn:get_priv() is returning data stored from other
>> transactions. This was discovered as we have code that triggers on
>> certain requests, and it was triggering on requests it should not
>> have been.
>>
>> You can reproduce with this config:
>> global
>> lua-load haproxy.lua
>>
>> defaults
>> mode http
>>
>> frontend f1
>> bind :8000
>> default_backend b1
>> http-request lua.test
>>
>> backend b1
>> http-request use-service lua.fakeserv
>>
>> And this lua file:
>> core.register_action("test", { "http-req" }, function(txn)
>> data = txn:get_priv()
>> if not data then
>> data = 0
>> end
>> data = data + 1
>> print(string.format("set to %d", data))
>> txn:set_priv(data)
>> end)
>>
>> core.register_service("fakeserv", "http", function(applet)
>> applet:set_status(200)
>> applet:start_response()
>> end)
>>
>> And this curl command:
>> curl http://localhost:8000 http://localhost:8000
>>
>> Which provides this output:
>> set to 1
>> set to 2
>>
>>
>>
>> Version information:
>> HA-Proxy version 1.8.13 2018/07/30
>> Copyright 2000-2018 Willy Tarreau 
>>
>> Build options :
>> TARGET  = osx
>> CPU = generic
>> CC  = gcc
>> CFLAGS  = -O0 -g -fno-strict-aliasing
>> -Wdeclaration-after-statement -fwrapv -fno-strict-overflow
>> -Wno-address-of-packed-member -Wno-null-dereference -Wno-unused-label
>> OPTIONS = USE_ZLIB=1 USE_OPENSSL=1 USE_LUA=1 USE_PCRE=1
>>
>> Default settings :
>> maxconn = 2000, bufsize = 16384, maxrewrite = 1024,
>> maxpollevents = 200
>>
>> Built with OpenSSL version : OpenSSL 1.1.0h  27 Mar 2018
>> Running on OpenSSL version : OpenSSL 1.1.0h  27 Mar 2018
>> OpenSSL library supports TLS extensions : yes
>> OpenSSL library supports SNI : yes
>> OpenSSL library supports : TLSv1.0 TLSv1.1 TLSv1.2
>> Built with Lua version : Lua 5.3.4
>> Built with transparent proxy support using:
>> Encrypted password support via crypt(3): yes
>> Built with PCRE version : 8.42 2018-03-20
>> Running on PCRE version : 8.42 2018-03-20
>> PCRE library supports JIT : no (USE_PCRE_JIT not set)
>> Built with zlib version : 1.2.11
>> Running on zlib version : 1.2.11
>> Compression algorithms supported : identity("identity"),
>> deflate("deflate"), raw-deflate("deflate"), gzip("gzip")
>> Built with network namespace support.
>>
>> Available polling systems :
>>  kqueue : pref=300,  test result OK
>>  poll : pref=200,  test result OK
>>  select : pref=150,  test result OK
>> Total: 3 (3 usable), will use kqueue.
>>
>> Available filters :
>> [SPOE] spoe
>> [COMP] compression
>> [TRACE] trace
>>
>>
>> -Patrick
>

Re: connection leak (stuck in CLOSE_WAIT) on master

[Patrick Hemmer]

On 2018/8/9 01:01, Patrick Hemmer wrote:
> There's an issue on current master (287527a) where haproxy is losing
> track of its connections, and they're getting stuck in CLOSE_WAIT. And
> it's counting these connections towards limits (such as maxconn).
> Eventually maxconn is reached and everything starts queuing, and
> timing out.
>
> Here's an example:
> # lsof -p 25520
> COMMAND   PIDUSER   FD TYPE DEVICE  
> SIZE/OFFNODE NAME
> 
> haproxy 25520 phemmer0u CHR   16,5
> 0x1409380c1797 /dev/ttys005
> haproxy 25520 phemmer1u CHR   16,5
> 0x1409380c1797 /dev/ttys005
> haproxy 25520 phemmer2u CHR   16,5
> 0x1409380c1797 /dev/ttys005
> haproxy 25520 phemmer3u 
> KQUEUE   count=0, state=0xa
> haproxy 25520 phemmer4uunix 0x933b1c4c9ed396b7   
> 0t0 /tmp/haproxy.sock.25520.tmp
> haproxy 25520 phemmer5uIPv4 0x933b1c4cbf5fc73f   
> 0t0 TCP *:8001 (LISTEN)
> haproxy 25520 phemmer6uIPv4 0x933b1c4c925fe437   
> 0t0 UDP *:51977
> haproxy 25520 phemmer7 PIPE 0x933b1c4c9f89c457 
> 16384 ->0x933b1c4c9f89bf17
> haproxy 25520 phemmer8 PIPE 0x933b1c4c9f89bf17 
> 16384 ->0x933b1c4c9f89c457
> haproxy 25520 phemmer9uunix 0x933b1c4c9548c96f   
> 0t0 /tmp/haproxy.sock.25520.tmp
> haproxy 25520 phemmer   11uIPv4 0x933b1c4cc0cca73f   
> 0t0 TCP 127.0.0.1:8001->127.0.0.1:59199 (CLOSE_WAIT)
> haproxy 25520 phemmer   12uIPv4 0x933b1c4cbfd20ddf   
> 0t0 TCP 127.0.0.1:59200->127.0.0.1:8081 (CLOSE_WAIT)
>
>
> FD 11 was the client making the request to haproxy, and FD 12 was
> haproxy to the server.
>
> > show fd
> 4 : st=0x05(R:PrA W:pra) ev=0x01(heopI) [lc]
> cnext=-13 cprev=-2 tmask=0x1 umask=0x0 owner=0x7f852af08430
> iocb=0x10bb89c40(listener_accept) l.st=RDY fe=GLOBAL
> 5 : st=0x05(R:PrA W:pra) ev=0x01(heopI) [lc]
> cnext=-13 cprev=-2 tmask=0x1 umask=0x0 owner=0x7f852af08a40
> iocb=0x10bb89c40(listener_accept) l.st=RDY fe=f1
> 7 : st=0x05(R:PrA W:pra) ev=0x00(heopi) [lc]
> cnext=-3 cprev=0 tmask=0x1 umask=0x0 owner=0x10bbc2ca0
> iocb=0x10bbc2ca0(poller_pipe_io_handler)
> 9 : st=0x20(R:pra W:pRa) ev=0x00(heopi) [lc]
> cnext=-3 cprev=-2 tmask=0x1 umask=0x1 owner=0x7f852ae14550
> iocb=0x10bba60a0(conn_fd_handler) cflg=0x00201300 fe=GLOBAL mux=PASS
> mux_ctx=0x7f852ca0
>  11 : st=0x22(R:pRa W:pRa) ev=0x10(Heopi) [lc]
> cnext=-3 cprev=-2 tmask=0x1 umask=0x0 owner=0x7f852ae13ef0
> iocb=0x10bba60a0(conn_fd_handler) cflg=0x80241300 fe=f1 mux=PASS
> mux_ctx=0x7f852ae14080
>  12 : st=0x25(R:PrA W:pRa) ev=0x00(heopi) [Lc]
> cnext=-17 cprev=-2 tmask=0x1 umask=0x0 owner=0x7f852ca000c0
> iocb=0x10bba60a0(conn_fd_handler) cflg=0x00202306 sv=s1/b1 mux=PASS
> mux_ctx=0x7f852ae14bb0
>
> > show sess
> 0x7f852ae14140: proto=tcpv4 src=127.0.0.1:59199 fe=f1 be=b1
> srv=s1 ts=00 age=2m9s calls=6 rq[f=8840020h,i=0,an=8000h,rx=,wx=,ax=]
> rp[f=8040h,i=0,an=a0h,rx=,wx=,ax=] s0=[7,8h,fd=11,ex=]
> s1=[7,118h,fd=12,ex=] exp=
> 0x7f852ae14830: proto=unix_stream src=unix:1 fe=GLOBAL
> be= srv= ts=00 age=20s calls=3
> rq[f=40c08202h,i=0,an=00h,rx=,wx=,ax=]
> rp[f=c0048202h,i=0,an=00h,rx=,wx=,ax=] s0=[7,ch,fd=9,ex=]
> s1=[7,4018h,fd=-1,ex=] exp=23h59m
>
>
>
> Here's the config I'm using:
> global
> stats socket /tmp/haproxy.sock level admin
> defaults
> log 127.0.0.1:1234 daemon
> mode http
> option httplog
> timeout queue 5s
> frontend f1
> bind :8001
> default_backend b1
> backend b1
> server s1 127.0.0.1:8081 maxconn 1
>
>
> Log output around the time it broke:
> <30>Aug  9 00:45:56 haproxy[25520]: 127.0.0.1:59186
> [09/Aug/2018:00:45:56.620] f1 b1/s1 0/167/1/107/275 200 121 - - 
> 2/2/1/1/0 0/2 "GET / HTTP/1.1"
> <30>Aug  9 00:45:57 haproxy[25520]: 127.0.0.1:59189
> [09/Aug/2018:00:45:56.726] f1 b1/s1 0/169/1/108/278 200 121 - - 
> 2/2/1/1/0 0/2 "GET / HTTP/1.1"
> <30>Aug  9 00:45:57 haproxy[25520]: 127.0.0.1:59193
> [09/Aug/2018:00:45:56.948] f1 b1/s1 0/56/1/109/167 200 121 - - 
> 2/2/1/1/0 0/1 "GET / HTTP/1.1"
> <30>Aug  9 00:45:57 haproxy[25520]: 127.0.0.1:59196
> [09/A

Re: haproxy processing request of a disconnected client

[Patrick Hemmer]

On 2018/8/9 13:00, Patrick Hemmer wrote:
> So I just noticed the behavior that when a request is queued and the
> client closes the connection, once a server slot frees up that request
> is still sent to the server which processes it and sends a response back.
> What's even more interesting is that the log indicates that everything
> was processed normally. It basically says the response was sent back
> to the client just fine.
>
> Example config:
> global
> stats socket /tmp/haproxy.sock level admin
> defaults
> log 127.0.0.1:1234 daemon
> mode http
> option httplog
> timeout queue 5s
> frontend f1
> bind :8001
> default_backend b1
> backend b1
> server s1 127.0.0.1:8081 maxconn 1
>
> Log output:
> <30>Aug  9 12:50:40 haproxy[12384]: 127.0.0.1:64723
> [09/Aug/2018:12:50:35.167] f1 b1/s1 0/0/0/5106/5106 200 118 - - 
> 2/2/1/1/0 0/0 "GET /y HTTP/1.1"
> <30>Aug  9 12:50:45 haproxy[12384]: 127.0.0.1:64726
> [09/Aug/2018:12:50:35.526] f1 b1/s1 0/4749/0/5102/9851 200 118 - -
>  1/1/0/1/0 0/1 "GET /x HTTP/1.1"
>
> ^ In this, the server sleeps for 5 seconds, and then replies. I sent
> the request for /y, and then a request for /x, but killed the client
> on the /x request after 1 second. The request for /y was processed,
> but then so was the request for /x. The close flags ("") indicate
> everything went fine.
>
> Information:
>
> lsof:
> COMMAND   PIDUSER   FD TYPE DEVICE  
> SIZE/OFFNODE NAME
> ...
> haproxy 12384 phemmer0u CHR   16,5
> 0x140daf691797 /dev/ttys005
> haproxy 12384 phemmer1u CHR   16,5
> 0x140daf691797 /dev/ttys005
> haproxy 12384 phemmer2u CHR   16,5
> 0x140daf691797 /dev/ttys005
> haproxy 12384 phemmer3u 
> KQUEUE   count=0, state=0xa
> haproxy 12384 phemmer4uunix 0x933b1c4c8f54438f   
> 0t0 /tmp/haproxy.sock.12384.tmp
> haproxy 12384 phemmer5uIPv4 0x933b1c4cc16309ff   
> 0t0 TCP *:8001 (LISTEN)
> haproxy 12384 phemmer6uIPv4 0x933b1c4c8fef8977   
> 0t0 UDP *:62112
> haproxy 12384 phemmer7uIPv4 0x933b1c4cbf5fd9ff   
> 0t0 TCP 127.0.0.1:8001->127.0.0.1:64723 (ESTABLISHED)
> haproxy 12384 phemmer8uIPv4 0x933b1c4cc1546ddf   
> 0t0 TCP 127.0.0.1:64724->127.0.0.1:8081 (ESTABLISHED)
> haproxy 12384 phemmer9uIPv4 0x933b1c4cc1c6209f   
> 0t0 TCP 127.0.0.1:8001->127.0.0.1:64726 (CLOSE_WAIT)
>
> admin socket 'show fd':
>   4 : st=0x05(R:PrA W:pra) ev=0x01(heopI) [nlc] cache=0
> owner=0x7f93f3705010 iocb=0x107f82c40(listener_accept)
> tmask=0x umask=0xfffe l.st=RDY fe=GLOBAL
>   5 : st=0x05(R:PrA W:pra) ev=0x01(heopI) [nlc] cache=0
> owner=0x7f93f3705650 iocb=0x107f82c40(listener_accept)
> tmask=0x umask=0xfffe l.st=RDY fe=f1
>   7 : st=0x25(R:PrA W:pRa) ev=0x00(heopi) [nlc] cache=0
> owner=0x7f93f3708280 iocb=0x107f9e2e0(conn_fd_handler) tmask=0x1
> umask=0x0 cflg=0x80201306 fe=f1 mux=PASS mux_ctx=0x7f93f3708490
>   8 : st=0x25(R:PrA W:pRa) ev=0x00(heopi) [nLc] cache=0
> owner=0x7f93f340ede0 iocb=0x107f9e2e0(conn_fd_handler) tmask=0x1
> umask=0x0 cflg=0x00202306 sv=s1/b1 mux=PASS mux_ctx=0x7f93f350d0e0
>   9 : st=0x22(R:pRa W:pRa) ev=0x10(Heopi) [nlc] cache=0
> owner=0x7f93f510 iocb=0x107f9e2e0(conn_fd_handler) tmask=0x1
> umask=0x0 cflg=0x80241300 fe=f1 mux=PASS mux_ctx=0x7f93f5100210
>  10 : st=0x25(R:PrA W:pRa) ev=0x00(heopi) [nlc] cache=0
> owner=0x7f93f3708100 iocb=0x107f9e2e0(conn_fd_handler) tmask=0x1
> umask=0x0 cflg=0x00201306 fe=GLOBAL mux=PASS mux_ctx=0x7f93f350d1a0
>
> admin socket 'show sess':
> 0x7f93f37084c0: proto=tcpv4 src=127.0.0.1:64723 fe=f1 be=b1 srv=s1
> ts=08 age=2s calls=3 rq[f=48840200h,i=0,an=8000h,rx=,wx=,ax=]
> rp[f=8040h,i=0,an=a0h,rx=,wx=,ax=] s0=[7,8h,fd=7,ex=]
> s1=[7,118h,fd=8,ex=] exp=
> 0x7f93f5100240: proto=tcpv4 src=127.0.0.1:64726 fe=f1 be=b1
> srv= ts=08 age=2s calls=3
> rq[f=c800020h,i=0,an=8000h,rx=,wx=,ax=]
> rp[f=8000h,i=0,an=00h,rx=,wx=,ax=] s0=[7,8h,fd=9,ex=]
> s1=[2,110h,fd=-1,ex=2s] exp=2s
> 0x7f93f350d1d0: proto=unix_stream src=unix:1 fe=GLOBAL be=
> srv= ts=02 age=0s calls=1
> rq[f=40c08202h,i=0,an=00h,rx=10s,wx=,ax=]
> rp[f=80008002h,i=0,an=00h,rx=,wx=,ax=] s0=[7,8h,fd=10,ex=]
> s1=[7,4018h,fd=-1,ex=] exp=10s
>
> Version info:
> # ./haproxy -vv
> HA-Proxy version 1.8.13 2018/07/30
> Copyright 2000-2018 Will

Re: [PATCH 0/2] sample fetches for available connections

[Patrick Hemmer]

On 2018/8/9 18:46, Patrick Hemmer wrote:
> These are 2 new sample fetches which provide the available connections.
> The be_conn_free fetch is similar to connslots, but has significant
> difference in that it does not count queue slots, nor backup servers
> (unless all servers are down).
>
> These are intended to be useful in combination with the priority
> queuing, so you can see how many connections are available, perhaps for
> taking action when the number is low. For example by reserving
> connections for high-priority requests, and rejecting low priority ones.
>
> -Patrick
>
> Patrick Hemmer (2):
>   MINOR: add be_conn_free sample fetch
>   MINOR: Add srv_conn_free sample fetch
>
>  doc/configuration.txt | 34 +---
>  src/backend.c | 60 +++
>  2 files changed, 91 insertions(+), 3 deletions(-)
>

Is there any interest in this?

Sorry for all the pings, lots of emails which look like they might have
fallen off the radar.

-Patrick

Re: [PATCH] MINOR: crypto: Add digest and hmac converters

[Patrick Hemmer]

On 2018/6/17 14:02, Baptiste wrote:
>
>
> Le dim. 17 juin 2018 à 14:10, Patrick Gansterer  > a écrit :
>
>
> > On 17 Jun 2018, at 13:36, Baptiste  > wrote:
> >
> > Can they be used to validate oauth tokens too?
>
> Depends on the implementation of the tokens, but if they are
> HMACSHA256 signed JWT, it’s very easy to validate them in a lua
> script now.
>
>
> That's how I am doing it now.
> Just wanted to know if a combination of http rules could do the job.
> And be maybe faster.
>
> Baptiste

Any progress on this? I'd also be interested in seeing native HMAC
functions added, as I'm doing something similar to a HMAC that I'd love
to replace.

-Patrick

Re: BUG: Tw is negative with lua sleep

[Patrick Hemmer]

On 2018/7/18 09:03, Frederic Lecaille wrote:
> Hello Patrick,
>
> On 07/17/2018 03:59 PM, Patrick Hemmer wrote:
>> Ping?
>>
>> -Patrick
>>
>> On 2018/6/22 15:10, Patrick Hemmer wrote:
>>> When using core.msleep in lua, the %Tw metric is a negative value.
>>>
>>> For example with the following config:
>>> haproxy.cfg:
>>> global
>>> lua-load /tmp/haproxy.lua
>>>
>>> frontend f1
>>> mode http
>>> bind :8000
>>> default_backend b1
>>> log 127.0.0.1:1234 daemon
>>> log-format Ta=%Ta\ Tc=%Tc\ Td=%Td\ Th=%Th\ Ti=%Ti\
>>> Tq=%Tq\ TR=%TR\ Tr=%Tr\ Tt=%Tt\ Tw=%Tw
>>>
>>> backend b1
>>> mode http
>>> http-request use-service lua.foo
>>>
>>> haproxy.lua:
>>> core.register_service("foo", "http", function(applet)
>>> core.msleep(100)
>>> applet:set_status(200)
>>> applet:start_response()
>>> end)
>>>
>>> The log contains:
>>> Ta=104 Tc=0 Td=0 Th=0 Ti=0 Tq=104 TR=104 Tr=104 Tt=104 Tw=-104
>>>
>>> ^ TR also looks wrong, as it did not take 104ms to receive the full
>>> request.
>>>
>>> This is built from the commit before current master: d8fd2af
>>>
>>> -Patrick
>>
>
> The patch attached to this mail fixes this issue at least for %TR field.
>
> But I am not sure at all it is correct or if there is no remaining
> issues. For instance the LUA tcp callback also updates the tv_request
> log field.
>
> So, let's wait for Thierry's validation.
>
> Regards.
>

Any update on this?

-Patrick

BUG: LUA txn:get_priv() scoped to connection, not transaction

[Patrick Hemmer]

There is a bug in the current stable haproxy (1.8.13) where the LUA
function txn:get_priv() is returning data stored from other
transactions. This was discovered as we have code that triggers on
certain requests, and it was triggering on requests it should not have been.

You can reproduce with this config:
global
lua-load haproxy.lua

defaults
mode http

frontend f1
bind :8000
default_backend b1
http-request lua.test

backend b1
http-request use-service lua.fakeserv

And this lua file:
core.register_action("test", { "http-req" }, function(txn)
data = txn:get_priv()
if not data then
data = 0
end
data = data + 1
print(string.format("set to %d", data))
txn:set_priv(data)
end)

core.register_service("fakeserv", "http", function(applet)
applet:set_status(200)
applet:start_response()
end)

And this curl command:
curl http://localhost:8000 http://localhost:8000

Which provides this output:
set to 1
set to 2



Version information:
HA-Proxy version 1.8.13 2018/07/30
Copyright 2000-2018 Willy Tarreau 

Build options :
TARGET  = osx
CPU = generic
CC  = gcc
CFLAGS  = -O0 -g -fno-strict-aliasing
-Wdeclaration-after-statement -fwrapv -fno-strict-overflow
-Wno-address-of-packed-member -Wno-null-dereference -Wno-unused-label
OPTIONS = USE_ZLIB=1 USE_OPENSSL=1 USE_LUA=1 USE_PCRE=1

Default settings :
maxconn = 2000, bufsize = 16384, maxrewrite = 1024,
maxpollevents = 200

Built with OpenSSL version : OpenSSL 1.1.0h  27 Mar 2018
Running on OpenSSL version : OpenSSL 1.1.0h  27 Mar 2018
OpenSSL library supports TLS extensions : yes
OpenSSL library supports SNI : yes
OpenSSL library supports : TLSv1.0 TLSv1.1 TLSv1.2
Built with Lua version : Lua 5.3.4
Built with transparent proxy support using:
Encrypted password support via crypt(3): yes
Built with PCRE version : 8.42 2018-03-20
Running on PCRE version : 8.42 2018-03-20
PCRE library supports JIT : no (USE_PCRE_JIT not set)
Built with zlib version : 1.2.11
Running on zlib version : 1.2.11
Compression algorithms supported : identity("identity"),
deflate("deflate"), raw-deflate("deflate"), gzip("gzip")
Built with network namespace support.

Available polling systems :
 kqueue : pref=300,  test result OK
 poll : pref=200,  test result OK
 select : pref=150,  test result OK
Total: 3 (3 usable), will use kqueue.

Available filters :
[SPOE] spoe
[COMP] compression
[TRACE] trace


-Patrick

[PATCH] DOC: add documentation for prio_class and prio_offset sample fetches.

[Patrick Hemmer]

This adds documentation that was missed as part of 268a707.
---
 doc/configuration.txt | 11 +++
 1 file changed, 11 insertions(+)


diff --git a/doc/configuration.txt b/doc/configuration.txt
index 48b69a5bd..d11b63185 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -13857,6 +13857,17 @@ nbsrv([]) : integer
   to handle some load. It is useful to report a failure when combined with
   "monitor fail".
 
+prio_class : integer
+  Returns the priority class of the current session for http mode or connection
+  for tcp mode. The value will be that set by the last call to "http-request
+  set-priority-class" or "tcp-request content set-priority-class".
+
+prio_offset : integer
+  Returns the priority offset of the current session for http mode or
+  connection for tcp mode. The value will be that set by the last call to
+  "http-request set-priority-offset" or "tcp-request content
+  set-priority-offset".
+
 proc : integer
   Returns an integer value corresponding to the position of the process calling
   the function, between 1 and global.nbproc. This is useful for logging and

Re: [PATCH 1/2] MEDIUM: add set-priority-class and set-priority-offset

[Patrick Hemmer]

On 2018/8/13 09:17, Aleksandar Lazic wrote:
> On 11/08/2018 14:48, Patrick Hemmer wrote:
>>
>
> [snipp]
>
>> To answer one of the earlier questions, I do plan on writing a blog
>> article yes. The question is when. I'm considering backporting this
>> feature to 1.8 for my own use at work, and if done, might wait for that
>> so I have some real world usage to comment on.
>
> That sounds very good. Due to the fact that this is a new feature I'm
> not sure if it will be back ported. It's not my decision but I think
> this would be a nice feature in 1.8 also.
It will not be officially backported to 1.8. The backport will be done
by me personally for my own use.

>
>> Willy's example use cases were spot on. But to add the use case which
>> triggered me to write this code: The intent is more for layer 7 DOS
>> mitigation. It's really hard to correctly identify bots vs real users.
>> You can usually get it right, but mis-identification is very easy. So
>> the idea here is that we would add a score to incoming requests based on
>> things like the user having a cookie, are they a registered user,
>> request rate, etc. Similar to how email spam filters work. Each one of
>> these things would increment or decrement the score, and then they would
>> be queued based on the result. Then when we do have a L7 attack, we only
>> give compute resources to the attackers when there are no real users in
>> the queue. Thus the users might see some slowdown, but it should be
>> minimal. And since we're not actually blocking an attacker, it makes it
>> much harder for them to figure out the criteria we're using to identify
>> them and get around it. And also since we're not blocking, users which
>> end up mis-identified as bots aren't impacted during normal operations,
>> only when we're under attack. And even then it should be minimal since
>> while a user might have triggered a few of the score rules, the bot
>> would have hopefully triggered more.
>
> Full ack here. This is a difficult topic which can create a lot of
> headache.
>
> Do you think to use one of the sample like DeviceAtlas or 51 or a more
> specific solution?
No. DeviceAtlas and 51degrees are not suitable for this task. All these
libraries do is analyze the UserAgent header (which is trivially easy to
fake). They do not do behavior analysis.

>
> Do you think about to use SPOE for the detection to offload the check
> mechanism?
SPOE can be utilized yes. There are many other services which can be
utilized as well, even some which have official integration with haproxy
such as Datadome <https://docs.datadome.co/docs/haproxy-setup>. The
queuing mechanism is meant to be the action taken upon detection, while
the detection itself can be anything, and is left up to the user. But
the configuration will need to be tailored to the system utilizing it.

>
> If this question / answers are some business secret then I fully
> understand when you can't answer it.
>
>> -Patrick
>
> Regards
> Aleks

Re: [PATCH 1/2] MEDIUM: add set-priority-class and set-priority-offset

[Patrick Hemmer]

On 2018/8/11 11:25, Aleksandar Lazic wrote:
> Hi Willy.
>
>
> On 11/08/2018 11:04, Willy Tarreau wrote:
>> Hi Aleks,
>>
>> On Sat, Aug 11, 2018 at 10:27:42AM +0200, Aleksandar Lazic wrote:
>>> I think it is a requeue mechanism for the connection and not a
>>> selector for
>>> a server or a backend, but I can be wrong.
>>
>> That's it. I'll try to explain shortly. You're probably aware how the
>> queuing mechanism works in haproxy when maxconn is set on a server :
>> once maxconn is reached, requests are queue either into the server's
>> queue, if something forces this request to use this specific server
>> (cookie, ...) otherwise into the backend's queue. Till now, our queues
>> were very fair (it was already shown on some blog posts from various
>> users over the last decade). A server would always consult both its
>> own queue and the backend's queue and pick the oldest request so that
>> the order of arrival was always preserved.
>>
>> Now with this new feature, it is possible to change the dequeuing
>> order when a server picks a request from the queues. There are two
>> criteria for this :
>>  - the request's class
>>  - the position in the queue for a given class.
>>
>> The server will always consult classes with lowest value first. And
>> within this class, it will pick the oldest waiting request. The
>> "set-priority-class" action allows you to set the class, and mark a
>> request as very important or unimportant. A good rule of thumb is to
>> consider that rare but important requests should be given a low class
>> number, that common requests should be given the default class number
>> (unchanged) and that expensive and painful requests which are not very
>> urgent should be given a high class number (dequeued only once no other
>> request competes with them). The "set-priority-offset" action allows to
>> tweak the apparent age of the request in the queue so that a server can
>> pick a request before another one for a given class. This is not used
>> to deal with the request's importance, but with the preferred response
>> time. For example, if your site periodically refreshes the pages but
>> cannot start to render before all objects are loaded, it could happen
>> that the browser clears the page when reloading the HTML part, then
>> takes ages to fetch css, js, etc, making the page unreadable for several
>> seconds every minute for a reader. By applying a time offset to these
>> extra components, you will make the mandatory elements such as CSS or JS
>> load much faster than the HTML one, resulting in an extra delay before
>> starting to fetch the HTML part (hence the page is not yet cleared), and
>> a shorter delay for the CSS/JS parts (making it render faster once
>> cleared).
>> Similarly you could be a hosting provider willing to offer certain
>> guarantees on the page load time to certain customers. With this you
>> could easily say that those paying for premium service are guaranteed
>> to see a 100 ms faster load time than those paying the entry level one,
>> simply by applying a -100ms offset on their requests.
>>
>> Some people want to use such features the other way around. Certain
>> sites have low importance stuff like an avatar upload button, a profile
>> settings page, etc, on which people "waste their time" instead of making
>> use of the main site, but still making use of the bandwidth and
>> processing
>> power. Sometimes they simply want to say that such auxiliary requests
>> are
>> applied an extra delay when there's competition to reach the servers.
>> Just
>> by adding an extra 1 second offset to such requests, you'll slow them
>> down
>> when the servers are highly loaded, and preserve the resources for the
>> other requests.
>>
>> It's really impressive when you run two load generators in parallel. You
>> will typically see, say, 100 ms response time for both of them by
>> default.
>> Then you apply +100ms to the requests coming from one of them, and then
>> you'll see a distribution like 66/166ms with the load of the slow one
>> dropping as its response time increases, giving more resources to the
>> other one, allowing its traffic to be processed faster.
>>
>> I hope I helped clear your doubts.
>
> Men! yes. As always a super good explanation ;-)
>
>> I'm having a few comments below :
>>
>>> listen|frontend test001
>>>   bind :8080
>>>   # some other options
>>>   http-request set-priority-class -10s if PAY_BUTTON
>>
>> The class just uses integers, not delays. Also it's an interesting case
>> you have here, as many sites prefer to *increase* the delay on the pay
>> button or even once there's anything in your cart. There's a reason :
>> visitors who don't find what they're looking for are likely to quit the
>> site, so you need to improve their experience while they're searching.
>> Once their shopping cart is filled with a few articles, they're not
>> willing to spend as much time looking for them again on another site,
>> so they're very likely to accept to wait 

Re: [PATCH 1/2] MEDIUM: add set-priority-class and set-priority-offset

[Patrick Hemmer]

On 2018/8/10 09:19, Willy Tarreau wrote:
> Hi Patrick,
>
> On Thu, Aug 09, 2018 at 06:29:33PM -0400, Patrick Hemmer wrote:
>> I also went and removed the queue position counter code from
>> stream_process_counters(), and the logging still appears to work fine
>> (but I could have easily missed some potential scenarios).
> OK nice!
>
>> In regards to the idea you added to the commit message on the queue index
>>> It could even be further improved to avoid manipulating the stream
>>> from the queue :
>>>   - queue_idx = initial position when enqueuing
>>>   - queue_idx = measured position when dequeuing
>> Because the stream can pass through multiple queues, we'd have to make
>> sure that every time we de-queue, that the stream code pulls the value
>> and increments itself. However looking at all the various places
>> pendconn_unlink() gets called, I think it would be difficult for the
>> stream code to know when the value needs to be pulled.
> In fact the only two ways to be queued multiple times are :
>   - get redispatched after a failed connection attempt to a server ; in
> this case the pendconn was already dequeued to connect to the first
> server so the state is unambigous
>
>   - being redistributed when a pendconn was lying in a server's queue
> and the server is marked down. In this case the pendconn is simply
> unlinked, process_stream is woken up and I don't know if we collect
> queue_idx during the call to process_stream before queuing again.
>
> Anyway this is very minor, I'd even say cosmetic. I'm pretty fine with
> the current state already!
>
>> Anyway, attached is the latest patch set for review again.
> I've applied it to a local temporary branch and am willing to merge it
> if you're OK. I've only adjusted 3 minor things :
>   - removed the warning in the 3rd patch's commit message mentioning
> the missing calls to pendconn_unlink() before logs, because these
> ones were finally moved to patch 1 but we didn't edit this message ;
>
>   - removed the now unused queue_idx from stream_process_counters()
> which emits a warning since it's unused
>
>   - removed the "__decl_hathreads(HA_SPINLOCK_T lock);" from struct
> pendconn that is a leftover from the rebase onto the last queue
> updates.
>
> So please just let me know, I'm ready ;-)
>
> Thanks,
> Willy
Those changes seem good to me.
Merge it!
Quite happy to see this done with. Any issues can be addressed as bug fixes.

Thanks

-Patrick

[PATCH 1/2] MINOR: add be_conn_free sample fetch

[Patrick Hemmer]

This adds the sample fetch 'be_conn_free([])'. This sample fetch
provides the total number of unused connections across available servers
in the
specified backend.
---
 doc/configuration.txt | 15 ++-
 src/backend.c | 38 ++
 2 files changed, 52 insertions(+), 1 deletion(-)


diff --git a/doc/configuration.txt b/doc/configuration.txt
index 48b69a5bd..b5c093e15 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -13682,7 +13682,20 @@ be_conn([]) : integer
   possibly including the connection being evaluated. If no backend name is
   specified, the current one is used. But it is also possible to check another
   backend. It can be used to use a specific farm when the nominal one is full.
-  See also the "fe_conn", "queue" and "be_sess_rate" criteria.
+  See also the "fe_conn", "queue", "be_conn_free", and "be_sess_rate" criteria.
+
+be_conn_free([]) : integer
+  Returns an integer value corresponding to the number of available connections
+  across available servers in the backend. Queue slots are not included. Backup
+  servers are also not included, unless all other servers are down. If no
+  backend name is specified, the current one is used. But it is also possible
+  to check another backend. It can be used to use a specific farm when the
+  nominal one is full. See also the "be_conn" and "connslots" criteria.
+
+  OTHER CAVEATS AND NOTES: at this point in time, the code does not take care
+  of dynamic connections. Also, if any of the server maxconn, or maxqueue is 0,
+  then this fetch clearly does not make sense, in which case the value returned
+  will be -1.
 
 be_sess_rate([]) : integer
   Returns an integer value corresponding to the sessions creation rate on the
diff --git a/src/backend.c b/src/backend.c
index 1aadca590..c26304a2d 100644
--- a/src/backend.c
+++ b/src/backend.c
@@ -1792,6 +1792,43 @@ smp_fetch_be_conn(const struct arg *args, struct sample *smp, const char *kw, vo
 	return 1;
 }
 
+/* set temp integer to the number of available connections across available
+	*	servers on the backend.
+ * Accepts exactly 1 argument. Argument is a backend, other types will lead to
+ * undefined behaviour.
+	*/
+static int
+smp_fetch_be_conn_free(const struct arg *args, struct sample *smp, const char *kw, void *private)
+{
+	struct server *iterator;
+	struct proxy *px;
+
+	smp->flags = SMP_F_VOL_TEST;
+	smp->data.type = SMP_T_SINT;
+	smp->data.u.sint = 0;
+
+	for (iterator = args->data.prx->srv; iterator; iterator = iterator->next) {
+		if (iterator->cur_state == SRV_ST_STOPPED)
+			continue;
+
+		px = iterator->proxy;
+		if (!srv_currently_usable(iterator) || ((iterator->flags & SRV_F_BACKUP) &&
+		(px->srv_act ||
+			(iterator != px->lbprm.fbck && !(px->options & PR_O_USE_ALL_BK)
+			continue;
+
+		if (iterator->maxconn == 0) {
+			/* configuration is stupid */
+			smp->data.u.sint = -1;  /* FIXME: stupid value! */
+			return 1;
+		}
+
+		smp->data.u.sint += (iterator->maxconn - iterator->cur_sess);
+	}
+
+	return 1;
+}
+
 /* set temp integer to the total number of queued connections on the backend.
  * Accepts exactly 1 argument. Argument is a backend, other types will lead to
  * undefined behaviour.
@@ -1897,6 +1934,7 @@ static int sample_conv_nbsrv(const struct arg *args, struct sample *smp, void *p
 static struct sample_fetch_kw_list smp_kws = {ILH, {
 	{ "avg_queue", smp_fetch_avg_queue_size, ARG1(1,BE),  NULL, SMP_T_SINT, SMP_USE_INTRN, },
 	{ "be_conn",   smp_fetch_be_conn,ARG1(1,BE),  NULL, SMP_T_SINT, SMP_USE_INTRN, },
+	{ "be_conn_free",  smp_fetch_be_conn_free,   ARG1(1,BE),  NULL, SMP_T_SINT, SMP_USE_INTRN, },
 	{ "be_id", smp_fetch_be_id,  0,   NULL, SMP_T_SINT, SMP_USE_BKEND, },
 	{ "be_name",   smp_fetch_be_name,0,   NULL, SMP_T_STR,  SMP_USE_BKEND, },
 	{ "be_sess_rate",  smp_fetch_be_sess_rate,   ARG1(1,BE),  NULL, SMP_T_SINT, SMP_USE_INTRN, },

[PATCH 2/2] MINOR: Add srv_conn_free sample fetch

[Patrick Hemmer]

This adds the 'srv_conn_free([/])' sample fetch. This fetch
provides the number of available connections on the designated server.
---
 doc/configuration.txt | 21 ++---
 src/backend.c | 22 ++
 2 files changed, 40 insertions(+), 3 deletions(-)


diff --git a/doc/configuration.txt b/doc/configuration.txt
index b5c093e15..b909fdc51 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -13690,7 +13690,8 @@ be_conn_free([]) : integer
   servers are also not included, unless all other servers are down. If no
   backend name is specified, the current one is used. But it is also possible
   to check another backend. It can be used to use a specific farm when the
-  nominal one is full. See also the "be_conn" and "connslots" criteria.
+  nominal one is full. See also the "be_conn", "connslots", and "srv_conn_free"
+  criteria.
 
   OTHER CAVEATS AND NOTES: at this point in time, the code does not take care
   of dynamic connections. Also, if any of the server maxconn, or maxqueue is 0,
@@ -13898,8 +13899,22 @@ srv_conn([/]) : integer
   evaluated. If  is omitted, then the server is looked up in the
   current backend. It can be used to use a specific farm when one server is
   full, or to inform the server about our view of the number of active
-  connections with it. See also the "fe_conn", "be_conn" and "queue" fetch
-  methods.
+  connections with it. See also the "fe_conn", "be_conn", "queue", and
+  "srv_conn_free" fetch methods.
+
+srv_conn_free([/]) : integer
+  Returns an integer value corresponding to the number of available connections
+  on the designated server, possibly including the connection being evaluated.
+  The value does not include queue slots. If  is omitted, then the
+  server is looked up in the current backend. It can be used to use a specific
+  farm when one server is full, or to inform the server about our view of the
+  number of active connections with it. See also the "be_conn_free" and
+  "srv_conn" fetch methods.
+
+  OTHER CAVEATS AND NOTES: at this point in time, the code does not take care
+  of dynamic connections. Also, if any of the server maxconn, or maxqueue is 0,
+  then this fetch clearly does not make sense, in which case the value returned
+  will be -1.
 
 srv_is_up([/]) : boolean
   Returns true when the designated server is UP, and false when it is either
diff --git a/src/backend.c b/src/backend.c
index c26304a2d..4bba3f4dd 100644
--- a/src/backend.c
+++ b/src/backend.c
@@ -1883,6 +1883,27 @@ smp_fetch_srv_conn(const struct arg *args, struct sample *smp, const char *kw, v
 	return 1;
 }
 
+/* set temp integer to the number of available connections on the server in the backend.
+ * Accepts exactly 1 argument. Argument is a server, other types will lead to
+ * undefined behaviour.
+ */
+static int
+smp_fetch_srv_conn_free(const struct arg *args, struct sample *smp, const char *kw, void *private)
+{
+	smp->flags = SMP_F_VOL_TEST;
+	smp->data.type = SMP_T_SINT;
+
+	if (args->data.srv->maxconn == 0) {
+			/* configuration is stupid */
+			smp->data.u.sint = -1;  /* FIXME: stupid value! */
+			return 1;
+	}
+
+	smp->data.u.sint = args->data.srv->maxconn - args->data.srv->cur_sess;
+
+	return 1;
+}
+
 /* set temp integer to the number of connections pending in the server's queue.
  * Accepts exactly 1 argument. Argument is a server, other types will lead to
  * undefined behaviour.
@@ -1942,6 +1963,7 @@ static struct sample_fetch_kw_list smp_kws = {ILH, {
 	{ "nbsrv", smp_fetch_nbsrv,  ARG1(1,BE),  NULL, SMP_T_SINT, SMP_USE_INTRN, },
 	{ "queue", smp_fetch_queue_size, ARG1(1,BE),  NULL, SMP_T_SINT, SMP_USE_INTRN, },
 	{ "srv_conn",  smp_fetch_srv_conn,   ARG1(1,SRV), NULL, SMP_T_SINT, SMP_USE_INTRN, },
+	{ "srv_conn_free", smp_fetch_srv_conn_free,  ARG1(1,SRV), NULL, SMP_T_SINT, SMP_USE_INTRN, },
 	{ "srv_id",smp_fetch_srv_id, 0,   NULL, SMP_T_SINT, SMP_USE_SERVR, },
 	{ "srv_is_up", smp_fetch_srv_is_up,  ARG1(1,SRV), NULL, SMP_T_BOOL, SMP_USE_INTRN, },
 	{ "srv_queue", smp_fetch_srv_queue,  ARG1(1,SRV), NULL, SMP_T_SINT, SMP_USE_INTRN, },

[PATCH 0/2] sample fetches for available connections

[Patrick Hemmer]

These are 2 new sample fetches which provide the available connections.
The be_conn_free fetch is similar to connslots, but has significant
difference in that it does not count queue slots, nor backup servers
(unless all servers are down).

These are intended to be useful in combination with the priority
queuing, so you can see how many connections are available, perhaps for
taking action when the number is low. For example by reserving
connections for high-priority requests, and rejecting low priority ones.

-Patrick

Patrick Hemmer (2):
  MINOR: add be_conn_free sample fetch
  MINOR: Add srv_conn_free sample fetch

 doc/configuration.txt | 34 +---
 src/backend.c | 60 +++
 2 files changed, 91 insertions(+), 3 deletions(-)

-- 
2.18.0

Re: [PATCH 1/2] MEDIUM: add set-priority-class and set-priority-offset

[Patrick Hemmer]

Re-adding the mailing list.

On 2018/8/6 22:37, Willy Tarreau wrote:
> Hi Patrick,
>>> I *think* that the change made to stream_process_counters() is not needed,
>>> because stream_process_counters() is normally used to keep the stats up
>>> to date so that when refreshing the stats page they don't make huge jumps.
>>> If you faced any corner case that this part managed to address, I'm
>>> interested in taking a look.
>> If I recall correctly, the scenario this was meant to address was when a
>> connection was removed from the queue ungracefully (e.g. a timeout).
>> In process_stream(), s->do_log(s) is called, which needs the counters
>> updated. However in this flow, the counters wouldn't get updated until
>> stream_free(s) a few lines later. stream_process_counters() seemed to be
>> made to address this scenario, so I put the code there.
>> I'll review this again though.
> Now I remember I thought about this case initially and I think I figured
> a few rare cases where it could possibly not be enough (maybe for aborts
> processed in HTTP keep-alive but I'm not sure and I prefer not to say too
> much bullshit). I remember sometimes getting an incorrect queue length at
> log time. Thus I have added a few calls to pendconn_dequeue() before all
> calls to do_log() which in my opinion are more robust for the long term.
> Thus it probably means that we can safely get rid of the calls in
> stream_process_counters(), which would be nice.
>
>>> You will notice that the code was significantly simplified thanks to the
>>> most painful part you had to deal with : the possibility for the trylock
>>> to fail. Now there is no trylock anymore and it cannot fail, so the very
>>> complex pendconn_next() is not needed anymore and I could replace it with
>>> a much simpler pendconn_first() which simply gives you the first element
>>> that is expected to be dequeued from a queue.
>> Found one spot in the code that looks like still falls into the scenario
>> where pendconn_next() is needed. This bit:
>>
>> int pendconn_redistribute(struct server *s)
>> ...
>> while ((p = pendconn_first(>pendconns))) {
>> if (p->strm_flags & SF_FORCE_PRST)
>> continue;
>>
>> This is going to infinitely loop.
> You're absolutely right, I didn't even notice this one! I think I was
> a bit too fast in replacing all loops with pendconn_first()!
>
>> An alternative option instead of pendconn_next() is on SF_FORCE_PRST we
>> build a new tree (or have some sort of temporary list), and pop them off
>> the current tree.
> In fact I don't think it's an issue, because here we need to remove all
> of them from the tree except the ones with the flag, so we don't even
> need to have something as complex as pendconn_next() to pop them up in
> order. Instead we can safely walk over the whole tree and only ignore
> SF_FORCE_PRST as the original code used to do, but using eb32_next().
> In fact my use of eb32_first() in the "while" loop in the previous
> patch was already wrong due to the "continue". We should simply have :
>
> for (node = eb32_first(>pendconns); node; node = eb32_next(node)) {
>   p = eb32_entry(, struct pendconn, node);
>   if (p->strm_flags & SF_FORCE_PRST)
>   continue;
>   ...
>   }
>
> If you want I'll update the patches once I'm at the office. Thanks for
> spotting this one.
>
> Willy
So I went and did that. It looks to work fine.
I also went and removed the queue position counter code from
stream_process_counters(), and the logging still appears to work fine
(but I could have easily missed some potential scenarios).

In regards to the idea you added to the commit message on the queue index
> It could even be further improved to avoid manipulating the stream
> from the queue :
>   - queue_idx = initial position when enqueuing
>   - queue_idx = measured position when dequeuing

Because the stream can pass through multiple queues, we'd have to make
sure that every time we de-queue, that the stream code pulls the value
and increments itself. However looking at all the various places
pendconn_unlink() gets called, I think it would be difficult for the
stream code to know when the value needs to be pulled.

Anyway, attached is the latest patch set for review again.

-Patrick
From e4acba5847fb11981c72af8430f6e6846211954f Mon Sep 17 00:00:00 2001
From: Patrick Hemmer 
Date: Fri, 11 May 2018 12:52:31 -0400
Subject: [PATCH 6/7] MEDIUM: queue: adjust position based on priority-class
 and priority-offset

The priority values are used when connections are queued to de

haproxy processing request of a disconnected client

[Patrick Hemmer]

So I just noticed the behavior that when a request is queued and the
client closes the connection, once a server slot frees up that request
is still sent to the server which processes it and sends a response back.
What's even more interesting is that the log indicates that everything
was processed normally. It basically says the response was sent back to
the client just fine.

Example config:
global
stats socket /tmp/haproxy.sock level admin
defaults
log 127.0.0.1:1234 daemon
mode http
option httplog
timeout queue 5s
frontend f1
bind :8001
default_backend b1
backend b1
server s1 127.0.0.1:8081 maxconn 1

Log output:
<30>Aug  9 12:50:40 haproxy[12384]: 127.0.0.1:64723
[09/Aug/2018:12:50:35.167] f1 b1/s1 0/0/0/5106/5106 200 118 - - 
2/2/1/1/0 0/0 "GET /y HTTP/1.1"
<30>Aug  9 12:50:45 haproxy[12384]: 127.0.0.1:64726
[09/Aug/2018:12:50:35.526] f1 b1/s1 0/4749/0/5102/9851 200 118 - - 
1/1/0/1/0 0/1 "GET /x HTTP/1.1"

^ In this, the server sleeps for 5 seconds, and then replies. I sent the
request for /y, and then a request for /x, but killed the client on the
/x request after 1 second. The request for /y was processed, but then so
was the request for /x. The close flags ("") indicate everything
went fine.

Information:

lsof:
COMMAND   PIDUSER   FD TYPE DEVICE   SIZE/OFF   
NODE NAME
...
haproxy 12384 phemmer0u CHR   16,5 0x140daf69   
1797 /dev/ttys005
haproxy 12384 phemmer1u CHR   16,5 0x140daf69   
1797 /dev/ttys005
haproxy 12384 phemmer2u CHR   16,5 0x140daf69   
1797 /dev/ttys005
haproxy 12384 phemmer3u 
KQUEUE   count=0, state=0xa
haproxy 12384 phemmer4uunix 0x933b1c4c8f54438f   
0t0 /tmp/haproxy.sock.12384.tmp
haproxy 12384 phemmer5uIPv4 0x933b1c4cc16309ff   
0t0 TCP *:8001 (LISTEN)
haproxy 12384 phemmer6uIPv4 0x933b1c4c8fef8977   
0t0 UDP *:62112
haproxy 12384 phemmer7uIPv4 0x933b1c4cbf5fd9ff   
0t0 TCP 127.0.0.1:8001->127.0.0.1:64723 (ESTABLISHED)
haproxy 12384 phemmer8uIPv4 0x933b1c4cc1546ddf   
0t0 TCP 127.0.0.1:64724->127.0.0.1:8081 (ESTABLISHED)
haproxy 12384 phemmer9uIPv4 0x933b1c4cc1c6209f   
0t0 TCP 127.0.0.1:8001->127.0.0.1:64726 (CLOSE_WAIT)

admin socket 'show fd':
  4 : st=0x05(R:PrA W:pra) ev=0x01(heopI) [nlc] cache=0
owner=0x7f93f3705010 iocb=0x107f82c40(listener_accept)
tmask=0x umask=0xfffe l.st=RDY fe=GLOBAL
  5 : st=0x05(R:PrA W:pra) ev=0x01(heopI) [nlc] cache=0
owner=0x7f93f3705650 iocb=0x107f82c40(listener_accept)
tmask=0x umask=0xfffe l.st=RDY fe=f1
  7 : st=0x25(R:PrA W:pRa) ev=0x00(heopi) [nlc] cache=0
owner=0x7f93f3708280 iocb=0x107f9e2e0(conn_fd_handler) tmask=0x1
umask=0x0 cflg=0x80201306 fe=f1 mux=PASS mux_ctx=0x7f93f3708490
  8 : st=0x25(R:PrA W:pRa) ev=0x00(heopi) [nLc] cache=0
owner=0x7f93f340ede0 iocb=0x107f9e2e0(conn_fd_handler) tmask=0x1
umask=0x0 cflg=0x00202306 sv=s1/b1 mux=PASS mux_ctx=0x7f93f350d0e0
  9 : st=0x22(R:pRa W:pRa) ev=0x10(Heopi) [nlc] cache=0
owner=0x7f93f510 iocb=0x107f9e2e0(conn_fd_handler) tmask=0x1
umask=0x0 cflg=0x80241300 fe=f1 mux=PASS mux_ctx=0x7f93f5100210
 10 : st=0x25(R:PrA W:pRa) ev=0x00(heopi) [nlc] cache=0
owner=0x7f93f3708100 iocb=0x107f9e2e0(conn_fd_handler) tmask=0x1
umask=0x0 cflg=0x00201306 fe=GLOBAL mux=PASS mux_ctx=0x7f93f350d1a0

admin socket 'show sess':
0x7f93f37084c0: proto=tcpv4 src=127.0.0.1:64723 fe=f1 be=b1 srv=s1
ts=08 age=2s calls=3 rq[f=48840200h,i=0,an=8000h,rx=,wx=,ax=]
rp[f=8040h,i=0,an=a0h,rx=,wx=,ax=] s0=[7,8h,fd=7,ex=]
s1=[7,118h,fd=8,ex=] exp=
0x7f93f5100240: proto=tcpv4 src=127.0.0.1:64726 fe=f1 be=b1
srv= ts=08 age=2s calls=3 rq[f=c800020h,i=0,an=8000h,rx=,wx=,ax=]
rp[f=8000h,i=0,an=00h,rx=,wx=,ax=] s0=[7,8h,fd=9,ex=]
s1=[2,110h,fd=-1,ex=2s] exp=2s
0x7f93f350d1d0: proto=unix_stream src=unix:1 fe=GLOBAL be=
srv= ts=02 age=0s calls=1
rq[f=40c08202h,i=0,an=00h,rx=10s,wx=,ax=]
rp[f=80008002h,i=0,an=00h,rx=,wx=,ax=] s0=[7,8h,fd=10,ex=]
s1=[7,4018h,fd=-1,ex=] exp=10s

Version info:
# ./haproxy -vv
HA-Proxy version 1.8.13 2018/07/30
Copyright 2000-2018 Willy Tarreau 

Build options :
TARGET  = osx
CPU = generic
CC  = gcc
CFLAGS  = -O0 -g -fno-strict-aliasing
-Wdeclaration-after-statement -fwrapv -fno-strict-overflow
-Wno-address-of-packed-member -Wno-null-dereference -Wno-unused-label
OPTIONS = USE_ZLIB=1 USE_OPENSSL=1 USE_LUA=1 USE_PCRE=1

Default settings :
maxconn = 2000, bufsize = 16384, maxrewrite = 1024,
maxpollevents = 200

Built with OpenSSL version : OpenSSL 1.1.0h  27 Mar 2018
Running on OpenSSL version : OpenSSL 1.1.0h  27 Mar 2018
OpenSSL library supports 

Re: haproxy and changing ELB IPs

[Patrick Hemmer]

On 2018/8/7 05:45, Lukas Tribus wrote:
> Hello,
>
>
>> We recently had an outage for short time related to NameServer's h/w failure 
>> (both primary and secondary went down).
>> We were told that it is possible for these IPs to change in the future. It 
>> never happened so far though.
> So you don't have changing nameservers at all, but it is possible that
> the IPs will change once.
>
> I suggest you don't over-engineer this. Automating a possible one time
> occurrence is a waste of time, imho.
>
>
>> is it possible to optionally log the NS IPs during every health check?
> No.
>
>
>> Would a reload suffice instead of restart? It should not be difficult to 
>> create a monitor
>> for resolv.conf file using inotify lets say and automatically reload/restart 
>> haproxy in case
>> it's content has changed.
> Sure, a reload would suffice.
>
>
> Regards,
> Lukas
>
As an alternative option, if the system utilizes NetworkManager, then
solving this becomes very easy. NetworkManager can be configured to
provide a local dnsmasq instance as a DNS proxy. If this is enabled,
then your resolver becomes a static "127.0.0.1". And since
NetworkManager also integrates with the DHCP client, if the nameserver
IPs change, then it'll reload dnsmasq, and you don't need to do anything
with haproxy.
Enabling this is as simple as adding "dns = dnsmasq" to NetworkManager.conf.

-Patrick

Re: BUG: Tw is negative with lua sleep

[Patrick Hemmer]

Ping?

-Patrick

On 2018/6/22 15:10, Patrick Hemmer wrote:
> When using core.msleep in lua, the %Tw metric is a negative value.
>
> For example with the following config:
> haproxy.cfg:
> global
> lua-load /tmp/haproxy.lua
>
> frontend f1
> mode http
> bind :8000
> default_backend b1
> log 127.0.0.1:1234 daemon
> log-format Ta=%Ta\ Tc=%Tc\ Td=%Td\ Th=%Th\ Ti=%Ti\ Tq=%Tq\
> TR=%TR\ Tr=%Tr\ Tt=%Tt\ Tw=%Tw
>
> backend b1
> mode http
> http-request use-service lua.foo
>
> haproxy.lua:
> core.register_service("foo", "http", function(applet)
> core.msleep(100)
> applet:set_status(200)
> applet:start_response()
> end)
>
> The log contains:
> Ta=104 Tc=0 Td=0 Th=0 Ti=0 Tq=104 TR=104 Tr=104 Tt=104 Tw=-104
>
> ^ TR also looks wrong, as it did not take 104ms to receive the full
> request.
>
> This is built from the commit before current master: d8fd2af
>
> -Patrick

BUG: Tw is negative with lua sleep

[Patrick Hemmer]

When using core.msleep in lua, the %Tw metric is a negative value.

For example with the following config:
haproxy.cfg:
global
lua-load /tmp/haproxy.lua

frontend f1
mode http
bind :8000
default_backend b1
log 127.0.0.1:1234 daemon
log-format Ta=%Ta\ Tc=%Tc\ Td=%Td\ Th=%Th\ Ti=%Ti\ Tq=%Tq\
TR=%TR\ Tr=%Tr\ Tt=%Tt\ Tw=%Tw

backend b1
mode http
http-request use-service lua.foo

haproxy.lua:
core.register_service("foo", "http", function(applet)
core.msleep(100)
applet:set_status(200)
applet:start_response()
end)

The log contains:
Ta=104 Tc=0 Td=0 Th=0 Ti=0 Tq=104 TR=104 Tr=104 Tt=104 Tw=-104

^ TR also looks wrong, as it did not take 104ms to receive the full request.

This is built from the commit before current master: d8fd2af

-Patrick

BUG: cannot take the address of an rvalue of type 'unsigned long'

[Patrick Hemmer]

Trying to compile current master on OS-X, and get:

gcc -Iinclude -Iebtree -Wall  -O0 -g -fno-strict-aliasing
-Wdeclaration-after-statement -fwrapv -fno-strict-overflow 
-Wno-address-of-packed-member -Wno-null-dereference
-Wno-unused-label   -DTPROXY -DUSE_ZLIB  -DENABLE_POLL
-DENABLE_KQUEUE -DUSE_OPENSSL
-I/Users/phemmer/git/haproxy/openssl-1.1.0h/include -DUSE_LUA
-I/Users/phemmer/git/haproxy/lua-5.3.4/src
-I/Users/phemmer/git/haproxy/lua-5.3.4/src -DUSE_PCRE
-I/opt/local/include  -DCONFIG_HAPROXY_VERSION=\"1.9-dev0-ba86c6-462\"
-DCONFIG_HAPROXY_DATE=\"2018/06/22\" \
-DBUILD_TARGET='"osx"' \
-DBUILD_ARCH='""' \
-DBUILD_CPU='"generic"' \
-DBUILD_CC='"gcc"' \
-DBUILD_CFLAGS='"-O0 -g -fno-strict-aliasing
-Wdeclaration-after-statement -fwrapv -fno-strict-overflow
-Wno-address-of-packed-member -Wno-null-dereference -Wno-unused-label"' \
-DBUILD_OPTIONS='"USE_ZLIB=1 USE_OPENSSL=1
USE_LUA=1 USE_PCRE=1"' \
 -c -o src/haproxy.o src/haproxy.c
clang: warning: argument unused during compilation:
'-fno-strict-overflow' [-Wunused-command-line-argument]
src/haproxy.c:2476:16: error: cannot take the address of an
rvalue of type 'unsigned long'
HA_ATOMIC_AND(_threads_mask, ~tid_bit);
  ^
include/common/hathreads.h:41:42: note: expanded from macro
'HA_ATOMIC_AND'
#define HA_ATOMIC_AND(val, flags)({*(val) &= (flags);})
 ^~~
1 error generated.
make: *** [src/haproxy.o] Error 1

Built with: Apple LLVM version 9.0.0 (clang-900.0.39.2)

This broke in change ba86c6c:
commit ba86c6c25bf252e44589ae2b4d51a67c4f47d244 (HEAD -> master,
origin/master, origin/HEAD)
Author: Christopher Faulet 
Date:   Thu Jun 21 09:57:39 2018 +0200

MINOR: threads: Be sure to remove threads from all_threads_mask on exit
   
When HAProxy is started with several threads, Each running thread
holds a bit in
the bitfiled all_threads_mask. This bitfield is used here and there
to check
which threads are registered to take part in a specific processing.
So when a
thread exits, it seems normal to remove it from all_threads_mask.
   
No direct impact could be identified with this right now but it would
be better to backport it to 1.8 as a preventive measure to avoid complex
situations like the one in previous bug.



-Patrick

Re: [PATCH] BUG/MINOR: lua: Segfaults with wrong usage of types.

[Patrick Hemmer]

On 2018/6/15 09:06, Frederic Lecaille wrote:
> On 06/15/2018 02:28 PM, Frederic Lecaille wrote:
>> On 06/15/2018 02:15 PM, Frederic Lecaille wrote:
>>> On 06/14/2018 11:05 PM, Patrick Hemmer wrote:
>>>> Haproxy segfaults if you pass the wrong argument type to a converter.
>>>> Example:
>>>>
>>>> haproxy.cfg:
>>>>  global
>>>>  lua-load /tmp/haproxy.lua
>>>>
>>>>  frontend f1
>>>>  mode http
>>>>  bind :8000
>>>>  default_backend b1
>>>>
>>>>  http-request lua.foo
>>>>
>>>>  backend b1
>>>>  mode http
>>>>  server s1 127.0.0.1:8080
>>>>
>>>> haproxy.lua:
>>>>  core.register_action("foo", { "http-req" }, function(txn)
>>>>  txn.sc:ipmask(txn.f:src(), 24, 112)
>>>>  end)
>>>>
>>>> Result:
>>>>  * thread #1, queue = 'com.apple.main-thread', stop reason =
>>>> EXC_BAD_ACCESS (code=1, address=0x18)
>>>>  frame #0: 0x7fffc9fcbf56
>>>> libsystem_platform.dylib`_platform_memmove$VARIANT$Haswell + 182
>>>> libsystem_platform.dylib`_platform_memmove$VARIANT$Haswell:
>>>>  ->  0x7fffc9fcbf56 <+182>: movb   (%rsi,%r8), %cl
>>>>  0x7fffc9fcbf5a <+186>: movb   %cl, (%rdi,%r8)
>>>>  0x7fffc9fcbf5e <+190>: subq   $0x1, %rdx
>>>>  0x7fffc9fcbf62 <+194>: je 0x7fffc9fcbf78; <+216>
>>>>  Target 0: (haproxy) stopped.
>>>>  (lldb) bt
>>>>  * thread #1, queue = 'com.apple.main-thread', stop reason =
>>>> EXC_BAD_ACCESS (code=1, address=0x18)
>>>>* frame #0: 0x7fffc9fcbf56
>>>> libsystem_platform.dylib`_platform_memmove$VARIANT$Haswell + 182
>>>>  frame #1: 0x7fffc9e7442e
>>>> libsystem_c.dylib`__memcpy_chk + 22
>>>>  frame #2: 0x00010002ec46
>>>> haproxy`hlua_lua2arg_check(L=0x00010120d298, first=3,
>>>> argp=0x7fff5fbfe690, mask=196, p=0x000101817000) at hlua.c:749
>>>>  frame #3: 0x00010001fa00
>>>> haproxy`hlua_run_sample_conv(L=0x00010120d298) at hlua.c:3393
>>>>  frame #4: 0x00010032400b haproxy`luaD_precall + 747
>>>>  frame #5: 0x0001003343c6 haproxy`luaV_execute + 3158
>>>>  frame #6: 0x000100323429 haproxy`luaD_rawrunprotected
>>>> + 89
>>>>  frame #7: 0x000100324516 haproxy`lua_resume + 278
>>>>  frame #8: 0x00010001b199
>>>> haproxy`hlua_ctx_resume(lua=0x000101205080, yield_allowed=1) at
>>>> hlua.c:1080
>>>>  frame #9: 0x000100027de8
>>>> haproxy`hlua_action(rule=0x00010101b180, px=0x000101817000,
>>>> sess=0x00010120cb70, s=0x00010120cc00, flags=2) at hlua.c:6198
>>>>  frame #10: 0x000100044bcd
>>>> haproxy`http_req_get_intercept_rule(px=0x000101817000,
>>>> rules=0x000101817048, s=0x00010120cc00,
>>>> deny_status=0x7fff5fbfee78) at proto_http.c:2760
>>>>  frame #11: 0x000100046182
>>>> haproxy`http_process_req_common(s=0x00010120cc00,
>>>> req=0x00010120cc10, an_bit=16, px=0x000101817000) at
>>>> proto_http.c:3461
>>>>  frame #12: 0x000100094c50
>>>> haproxy`process_stream(t=0x00010120cf40,
>>>> context=0x00010120cc00, state=9) at stream.c:1905
>>>>  frame #13: 0x00010016179f
>>>> haproxy`process_runnable_tasks at task.c:362
>>>>  frame #14: 0x0001000ea0eb haproxy`run_poll_loop at
>>>> haproxy.c:2403
>>>>  frame #15: 0x0001000e7c74
>>>> haproxy`run_thread_poll_loop(data=0x7fff5fbff3a4) at
>>>> haproxy.c:2464
>>>>  frame #16: 0x0001000e4a49 haproxy`main(argc=3,
>>>> argv=0x7fff5fbff590) at haproxy.c:3082
>>>>  frame #17: 0x7fffc9db9235 libdyld.dylib`start + 1
>>>>
>>>> Issue goes away if you change the lua txn.sc:ipmask() line to:
>>>>  txn.sc:ipmask(txn.f:src(), '24', '112')
>>>>
>>>> Reproduced with current master (9db0fed) and lua version 5.3.4.
>>>>
>>>> -Patrick
>>>
>>> It seems the patch attached to this mail fixes this issue. It at
>>> least make the varnishtest test file pass.
>>>
>>> Must be checked by Thierry.
>>
>> Should have mentionned that I could not reproduce this issue without
>> compiling the thread support (USE_THREAD=1).
>
> There is potentially the same issue in hlua_run_sample_conv(). See the
> updated patch attached to this mail.
>
>
I can confirm this patch addresses the segfault for my use case.

Thanks

-Patrick

BUG: segfault with lua sample converters & wrong arg types

[Patrick Hemmer]

Haproxy segfaults if you pass the wrong argument type to a converter.
Example:

haproxy.cfg:
global
lua-load /tmp/haproxy.lua
   
frontend f1
mode http
bind :8000
default_backend b1
   
http-request lua.foo
   
backend b1
mode http
server s1 127.0.0.1:8080

haproxy.lua:
core.register_action("foo", { "http-req" }, function(txn)
txn.sc:ipmask(txn.f:src(), 24, 112)
end)

Result:
* thread #1, queue = 'com.apple.main-thread', stop reason =
EXC_BAD_ACCESS (code=1, address=0x18)
frame #0: 0x7fffc9fcbf56
libsystem_platform.dylib`_platform_memmove$VARIANT$Haswell + 182
libsystem_platform.dylib`_platform_memmove$VARIANT$Haswell:
->  0x7fffc9fcbf56 <+182>: movb   (%rsi,%r8), %cl
0x7fffc9fcbf5a <+186>: movb   %cl, (%rdi,%r8)
0x7fffc9fcbf5e <+190>: subq   $0x1, %rdx
0x7fffc9fcbf62 <+194>: je 0x7fffc9fcbf78; <+216>
Target 0: (haproxy) stopped.
(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason =
EXC_BAD_ACCESS (code=1, address=0x18)
  * frame #0: 0x7fffc9fcbf56
libsystem_platform.dylib`_platform_memmove$VARIANT$Haswell + 182
frame #1: 0x7fffc9e7442e libsystem_c.dylib`__memcpy_chk + 22
frame #2: 0x00010002ec46
haproxy`hlua_lua2arg_check(L=0x00010120d298, first=3,
argp=0x7fff5fbfe690, mask=196, p=0x000101817000) at hlua.c:749
frame #3: 0x00010001fa00
haproxy`hlua_run_sample_conv(L=0x00010120d298) at hlua.c:3393
frame #4: 0x00010032400b haproxy`luaD_precall + 747
frame #5: 0x0001003343c6 haproxy`luaV_execute + 3158
frame #6: 0x000100323429 haproxy`luaD_rawrunprotected + 89
frame #7: 0x000100324516 haproxy`lua_resume + 278
frame #8: 0x00010001b199
haproxy`hlua_ctx_resume(lua=0x000101205080, yield_allowed=1) at
hlua.c:1080
frame #9: 0x000100027de8
haproxy`hlua_action(rule=0x00010101b180, px=0x000101817000,
sess=0x00010120cb70, s=0x00010120cc00, flags=2) at hlua.c:6198
frame #10: 0x000100044bcd
haproxy`http_req_get_intercept_rule(px=0x000101817000,
rules=0x000101817048, s=0x00010120cc00,
deny_status=0x7fff5fbfee78) at proto_http.c:2760
frame #11: 0x000100046182
haproxy`http_process_req_common(s=0x00010120cc00,
req=0x00010120cc10, an_bit=16, px=0x000101817000) at
proto_http.c:3461
frame #12: 0x000100094c50
haproxy`process_stream(t=0x00010120cf40, context=0x00010120cc00,
state=9) at stream.c:1905
frame #13: 0x00010016179f haproxy`process_runnable_tasks at
task.c:362
frame #14: 0x0001000ea0eb haproxy`run_poll_loop at
haproxy.c:2403
frame #15: 0x0001000e7c74
haproxy`run_thread_poll_loop(data=0x7fff5fbff3a4) at haproxy.c:2464
frame #16: 0x0001000e4a49 haproxy`main(argc=3,
argv=0x7fff5fbff590) at haproxy.c:3082
frame #17: 0x7fffc9db9235 libdyld.dylib`start + 1

Issue goes away if you change the lua txn.sc:ipmask() line to:
txn.sc:ipmask(txn.f:src(), '24', '112')

Reproduced with current master (9db0fed) and lua version 5.3.4.

-Patrick

Re: [Feature request] Call fan-out to all endpoints.

[Patrick Hemmer]

On 2018/6/10 13:27, Aleksandar Lazic wrote:
> Hi.
>
> On 10/06/2018 17:56, amotz wrote:
>> Baptiste wrote:
>>> Hi,
>>>
>>> what's the use case?
>>> Is this API gateway kind of thing?
>>>
>>> Baptiste
>>
>> From my experience this is mostly needed for operations/management API.
>>
>> Some examples:
>> getStaus (i.e get the status/health from all endpoint)
>> flashCache (make all endpoint flash their cache)
>> setConfig (you get the point ...)
>> more...
>>
>> with regard to the fan-in question by Jonathan.
>> Maybe return 207 (multi-status)  https://httpstatuses.com/207 ?
>> IMO, the most intuitive response would be a json array of all the
>> endpoints
>> responses, but I'm open for suggestions.
>
> Let's say you have a `option allendpoints /_fan-out`.
>
> When you now call `curl -sS https://haproxy:8080/_fan-out/health` then
> you will receive a json from *all active* endpoints (pods,
> app-server,...) with the result of there `/health`, something like this?
>
> That sounds interesting. Maybe it's possible with a
> `external-check command ` or some lua code?
>
> https://cbonte.github.io/haproxy-dconv/1.8/configuration.html#external-check%20%28Alphabetically%20sorted%20keywords%20reference%29
>

Just throwing out my own $0.02, I don't think this is a good
responsibility for haproxy to support. This is very specific application
level logic.
Haproxy don't care about content types (json). What if I want to use
this feature, but with some other encoding?
How should haproxy respond if a server sends a 1GB response? It can't
buffer the whole thing in memory so it can encode it and add it to the
response message.
What about the non-happy-path cases? What if one of the servers times
out, what should haproxy put in the response? What if a server sends a
partial response?
How should the headers from a server response be encoded?

This is basically the invention of a new protocol.

Don't get me wrong, the underlying goal, having a client send a single
request and that request getting duplicated amongst the servers, is a
good one. In fact we do this at my work. But we use a custom application
that is specifically designed to handle the protocol we are wrapping.

I think this might be reasonable to do in LUA, and maybe even possible
already, but there's still going to be lots of the fore-mentioned
difficulties.
However to put some measure of positive spin on things, I think HTTP/2
would fit very well with this use case. HTTP/2 supports server push
messages. Meaning it's built in to the protocol that the client can send
a single request, and receive multiple responses. Haproxy doesn't
support fully H2 passthrough right now, but that may not be necessary. I
think LUA really only needs a few things to be able to support this: The
ability to receive H2 requests & generate responses (LUA already has
http/1.1 response capabilities, but I have no idea if they work with H2
requests), and then the ability to trigger a request to a server, and
have that sent back to the client as a server-push message.

-Patrick

Re: [PATCH 1/2] MEDIUM: add set-priority-class and set-priority-offset

[Patrick Hemmer]

On 2018/5/31 00:57, Willy Tarreau wrote:
> Hi Patrick,
>
> On Thu, May 31, 2018 at 12:16:27AM -0400, Patrick Hemmer wrote:
>>> I looked at the code to see if something could cause that. I found that the
>>> key increment could be a reason (you must restart from the next element,
>>> not an upper value since there will be many duplicate keys) 
>> Gah, I completely forgot about duplicate keys. Will fix.
> I'll send you (later today) some splitted patches that will help for
> this. This simplifies review and experimentations on the walk algorithm.
I kept the patch set you created, and have made some minor adjustments
to address the mentioned issues.

I think the only uncertainty I had during implementation was an atomic
operator for retrieving the queue_idx value inside
stream_process_counters. There's no load operator, but I'm not sure if
that is by design, or what the preferred solution is here. In theory if
the value is aligned, the load is atomic anyway and we don't need an
explicit atomic operator. But I'm not sure if that's an assumption we
want to make.

>> The warning is addressable. It means the user should add a `timeout
>> queue` and set it less than 524287ms. This is similar to the "missing
>> timeouts for frontend/backend" warning we print. Though I think it would
>> make sense to add "queue" to that existing warning instead, as it
>> already mentions the timeouts for client, connect & server.
> In fact the problem I'm having is that I've already seen some very high
> timeout values in field (eg on a print server), which means that some
> people do need to have a large value here. For sure they don't need to
> reach 24 days but 8min will definitely be too short for certain extreme
> cases. And I understand the impact of such large values with the queue,
> I just don't know how we can address this at the moment, it's what I'm
> still thinking about.
>
>>> We need to find a more suitable name for this "cntdepend" as it really 
>>> doesn't
>>> tell me that it has anything to do with what it says. I don't have anything
>>> to propose for now I'm afraid.
>> Yeah, not fond of the name, but it was something, and is easy to change.
>> The name originated from "cnt"=counter, and "pend" to be consistent with
>> the naming of "nbpend" and "totpend" right above it. So: counter of
>> de-queued pending connections.
> OK. In fact given that it's a wrapping counter it's why I considered it
> could be an index for the srv and px parts, but the stream part stores
> a distance between the recorded index and the current index.
>
>>> @@ -2467,6 +2469,10 @@ struct task *process_stream(struct task *t, void 
>>> *context, unsigned short state)
>>> return t; /* nothing more to do */
>>> }
>>>  
>>> +   // remove from pending queue here so we update counters
>>> +   if (s->pend_pos)
>>> +   pendconn_free(s->pend_pos);
>>> +
>>> if (s->flags & SF_BE_ASSIGNED)
>>> HA_ATOMIC_SUB(>be->beconn, 1);
>>>
>>> This part is not supposed to be needed since it's already performed
>>> in stream_free() a few lines later. Or if it's required to get the
>>> queue values correctly before logging, then something similar will
>>> be required at every place where we log after trying to connect to
>>> a server (there are a few places depending on the log format and
>>> the logasap option).
>> Yes, it was put there to update the counters before logging. Re-thinking
>> this, updating of the counters needs to be moved into
>> pendconn_process_next_stream anyway. Since this function updates
>> px->cntdepend and is called in a loop, calculating logs.prx_queue_pos
>> after this loop completes will yield incorrect values.
> I thought about the same but was not yet completely certain. I suspect in
> fact the stream's value should indeed be updated upon dequeue, and that
> only the error case has to be taken into account before logging (if there
> was no dequeue). In this case I think we can have this one performed here
> provided we ensure other log places will not be triggered on error.
>
>>> Now I think I understand how the cntdepend works : isn't it simply a
>>> wrapping queue index that you use to measure the difference between
>>> when you queued and when you dequeued ? In this case, wouldn't something
>>> like "queue_idx" be more explicit ? At least it becomes more obvious when
>>> doing the measure that we measure a length by subtracting two inde

Re: haproxy requests hanging since b0bdae7

[Patrick Hemmer]

On 2018/6/6 08:24, Olivier Houchard wrote:
> Hi Willy,
>
> On Wed, Jun 06, 2018 at 02:09:01PM +0200, Willy Tarreau wrote:
>> On Wed, Jun 06, 2018 at 02:04:35PM +0200, Olivier Houchard wrote:
>>> When building without threads enabled, instead of just using the global
>>> runqueue, just use the local runqueue associated with the only thread, as
>>> that's what is now expected for a single thread in prcoess_runnable_tasks().
>> Just out of curiosity, shouldn't we #ifdef out the global runqueue
>> definition when running without threads in order to catch such cases
>> in the future ?
>>
> I think this is actually a good idea.
> My only concern is it adds quite a bit of #ifdef USE_THREAD, see the attached
> patch.
>
> Regards,
>
> Olivier
With this patch I'm getting:

include/proto/task.h:138:26: error: use of undeclared identifier 'rqueue'

With the previous patch, both reported issues are resolved.

-Patrick

timeout queue broken since f6e6dc1

[Patrick Hemmer]

Just ran across an issue where the `timeout queue` option is
non-functional. I can send a request to haproxy which sits in the queue
for well past the configured limit.

It appears the issue popped up as a result of this commit:

commit f6e6dc12cd533b2d8bb6413a4b5f875ddfd3e6e3 (refs/bisect/bad)
Author: Olivier Houchard 
Date:   Fri May 18 18:38:23 2018 +0200

MAJOR: tasks: Create a per-thread runqueue.
   
A lot of tasks are run on one thread only, so instead of having them all
in the global runqueue, create a per-thread runqueue which doesn't
require
any locking, and add all tasks belonging to only one thread to the
corresponding runqueue.
   
The global runqueue is still used for non-local tasks, and is visited
by each thread when checking its own runqueue. The nice parameter is
thus used both in the global runqueue and in the local ones. The rare
tasks that are bound to multiple threads will have their nice value
used twice (once for the global queue, once for the thread-local one).


Reproduced with TARGET=osx
Compiler clang-900.0.39.2

-Patrick

haproxy requests hanging since b0bdae7

[Patrick Hemmer]

It seems that commit b0bdae7 has completely broken haproxy for me. When
I send a request to haproxy, it just sits there. The backend server
receives nothing, and the client waits for a response.
Running with debug enabled I see just a single line:
:f1.accept(0004)=0005 from [127.0.0.1:63663] ALPN=

commit b0bdae7b88d53cf8f18af0deab6d4c29ac25b7f9 (refs/bisect/bad)
Author: Olivier Houchard 
Date:   Fri May 18 18:45:28 2018 +0200

MAJOR: tasks: Introduce tasklets.
   
Introduce tasklets, lightweight tasks. They have no notion of priority,
they are just run as soon as possible, and will probably be used for I/O
later.
   
For the moment they're used to replace the temporary thread-local list
that was used in the scheduler. The first part of the struct is common
with tasks so that tasks can be cast to tasklets and queued in this
list.
Once a task is in the tasklet list, it has its leaf_p set to 0x1 so that
it cannot accidently be confused as not in the queue.
   
Pure tasklets are identifiable by their nice value of -32768 (which is
normally not possible).

Issue reproducible with a very simple config:

defaults
  mode http
frontend f1
  bind :8081
  default_backend b1
backend b1
  server s1 127.0.0.1:8081

Compiled on OS-X with only a single make variable of TARGET=osx
Compiler: clang-900.0.39.2


-Patrick

Re: [PATCH 1/2] MEDIUM: add set-priority-class and set-priority-offset

[Patrick Hemmer]

On 2018/5/30 04:00, Willy Tarreau wrote:
> Hi Patrick,
>
> I'm finally back on this.
>
> On Mon, May 14, 2018 at 01:05:03AM -0400, Patrick Hemmer wrote:
>> On 2018/5/11 12:52, Patrick Hemmer wrote:
>>> This adds the set-priority-class and set-priority-offset actions to
>>> http-request and tcp-request content.
>>> The priority values are used when connections are queued to determine
>>> which connections should be served first. The lowest priority class is
>>> served first. When multiple requests from the same class are found, the
>>> earliest (according to queue_time + offset) is served first.
>>> ---
>>>  doc/configuration.txt  |  38 ++
>>>  doc/lua-api/index.rst  |  18 +++
>>>  include/types/proxy.h  |   3 +-
>>>  include/types/queue.h  |   2 +-
>>>  include/types/server.h |   3 +-
>>>  include/types/stream.h |   7 +-
>>>  src/cfgparse.c |  15 +++
>>>  src/hlua.c |  74 ---
>>>  src/log.c  |   4 +-
>>>  src/proto_http.c   |   4 +-
>>>  src/proxy.c|   2 +-
>>>  src/queue.c| 345
>>> ++---
>>>  src/server.c   |   2 +-
>>>  src/stream.c   |  10 +-
>>>  14 files changed, 453 insertions(+), 74 deletions(-)
>>>
>>>
>> I found one issue I'll need to fix. During final code cleanup I
>> accidentally replaced good code with the `key_incr` function in 2 places
>> (pendconn_redistribute and pendconn_grab_from_px). Will fix and submit
>> new patch pending feedback.
> So I spent some time testing the code yesterday. Overall it works pretty
> well, and allows to maintain very low response times for certain requests
> while others are higher (when using the class) or to finely adjust the
> response time distribution between requests.
>
> I found that using classes under high loads result in the highest priority
> requests to totally exclude the other ones, which is expected, while offsets
> provide a smoother transition but may not fit all purposes either.
>
> I noticed a strange effect which is that when injecting under low load with
> a higher priority (either offset or class) than another high level traffic,
> the response time on the higher priority traffic follows a sawtooth shape,
> it progressively raises from 0 to 50-80ms and suddenly drops to zero again.
>
> I looked at the code to see if something could cause that. I found that the
> key increment could be a reason (you must restart from the next element,
> not an upper value since there will be many duplicate keys) 
Gah, I completely forgot about duplicate keys. Will fix.

> but it doesn't
> change anything. It could be something in the lower layers as well, I don't
> really know for now. At least what I've seen is that the server's response
> time when attacked directly, is stable. It's very hard to say if this is
> due to the change or not given that it was not possible to set priorities
> in the past!
>
> By the way, regarding this "restart from next" thing, I looked at the locking
> and the element you find is perfectly safe to be used to get the next one. So
> the tree progressing can definitely restart using eb32_next(node), modulo the
> wrapping going back to eb32_first() as we do in the scheduler. I'll see if I
> can propose you something around this.
>
> I found some minor suggestions that I can take care of, like this :
>
> @@ -125,6 +125,9 @@ struct stream {
>  
> struct server *srv_conn;/* stream already has a slot on a 
> server and is not in queue */
> struct pendconn *pend_pos;  /* if not NULL, points to the pending 
> position in the pending queue */
> +   int16_t priority_class; /* priority class of the stream for 
> the pending queue */
> +   int32_t priority_offset;/* priority offset of the stream for 
> the pending queue */
> +   unsigned int cntdepend; /* value of proxy/server cntdepend at 
> time of enqueue */
>  
> struct http_txn *txn;   /* current HTTP transaction being 
> processed. Should become a list. */
>
> This is creating a 48-bit hole between the two fields, I'll check if there's
> a nearby 16-bit hole to place the priority_class entry. Otherwise I'll add
> a comment to mention there's a hole there.
>
> --- a/src/cfgparse.c
> +++ b/src/cfgparse.c
> @@ -8131,6 +8131,21 @@ out_uri_auth_compat:
> }
> }
>  
> +   if ((curproxy->mode == PR_MODE_TCP || curproxy->mode == 
> PR_MODE_HTTP) &&a

Re: [PATCH 1/2] MEDIUM: add set-priority-class and set-priority-offset

[Patrick Hemmer]

On 2018/5/11 12:52, Patrick Hemmer wrote:
> This adds the set-priority-class and set-priority-offset actions to
> http-request and tcp-request content.
> The priority values are used when connections are queued to determine
> which connections should be served first. The lowest priority class is
> served first. When multiple requests from the same class are found, the
> earliest (according to queue_time + offset) is served first.
> ---
>  doc/configuration.txt  |  38 ++
>  doc/lua-api/index.rst  |  18 +++
>  include/types/proxy.h  |   3 +-
>  include/types/queue.h  |   2 +-
>  include/types/server.h |   3 +-
>  include/types/stream.h |   7 +-
>  src/cfgparse.c |  15 +++
>  src/hlua.c |  74 ---
>  src/log.c  |   4 +-
>  src/proto_http.c   |   4 +-
>  src/proxy.c|   2 +-
>  src/queue.c| 345
> ++---
>  src/server.c   |   2 +-
>  src/stream.c   |  10 +-
>  14 files changed, 453 insertions(+), 74 deletions(-)
>
>
I found one issue I'll need to fix. During final code cleanup I
accidentally replaced good code with the `key_incr` function in 2 places
(pendconn_redistribute and pendconn_grab_from_px). Will fix and submit
new patch pending feedback.

-Patrick

[PATCH 0/2] Re: Priority based queuing

[Patrick Hemmer]

Ok, so here is the full submission for priority based queuing.

Notes since previous update:

Wasn't really able to optimize the tree search any. Tried a few things,
but nothing made a measurable performance difference.

I added a warning message and documentation making clear the issues with
timestamp wrapping.
Though one thing that might not be completely obvious is that even if
the user does not configure `set-priority-offset` at all, they're still
susceptible to the wrapping issue as the priority is the queue key
whether priority is adjusted or not.

The implementation of the %sq (srv_queue) and %bq (backend_queue) was
change to keep the description accurate. The description is "number of
requests which were processed before this one". The previous
implementation just stored the size of the queue at the time the
connection was queued. Since we can inject a connection into the middle
of the queue, this no longer works. Now we keep a count of dequeued
connections, and take the difference between when the connection was
queued, and then dequeued. This also means the value will be slightly
different even for users who don't use priority, as the previous method
would have included connections which closed without being processed.

I added sample fetches for retrieving the class/offset of the current
transaction. I think it might be beneficial to add some other fetches
for tracking the health of the queue, such as average class/offset, or
an exponential moving average of the class/offset for requests added to
the queue, requests processed, and requests which closed/timed out. But
this is just more stuff the code would have to store, so unsure if
they're worth it.


I wasn't convinced the 64-bit key was a bad idea, so I implemented the
idea with a 12/52 split and an absolute timestamp.  On my system (which
is 64-bit) the performance is about 20% faster. The code is much
simpler. And it also solves the limitations and issues with wrapping.
The patch for this is included in case it's of interest.

-Patrick


Patrick Hemmer (2):
  MEDIUM: add set-priority-class and set-priority-offset
  use a 64-bit int with absolute timestamp for priority-offset

 doc/configuration.txt  |  38 +++
 doc/lua-api/index.rst  |  18 
 include/types/proxy.h  |   3 +-
 include/types/queue.h  |   2 +-
 include/types/server.h |   3 +-
 include/types/stream.h |   7 +-
 src/cfgparse.c |  15 +++
 src/hlua.c |  69 +
 src/log.c  |   4 +-
 src/proto_http.c   |   4 +-
 src/proxy.c|   2 +-
 src/queue.c| 261
+
 src/server.c   |   2 +-
 src/stream.c   |  10 +-
 14 files changed, 366 insertions(+), 72 deletions(-)

-- 
2.16.3

[PATCH 2/2] use a 64-bit int with absolute timestamp for priority-offset

[Patrick Hemmer]

---
 include/types/queue.h |   2 +-
 src/hlua.c|   5 --
 src/queue.c   | 144
+++---
 3 files changed, 33 insertions(+), 118 deletions(-)


diff --git a/include/types/queue.h b/include/types/queue.h
index 03377da69..5f4693942 100644
--- a/include/types/queue.h
+++ b/include/types/queue.h
@@ -35,7 +35,7 @@ struct pendconn {
 	struct stream *strm;
 	struct proxy  *px;
 	struct server *srv;/* the server we are waiting for, may be NULL */
-	struct eb32_node node;
+	struct eb64_node node;
 	__decl_hathreads(HA_SPINLOCK_T lock);
 };
 
diff --git a/src/hlua.c b/src/hlua.c
index 6e727648d..dd7311ff8 100644
--- a/src/hlua.c
+++ b/src/hlua.c
@@ -5351,11 +5351,6 @@ __LJMP static int hlua_txn_set_priority_offset(lua_State *L)
 	htxn = MAY_LJMP(hlua_checktxn(L, 1));
 	offset = MAY_LJMP(luaL_checkinteger(L, 2));
 
-	if (offset < -0x7)
-		offset = -0x7;
-	else if (offset > 0x7)
-		offset = 0x7;
-
 	htxn->s->priority_offset = offset;
 
 	return 0;
diff --git a/src/queue.c b/src/queue.c
index cf445f97d..0f86fdb36 100644
--- a/src/queue.c
+++ b/src/queue.c
@@ -14,7 +14,7 @@
 #include 
 #include 
 #include 
-#include 
+#include 
 
 #include 
 #include 
@@ -26,23 +26,6 @@
 #include 
 
 
-#define NOW_OFFSET_BOUNDARY() (now_ms - (TIMER_LOOK_BACK >> 12) & 0xf)
-#define KEY_CLASS(key) (key & 0xfff0)
-#define KEY_OFFSET(key) (key & 0xf)
-#define KEY_CLASS_BOUNDARY(key) (KEY_CLASS(key) | NOW_OFFSET_BOUNDARY())
-
-static u32 key_incr(u32 key) {
-	u32 key_next = key + 1;
-
-	if (KEY_CLASS(key_next) != KEY_CLASS(key))
-		key_next = KEY_CLASS(key_next);
-	else if (key_next == KEY_CLASS_BOUNDARY(key))
-		key_next += 0x10;
-
-	return key_next;
-}
-
-
 struct pool_head *pool_head_pendconn;
 
 /* perform minimal intializations, report 0 in case of error, 1 if OK. */
@@ -100,57 +83,7 @@ static void pendconn_unlink(struct pendconn *p)
 		p->px->nbpend--;
 	}
 	HA_ATOMIC_SUB(>px->totpend, 1);
-	eb32_delete(>node);
-}
-
-/* Retrieve the next pending connection from the given pendconns ebtree with
- * key >= min.
- *
- * See pendconn_add for an explanation of the key & queue behavior.
- *
- * This function handles all the cases where due to the timestamp wrapping
- * the first node in the tree is not the highest priority.
- */
-static struct pendconn *pendconn_next(struct eb_root *pendconns, u32 min) {
-	struct eb32_node *node, *node2 = NULL;
-	u32 max;
-
-	// min is inclusive
-	// max is exclusive
-	max = KEY_CLASS_BOUNDARY(min);
-
-	node = eb32_lookup_ge(pendconns, min);
-
-	if (node) {
-		if (node->key < max || (max <= min && KEY_CLASS(node->key) == KEY_CLASS(min)))
-			return eb32_entry(node, struct pendconn, node);
-		if (KEY_CLASS(node->key) != KEY_CLASS(min))
-			node2 = node;
-		if (max > min)
-			goto class_next;
-	}
-
-	if (max <= min)
-		node = eb32_lookup_ge(pendconns, KEY_CLASS(min));
-	if (!node)
-		return NULL;
-	if (node->key < max && node->key < min)
-		return eb32_entry(node, struct pendconn, node);
-
-class_next:
-	if (node2) {
-		min = KEY_CLASS_BOUNDARY(node2->key);
-		if (node2->key >= min)
-			return eb32_entry(node2, struct pendconn, node);
-	} else
-		min = KEY_CLASS_BOUNDARY(min) + 0x10;
-	node = eb32_lookup_ge(pendconns, min);
-	if (node && KEY_CLASS(node->key) == KEY_CLASS(min))
-		return eb32_entry(node, struct pendconn, node);
-	if (node2)
-		return eb32_entry(node2, struct pendconn, node);
-
-	return NULL;
+	eb64_delete(>node);
 }
 
 /* Process the next pending connection from either a server or a proxy, and
@@ -174,8 +107,8 @@ class_next:
 static int pendconn_process_next_strm(struct server *srv, struct proxy *px)
 {
 	struct pendconn *p = NULL, *pp = NULL;
+	struct eb64_node *node;
 	struct server   *rsrv;
-	u32 pkey, ppkey;
 	int remote;
 
 	rsrv = srv->track;
@@ -183,18 +116,26 @@ static int pendconn_process_next_strm(struct server *srv, struct proxy *px)
 		rsrv = srv;
 
 	if (srv->nbpend) {
-		for (p = pendconn_next(>pendconns, NOW_OFFSET_BOUNDARY());
-		 p;
-		 p = pendconn_next(>pendconns, key_incr(p->node.key)))
+		for (node = eb64_first(>pendconns);
+		 node;
+ node = eb64_lookup_ge(>pendconns, node->key + 1)) {
+			p = eb64_entry(node, struct pendconn, node);
 			if (!HA_SPIN_TRYLOCK(PENDCONN_LOCK, >lock))
 break;
+		}
+		if (!node)
+			p = NULL;
 	}
 	if (px->nbpend) {
-		for (pp = pendconn_next(>pendconns, NOW_OFFSET_BOUNDARY());
-		 pp;
-		 pp = pendconn_next(>pendconns, key_incr(pp->node.key)))
+		for (node = eb64_first(>pendconns);
+		 node;
+		 node = eb64_lookup_ge(>pendconns, node->key + 1)) {
+			pp = eb64_entry(node, struct pendconn, node);
 			if (!HA_SPIN_TRYLOCK(PENDCONN_LOCK, >lock))
 break;
+		}
+		if (!node)
+			pp = NULL;
 	}
 
 	if (!p && !pp)
@@ -206,23 +147,7 @@ static int pendconn_process_next_strm(struct server *srv, struct proxy *px)
 		p = pp;
 		goto pendconn_found;
 	}
-	if (KEY_CLASS(p->node.key) < KEY_CLASS(pp->node.key)) {
-		

[PATCH 1/2] MEDIUM: add set-priority-class and set-priority-offset

[Patrick Hemmer]

This adds the set-priority-class and set-priority-offset actions to
http-request and tcp-request content.
The priority values are used when connections are queued to determine
which connections should be served first. The lowest priority class is
served first. When multiple requests from the same class are found, the
earliest (according to queue_time + offset) is served first.
---
 doc/configuration.txt  |  38 ++
 doc/lua-api/index.rst  |  18 +++
 include/types/proxy.h  |   3 +-
 include/types/queue.h  |   2 +-
 include/types/server.h |   3 +-
 include/types/stream.h |   7 +-
 src/cfgparse.c |  15 +++
 src/hlua.c |  74 ---
 src/log.c  |   4 +-
 src/proto_http.c   |   4 +-
 src/proxy.c|   2 +-
 src/queue.c| 345
++---
 src/server.c   |   2 +-
 src/stream.c   |  10 +-
 14 files changed, 453 insertions(+), 74 deletions(-)


diff --git a/doc/configuration.txt b/doc/configuration.txt
index cbea3309d..7ec010811 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -3911,6 +3911,7 @@ http-request { allow | auth [realm ] | redirect  | reject |
   replace-value|
   set-method  | set-path  | set-query  |
   set-uri  | set-tos  | set-mark  |
+  set-priority-class  | set-priority-offset 
   add-acl()  |
   del-acl()  |
   del-map()  |
@@ -4107,6 +4108,24 @@ http-request { allow | auth [realm ] | redirect  | reject |
   downloads). This works on Linux kernels 2.6.32 and above and requires
   admin privileges.
 
+- "set-priority-class" is used to set the queue priority class of the
+  current request. The value must be a sample expression which converts to
+  an integer in the range -2047..2047. Results outside this range will be
+  truncated. The priority class determines the order in which queued
+  requests are processed. Lower values have higher priority.
+
+- "set-priority-offset" is used to set the queue priority timestamp offset
+  of the current request. The value must be a sample expression which
+  converts to an integer in the range -524287..524287. Results outside this
+  range will be truncated. When a request is queued, it is ordered first by
+  the priority class, then by the current timestamp adjusted by the given
+  offset in milliseconds. Lower values have higher priority.
+  Note that the resulting timestamp is is only tracked with enough precision
+  for 524,287ms (8m44s287ms). If the request is queued long enough to where
+  the adjusted timestamp exceeds this value, it will be misidentified as
+  highest priority. Thus it is important to set "timeout queue" to a value,
+  where when combined with the offset, does not exceed this limit.
+
 - "add-acl" is used to add a new entry into an ACL. The ACL must be loaded
   from a file (even a dummy empty file). The file name of the ACL to be
   updated is passed between parentheses. It takes one argument: ,
@@ -9446,6 +9465,7 @@ tcp-request content  [{if | unless} ]
 - accept : the request is accepted
 - reject : the request is rejected and the connection is closed
 - capture : the specified sample expression is captured
+- set-priority-class  | set-priority-offset 
 - { track-sc0 | track-sc1 | track-sc2 }  [table ]
 - sc-inc-gpc0()
 - sc-inc-gpc1()
@@ -9507,6 +9527,24 @@ tcp-request content  [{if | unless} ]
   The "unset-var" is used to unset a variable. See above for details about
   .
 
+  The "set-priority-class" is used to set the queue priority class of the
+  current request. The value must be a sample expression which converts to an
+  integer in the range -2047..2047. Results outside this range will be
+  truncated. The priority class determines the order in which queued requests
+  are processed. Lower values have higher priority.
+
+  The "set-priority-offset" is used to set the queue priority timestamp offset
+  of the current request. The value must be a sample expression which converts
+  to an integer in the range -524287..524287. Results outside this range will be
+  truncated. When a request is queued, it is ordered first by the priority
+  class, then by the current timestamp adjusted by the given offset in
+  milliseconds. Lower values have higher priority.
+  Note that the resulting timestamp is is only tracked with enough precision for
+  524,287ms (8m44s287ms). If the request is queued long enough to where the
+  adjusted timestamp exceeds this value, it will be misidentified as highest
+  priority.  Thus it is important to set "timeout queue" to a value, where when
+  combined with the offset, does not exceed this limit.
+
   The "send-spoe-group" is used to trigger sending of a group of SPOE
   messages. To do so, the SPOE engine used to send messages must be defined, as
   well as the SPOE 

Re: [PATCH 1/2] MEDIUM: add set-priority-class and set-priority-offset

[Patrick Hemmer]

On 2018/5/10 01:35, Patrick Hemmer wrote:
> This adds the set-priority-class and set-priority-offset actions to
> http-request and tcp-request content.
> The priority values are used when connections are queued to determine
> which connections should be served first. The lowest priority class is
> served first. When multiple requests from the same class are found, the
> earliest (according to queue_time + offset) is served first.
> ---
>  include/types/proxy.h  |   2 +-
>  include/types/queue.h  |   2 +-
>  include/types/server.h |   2 +-
>  include/types/stream.h |   2 +
>  src/proxy.c|   2 +-
>  src/queue.c| 280
> ++---
>  src/server.c   |   2 +-
>  src/stream.c   |   2 +
>  8 files changed, 250 insertions(+), 44 deletions(-)
>
>
Noticed I had some uncommitted changes. Not much, but previous version
might not work right.

-Patrick
From 240a54488c01c4b0a15a561b8e7533922a487492 Mon Sep 17 00:00:00 2001
From: Patrick Hemmer <hapr...@stormcloud9.net>
Date: Fri, 4 May 2018 16:31:16 -0400
Subject: [PATCH] MEDIUM: add set-priority-class and set-priority-offset

This adds the set-priority-class and set-priority-offset actions to 
http-request and tcp-request content.
The priority values are used when connections are queued to determine which 
connections should be served first. The lowest priority class is served first. 
When multiple requests from the same class are found, the earliest (according 
to queue_time + offset) is served first.
---
 include/types/proxy.h  |   2 +-
 include/types/queue.h  |   2 +-
 include/types/server.h |   2 +-
 include/types/stream.h |   2 +
 src/hlua.c |   4 +-
 src/proxy.c|   2 +-
 src/queue.c| 280 ++---
 src/server.c   |   2 +-
 src/stream.c   |   2 +
 9 files changed, 252 insertions(+), 46 deletions(-)

diff --git a/include/types/proxy.h b/include/types/proxy.h
index 16c13a1c1..2cc1dfd9e 100644
--- a/include/types/proxy.h
+++ b/include/types/proxy.h
@@ -322,7 +322,7 @@ struct proxy {
int serverfin;  /* timeout to apply to server 
half-closed connections */
} timeout;
char *id, *desc;/* proxy id (name) and 
description */
-   struct list pendconns;  /* pending connections with no 
server assigned yet */
+   struct eb_root pendconns;   /* pending connections 
with no server assigned yet */
int nbpend; /* number of pending 
connections with no server assigned yet */
int totpend;/* total number of pending 
connections on this instance (for stats) */
unsigned int feconn, beconn;/* # of active frontend and 
backends streams */
diff --git a/include/types/queue.h b/include/types/queue.h
index 42dbbd047..03377da69 100644
--- a/include/types/queue.h
+++ b/include/types/queue.h
@@ -35,7 +35,7 @@ struct pendconn {
struct stream *strm;
struct proxy  *px;
struct server *srv;/* the server we are waiting for, may be 
NULL */
-   struct listlist;   /* next pendconn */
+   struct eb32_node node;
__decl_hathreads(HA_SPINLOCK_T lock);
 };
 
diff --git a/include/types/server.h b/include/types/server.h
index 0cd20c096..5339c911e 100644
--- a/include/types/server.h
+++ b/include/types/server.h
@@ -213,7 +213,7 @@ struct server {
struct freq_ctr sess_per_sec;   /* sessions per second on this 
server */
struct be_counters counters;/* statistics counters */
 
-   struct list pendconns;  /* pending connections */
+   struct eb_root pendconns;   /* pending connections 
*/
struct list actconns;   /* active connections */
struct list *priv_conns;/* private idle connections 
attached to stream interfaces */
struct list *idle_conns;/* sharable idle connections 
attached or not to a stream interface */
diff --git a/include/types/stream.h b/include/types/stream.h
index 0dbc79f44..489a0b648 100644
--- a/include/types/stream.h
+++ b/include/types/stream.h
@@ -125,6 +125,8 @@ struct stream {
 
struct server *srv_conn;/* stream already has a slot on a 
server and is not in queue */
struct pendconn *pend_pos;  /* if not NULL, points to the pending 
position in the pending queue */
+   int16_t priority_class; /* priority class of the stream for the 
pending queue */
+   int32_t priority_offset;/* priority offset of the stream for 
the pending queue */
 
struct http_txn *txn;   /* current HTTP transaction being 
processed. Should become a list. */
 
diff --git a/src/hlua.c b/src/hlua.c
index 65fa62ff6

[PATCH 2/2] tests for queue set-priority

[Patrick Hemmer]

---
 Makefile.test|  21 +
 tests/test-queue.cfg |   8 
 tests/test_queue.c   | 117
+++
 3 files changed, 146 insertions(+)


diff --git a/Makefile.test b/Makefile.test
new file mode 100644
index 0..41dd03c71
--- /dev/null
+++ b/Makefile.test
@@ -0,0 +1,21 @@
+include Makefile
+
+tests/haproxy_test.c: src/haproxy.c
+	sed -e 's/^int main(/int __main(/' < $< > $@
+
+tests/test_queue.o: tests/test_queue.c $(DEP)
+	$(CC) $(COPTS) \
+	  -DBUILD_TARGET='"$(strip $(TARGET))"' \
+	  -DBUILD_ARCH='"$(strip $(ARCH))"' \
+	  -DBUILD_CPU='"$(strip $(CPU))"' \
+	  -DBUILD_CC='"$(strip $(CC))"' \
+	  -DBUILD_CFLAGS='"$(strip $(VERBOSE_CFLAGS))"' \
+	  -DBUILD_OPTIONS='"$(strip $(BUILD_OPTIONS))"' \
+	   -c -o $@ $<
+
+tests/test_queue: tests/test_queue.o $(OPTIONS_OBJS) $(EBTREE_OBJS) $(OBJS)
+	$(LD) $(LDFLAGS) -o $@ $(filter-out src/haproxy.o,$^) $(LDOPTS)
+
+.PHONY: test
+test: tests/test_queue
+	./tests/test_queue
diff --git a/tests/test-queue.cfg b/tests/test-queue.cfg
new file mode 100644
index 0..903cf22d0
--- /dev/null
+++ b/tests/test-queue.cfg
@@ -0,0 +1,8 @@
+defaults
+	timeout client 5s
+	timeout server 5s
+	timeout connect 5s
+
+listen l1
+	bind :8001
+	server s1 127.0.0.1: maxconn 1
diff --git a/tests/test_queue.c b/tests/test_queue.c
new file mode 100644
index 0..a92bd0233
--- /dev/null
+++ b/tests/test_queue.c
@@ -0,0 +1,117 @@
+#include "haproxy_test.c"
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+// Usage:
+// make -f Makefile.test USE_THREAD=1 DEBUG=-DDEBUG_THREAD TARGET=XXX test
+
+#ifndef USE_THREAD
+#error USE_THREAD is not set (USE_THREAD=1)
+#endif
+#ifndef DEBUG_THREAD
+#error DEBUG_THREAD is not set (DEBUG=-DDEBUG_THREAD)
+#endif
+
+struct test_spec_pendconn {
+	int key;
+	int locked;
+};
+
+struct test_spec {
+	struct test_spec_pendconn pendconns[3];
+	int expected;
+};
+
+int main(void)
+{
+	int argc = 3;
+	char *argv[] = {"haproxy","-f","tests/test-queue.cfg"};
+	init(argc, argv);
+
+	struct proxy *px;
+	struct server *srv;
+	struct stream *strm;
+
+	for (px = proxies_list; px; px = px->next)
+		if (!strcmp(px->id, "l1"))
+			break;
+	srv = px->srv;
+
+	struct test_spec tests[] = {
+		/*  0  1  2 */
+		/* [0123456789ABCDEF],[0123456789ABCDEF],[0123456789ABCDEF] */
+		/* [|...*...],[|...],[|...] */ {{{0x0c}},0},
+		/* [...*|...],[|...],[|...] */ {{{0x03}},0},
+		/* [...*|...],[...*|...],[|...] */ {{{0x03},{0x13}},0},
+		/* [...*|...*...],[|...],[|...] */ {{{0x03},{0x0c}},1},
+		/* [...*|X..*...],[|...],[|...] */ {{{0x03},{0x09,1},{0x0c}},2},
+		/* [...*|...X...],[|...],[|...] */ {{{0x03},{0x0c,1}},0},
+		/* [...*|...X...],[...X|...],[|...] */ {{{0x03},{0x0c,1},{0x13,1}},0},
+		/* [|...X...],[...X|...*...],[|...] */ {{{0x0c,1},{0x13,1},{0x1c}},2},
+		/* [|...X...],[...*|...X...],[|...] */ {{{0x0c,1},{0x13},{0x1c,1}},1},
+		/* [|...X...],[...*|...],[|...] */ {{{0x0c,1},{0x13}},1},
+		/* [|...X...],[...*|...*...],[|...] */ {{{0x0c,1},{0x13},{0x1c}},2},
+		/* [|...],[...*|...],[|...] */ {{{0x13}},0},
+		/* [|...],[...*|...*...],[|...] */ {{{0x13},{0x1c}},1},
+		/* [|...],[|...*...],[|...] */ {{{0x1c}},0},
+		/* [|...],[...*|...],[|...] */ {{{0x13}},0},
+		/* [|...X...],[...*|...],[..*.|...] */ {{{0x0c,1},{0x13},{0x22}},1},
+		/* [|...X...],[|...],[|...] */ {{{0x0c,1}},-1},
+		/* [*...*...],[|...],[|...] */ {{{0x08},{0x0c}},0},
+		/* [...**...],[|...],[|...] */ {{{0x03},{0x08}},1},
+		/* [|...],[...**...],[|...] */ {{{0x13},{0x1c}},1},
+	};
+
+	now_ms = 0x0;
+	struct eb32_node *node;
+	struct test_spec test;
+	for (int i = 0; i < sizeof(tests)/sizeof(tests[0]); i++ ) {
+		while ((node = eb32_first(>pendconns)))
+			eb32_delete(node);
+		srv->served = 0;
+
+		test = tests[i];
+		struct pendconn *pcs[sizeof(test.pendconns)/sizeof(test.pendconns[0])];
+
+		for (int j = 0; j < sizeof(test.pendconns)/sizeof(test.pendconns[0]); j++) {
+			if (!test.pendconns[j].key) {
+pcs[j] = NULL;
+break;
+			}
+
+			strm = stream_new(session_new(px, NULL, NULL), NULL);
+			strm->priority_class = (test.pendconns[j].key >> 4) - 0x7ff;
+			strm->priority_offset = (test.pendconns[j].key & 0xf) * 0x1;
+			if (strm->priority_offset >= 0x8)
+strm->priority_offset -= 0x10;
+
+			pcs[j] = pendconn_add(strm);
+
+			if 

[PATCH 0/2] Re: Priority based queuing

[Patrick Hemmer]

On 2018/5/5 13:55, Willy Tarreau wrote:
> On Sat, May 05, 2018 at 01:33:51PM -0400, Patrick Hemmer wrote:
>>> Also I'm thinking that we can even use 32-bit by putting the frontier
>>> between the date and the fixed priority (let's call it class) somewhere
>>> else :
>>>   - 8 bit class + 24 bit offset  => 256 classes and +/- 2.3 hours
offsets
>>>   - 12 bit class + 20 bit offset => 4096 classes and +/- 8 minutes
offsets
>>>   - 16 bit class + 16 bit offset => 64k classes and +/- 32 second
offsets
>> What's the benefit to using 32-bit over 64-bit, and is that benefit
>> worth the cost of the added code complexity and processing time?
>
> Slightly smaller memory footprint (doesn't count much), but more
> importantly much smaller processing time especially on 32-bit systems.
I would have expected the extra processing for wrapping to negate the
benefit.

>
>> If we
>> used 64-bit, we could do a 16/48 bit split and have an absolute
>> timestamp out to year 10889 at millisecond precision.
>
> Not really because that would force us to implement yet another clock
> and then to have two distinct set of clocks for queues and for all other
> internal events and timeouts (including the queue timeout!).
We already have the `now` and `date` variables. The priority is only
used for the queue key, and not to replace anything else. The queue key
is already getting replaced, so it would be no additional work.

> That would
> really be the beginning of a mess. We've been running with 32-bit
> millisecond-precise ticks for over a decade without ever a complain that
> it was not enough nor that the wrapping at 49.7 days was an issue. If
> one day we'd decide to change this, it would be to support finer-grained
> timestamps (eg: nanoseconds or CPU cycles) which would not necessarily
> protect us from wrapping either.
>
>>> I'm pretty sure that the two first options are way enough to cover
almost
>>> everyone's needs.
>> For me a 12-bit class should work. My intention is to implement
>> something similar to email spam filtering, where you generate a score
>> based on the rules that match, and some rules are worth more than
>> others. 8 bits might be a bit difficult to work within.
>
> Makes sense.
>
>> I also suspect the 16-bit option (32 second offset) is too small for
>> date based.
>
> Yes I agree. I'm pretty sure that for some TCP connections certain users
> will want to have a long delay (eg: wait a minute before your FTP transfer
> starts).
>
>> The rest of this all seems fine. I've got no other thoughts to add.
>
> Great, thanks!
>
>> So the question is, which route do we go?
>> 1. 32-bit int (12/20 split)
>> 2. 64-bit int (32/32 split)
>
> I think you can start with the 12/20 split first, knowing that if
> anybody ever requests an extension of either time or classes we can
> switch to 64 without much effort. Until this happens we'll be more
> efficient on small systems. Conversely we can't do it the other way
> around because we could break some setups :-)
>
>> 3. 64-bit int (16/48 split) with absolute timestamps
>
> I don't like it much for the reasons above.
>
>> 4. 64-bit int with a single `set-priority` action. Let the user use the
>> a new`ms()` fetch for time based. The user does the bit shifting if they
>> want to use both (using the `mul()` converter, or we can add a
`shift()`).
>
> I think this level of complexity ought to be done once in the code and not
> each and every time for all users. It would only be manageable for a
number
> of the people on this list, but newcomers would call us words for this :-)
>
> Thanks,
> Willy

Ok, so this patch set includes a fully functional implementation of the
priority class & offset. Unfortunately handling the offset got really
complicated due to the wrapping & locking.
There might be a little room for improvement, but I don't think much.

I'm still working on the code, but I wanted to submit this for review as
the final version probably won't be much different.

The second patch is a test I wrote to try and catch all the different
scenarios. I'm including in case you were inclined to play with the tree
search algorithm and test it. It's not intended to be merged.

Things to do:
* Try and optimize the macros & tree search.
* Output a warning if `timeout queue` > max offset.
* Add LUA methods & sample fetches.
* General cleanup.
* Docs

-Patrick


Patrick Hemmer (2):
  MEDIUM: add set-priority-class and set-priority-offset
  tests for queue set-priority

 Makefile.test  |  21 
 include/types/proxy.h  |   2 +-
 include/types/queue.h  |   2 +-
 include/types/server.h |   2 +-
 include/types/stream.h |   2 +
 src/prox

[PATCH 1/2] MEDIUM: add set-priority-class and set-priority-offset

[Patrick Hemmer]

This adds the set-priority-class and set-priority-offset actions to
http-request and tcp-request content.
The priority values are used when connections are queued to determine
which connections should be served first. The lowest priority class is
served first. When multiple requests from the same class are found, the
earliest (according to queue_time + offset) is served first.
---
 include/types/proxy.h  |   2 +-
 include/types/queue.h  |   2 +-
 include/types/server.h |   2 +-
 include/types/stream.h |   2 +
 src/proxy.c|   2 +-
 src/queue.c| 280
++---
 src/server.c   |   2 +-
 src/stream.c   |   2 +
 8 files changed, 250 insertions(+), 44 deletions(-)


diff --git a/include/types/proxy.h b/include/types/proxy.h
index 16c13a1c1..2cc1dfd9e 100644
--- a/include/types/proxy.h
+++ b/include/types/proxy.h
@@ -322,7 +322,7 @@ struct proxy {
 		int serverfin;  /* timeout to apply to server half-closed connections */
 	} timeout;
 	char *id, *desc;			/* proxy id (name) and description */
-	struct list pendconns;			/* pending connections with no server assigned yet */
+	struct eb_root pendconns;			/* pending connections with no server assigned yet */
 	int nbpend;/* number of pending connections with no server assigned yet */
 	int totpend;/* total number of pending connections on this instance (for stats) */
 	unsigned int feconn, beconn;		/* # of active frontend and backends streams */
diff --git a/include/types/queue.h b/include/types/queue.h
index 42dbbd047..03377da69 100644
--- a/include/types/queue.h
+++ b/include/types/queue.h
@@ -35,7 +35,7 @@ struct pendconn {
 	struct stream *strm;
 	struct proxy  *px;
 	struct server *srv;/* the server we are waiting for, may be NULL */
-	struct listlist;   /* next pendconn */
+	struct eb32_node node;
 	__decl_hathreads(HA_SPINLOCK_T lock);
 };
 
diff --git a/include/types/server.h b/include/types/server.h
index 0cd20c096..5339c911e 100644
--- a/include/types/server.h
+++ b/include/types/server.h
@@ -213,7 +213,7 @@ struct server {
 	struct freq_ctr sess_per_sec;		/* sessions per second on this server */
 	struct be_counters counters;		/* statistics counters */
 
-	struct list pendconns;			/* pending connections */
+	struct eb_root pendconns;			/* pending connections */
 	struct list actconns;			/* active connections */
 	struct list *priv_conns;		/* private idle connections attached to stream interfaces */
 	struct list *idle_conns;		/* sharable idle connections attached or not to a stream interface */
diff --git a/include/types/stream.h b/include/types/stream.h
index 0dbc79f44..489a0b648 100644
--- a/include/types/stream.h
+++ b/include/types/stream.h
@@ -125,6 +125,8 @@ struct stream {
 
 	struct server *srv_conn;/* stream already has a slot on a server and is not in queue */
 	struct pendconn *pend_pos;  /* if not NULL, points to the pending position in the pending queue */
+	int16_t priority_class; /* priority class of the stream for the pending queue */
+	int32_t priority_offset;/* priority offset of the stream for the pending queue */
 
 	struct http_txn *txn;   /* current HTTP transaction being processed. Should become a list. */
 
diff --git a/src/proxy.c b/src/proxy.c
index 31253f14d..88e274b6b 100644
--- a/src/proxy.c
+++ b/src/proxy.c
@@ -726,7 +726,7 @@ void init_new_proxy(struct proxy *p)
 {
 	memset(p, 0, sizeof(struct proxy));
 	p->obj_type = OBJ_TYPE_PROXY;
-	LIST_INIT(>pendconns);
+	memset(>pendconns, 0, sizeof(p->pendconns));
 	LIST_INIT(>acl);
 	LIST_INIT(>http_req_rules);
 	LIST_INIT(>http_res_rules);
diff --git a/src/queue.c b/src/queue.c
index 1c730c75c..28d36548d 100644
--- a/src/queue.c
+++ b/src/queue.c
@@ -14,12 +14,16 @@
 #include 
 #include 
 #include 
+#include 
 
+#include 
 #include 
+#include 
 #include 
 #include 
 #include 
 #include 
+#include 
 
 
 struct pool_head *pool_head_pendconn;
@@ -76,8 +80,69 @@ static void pendconn_unlink(struct pendconn *p)
 	else
 		p->px->nbpend--;
 	HA_ATOMIC_SUB(>px->totpend, 1);
-	LIST_DEL(>list);
-	LIST_INIT(>list);
+	eb32_delete(>node);
+}
+
+#define offset_boundary() (now_ms - (TIMER_LOOK_BACK >> 12) & 0xf)
+#define key_class(key) (key & 0xfff0)
+#define key_offset(key) (key & 0xf)
+#define key_class_boundary(key) (key_class(key) | offset_boundary())
+static u32 key_incr(u32 key) {
+	u32 key_next;
+
+	key_next = key + 1;
+	if (key_class(key_next) != key_class(key))
+		key_next = key_class(key_next);
+	else if (key_next == key_class_boundary(key))
+		key_next += 0x10;
+
+	return key_next;
+}
+
+static struct pendconn *next_pendconn(struct eb_root *pendconns, u32 min) {
+	struct eb32_node *node, *node2 = NULL;
+	u32 max;
+
+	// min is inclusive
+	// max is exclusive
+	max = key_class_boundary(min);
+
+	node = eb32_lookup_ge(pendconns, min);
+
+	if (node) {
+		if (node->key < max || (max <= min && key_class(node->key) == 

Re: Priority based queuing

[Patrick Hemmer]

On 2018/5/5 01:29, Willy Tarreau wrote:
> On Fri, May 04, 2018 at 06:49:00PM -0400, Patrick Hemmer wrote:
>> I'm not quite following the need for multiple queues. Why wouldn't you
>> just have one sorted queue, where if multiple pending requests have the
>> same priority, then they're FIFO.
> That's what the time-ordered queue does. I proposed this so that you can
> ensure that lower priorities are always processed first until there is no
> more of the same level.
Ah, I was considering the 2 solutions as an either-or choice. Not that
you'd use both in the same configuration, or at least in the same backend.

>> The queue was implemented using the existing pendconns code. The only
>> thing that was changed was to insert in sorted order. Obviously this is
>> just a quick and dirty implementation. There are lots of priority queue
>> implementations to choose from, and I don't know which one is best
>> suited to this kind of task. We could keep the existing sorted
>> linked-list, and just do some optimizations for insertion. Or we could
>> switch to some sort of tree. The linked-list has the advantage in that I
>> suspect that the vast majority of additions will append to the end, and
>> pops are always off the front, so well suited to a linked list. A tree
>> might become very unbalanced very fast, and require lots of maintenance.
> No, trees are quite cheap, we use them absolutely everywhere now. Just
> take a look at wake_expired_tasks() to get an idea. In fact every single
> time we've been relying on lists doing linear lookups, we've got several
> bug reports about haproxy eating all the CPU because some people were
> using it in a way that wasn't initially considered as reasonable but
> which made a lot of sense to them.
This is why I liked the idea of a single implementation. It gave the
flexibility to the user to do whatever they wanted (time or class
based). Granted I was only thinking of doing one or the other, but the
user could still do both, they would just have to manage dedicating a
range to the date/class. Maybe someone would want to consider date
before class.

> In this case you'd be better of with the principle consisting in using
> a larger integer with the priority at the top and the date at the bottom.
> But that's where I said that it would require two lookups to deal with
> wrapping date so I'm not sure it's any better than using multiple queues.
> Well, at least it's less maintenance burden. The detailed principle is
> the following :
>
>uint64_t priority :
>
> 6332 31 0
> [ hard priority | date   ]
>
>next = lookup_first(queue);
>if (!next)
>   return;
>  
>=> next is the lowest value of both priority and date
>
> Then you perform a second lookup with the oldest past date for the same
> priority level :
>
>key = (next->key & 0xULL) | (now_ms - TIMER_LOOK_BACK);
>next2 = lookup_ge(queue, key);
>if (next2 && !((next2->value ^ next->value) >> 32))
>   next=next2
>
> Then you dequeue next which is always the next one.
>
> That's where I thought that using two distinct levels was simpler, but
> if we factor in the fact that it would limit the number of priority
> classes and would require more maintenance code, it's probably not
> worth the hassle to save the 4 extra lines we have above to respect
> the wrapping date.
>
> Also I'm thinking that we can even use 32-bit by putting the frontier
> between the date and the fixed priority (let's call it class) somewhere
> else :
>   - 8 bit class + 24 bit offset  => 256 classes and +/- 2.3 hours offsets
>   - 12 bit class + 20 bit offset => 4096 classes and +/- 8 minutes offsets
>   - 16 bit class + 16 bit offset => 64k classes and +/- 32 second offsets
What's the benefit to using 32-bit over 64-bit, and is that benefit
worth the cost of the added code complexity and processing time? If we
used 64-bit, we could do a 16/48 bit split and have an absolute
timestamp out to year 10889 at millisecond precision.

> I'm pretty sure that the two first options are way enough to cover almost
> everyone's needs.
For me a 12-bit class should work. My intention is to implement
something similar to email spam filtering, where you generate a score
based on the rules that match, and some rules are worth more than
others. 8 bits might be a bit difficult to work within.
I also suspect the 16-bit option (32 second offset) is too small for
date based.

> Let's say for a moment that we'd use the second option
> (12+20 bits), the queue insertion code becomes this :
>
>pendconn->node.key = (stream->prio_class << 20) +
> (now_ms + stream->pr

Re: Priority based queuing

[Patrick Hemmer]

On 2018/5/2 22:56, Willy Tarreau wrote:
> On Wed, May 02, 2018 at 04:29:33PM -0400, Patrick Hemmer wrote:
>> I think you're misunderstanding my design, as scoring wouldn't work like
>> this at all. If you give the gold class a score of 1000 (where higher
>> number means higher priority), then the only thing that would get
>> processed before gold class would be another class that has a score >
>> 1000. If nothing does, and a gold request comes in, it gets processed
>> first, no matter how big the queue is.
>> Think of scoring as instead having multiple queues. Each queue is only
>> processed if the queue before it is empty.
> (...)
>
> OK you convinced me. Not on everything, but on the fact that we're trying
> to address different points. My goal is to make it possible to improve
> quality of service between good requests, and your goal is to improve the
> classification between good, suspicious, and bad requests. Each of us see
> how to expand a little bit our respective model to address part of the
> other one (though less efficiently).
>
> I agree that for your goal, multi-queue is better, but I still maintain
> that for my goal, the time-based queue gives better control. The good
> news is that the two are orthogonal and 100% compatible.
>
> Basically the queueing system should be redesigned as a list of time-based
> trees that are visited in order of priority (or traffic classes, we'll have
> to find the proper wording to avoid confusion). If you think you can address
> your needs with just a small set of different priorities, probably that we
> can implement this with a small array (2-4 queues). If you think you need
> more, then we'd rather think about building a composite position value in
> the tree made of the priority at the top of the word and the service time
> at the bottom of the word. This way, picking the first value will always
> find the lowest priority value first. There's one subtlety related to
> wrapping time there however, but it can be addressed in two lookups.
>
> Please let me know if you'd be fine with designing and implementing
> something like this.
>
> Cheers,
> Willy

I'm not quite following the need for multiple queues. Why wouldn't you
just have one sorted queue, where if multiple pending requests have the
same priority, then they're FIFO.

I had sent another email which proposed an implementation that would
satisfy both designs
(https://www.mail-archive.com/haproxy@formilux.org/msg29876.html).
Unfortunately I was composing it when you sent your most recent reply,
so I didn't see it before I sent.
I went ahead and implemented the functionality I had talked about in
that other email, and have attached a WIP patch. I've done some testing
on it, and it seems to work fine. The syntax is exactly as proposed:

# time-based priority
http-request set-priority date(20) if LOW_PRIORITY
http-request set-priority date(-20) if HIGH_PRIORITY

# score-based priority
http-request set-priority int(20) if LOW_PRIORITY
http-request set-priority int(-20) if HIGH_PRIORITY

Some notes on the implementation:

I used a `long` for tracking the priority. Obviously this has
limitations when used with date(), as the timestamps are fairly large. I
can change this to `long long`.

The queue was implemented using the existing pendconns code. The only
thing that was changed was to insert in sorted order. Obviously this is
just a quick and dirty implementation. There are lots of priority queue
implementations to choose from, and I don't know which one is best
suited to this kind of task. We could keep the existing sorted
linked-list, and just do some optimizations for insertion. Or we could
switch to some sort of tree. The linked-list has the advantage in that I
suspect that the vast majority of additions will append to the end, and
pops are always off the front, so well suited to a linked list. A tree
might become very unbalanced very fast, and require lots of maintenance.

Using the date() fetch obviously doesn't provide good precision. We
could add a `datems()`, `datens()`, or `ticks()` (ms since start) sample
fetch. But obviously we would need to use `long long` for that, Though
ticks could use a `long`, but would wrap at around 16 months.

I still plan on adding a sample fetch for obtaining the current priority
value, and a lua function for setting it. Not sure if we want a lua
function for retrieving, or if using the fetch is good enough.

-Patrick

From 4783dccfc869cfb4219d195ba21f896272211c9e Mon Sep 17 00:00:00 2001
From: Patrick Hemmer <hapr...@stormcloud9.net>
Date: Fri, 4 May 2018 16:31:16 -0400
Subject: [PATCH] MINOR: add {http,tcp}-request set-priority for queue
 prioritization

This adds the set-priority action to http-request and tcp-request content.
The priority value is used when connections are queued to determi

Re: [PATCH] MINOR: Add server name & puid to LUA Server class.

[Patrick Hemmer]

On 2018/5/3 02:52, Thierry Fournier wrote:
>> On 2 May 2018, at 16:49, Willy Tarreau <w...@1wt.eu> wrote:
>>
>> Hi Thierry,
>>
>> when you have a moment, could you please give a quick look at these
>> patches from Patrick so that I know if I can merge them or not ? There
>> are 2 other ones on the list.
>
> Hi Willy and Patrick,
>
> I check it. I don’t understand why you convert the puid in string.
> You could add directly the ouid integer as is in a Lua variable with
> the function lua_pushinteger().
>
> Thierry
I did this for consistency, as this is how Proxy.uuid behaves. Even
though it could return an integer, it converts it to a string and
returns that instead.

>> Thanks,
>> Willy
>>
>> On Sun, Apr 29, 2018 at 02:23:48PM -0400, Patrick Hemmer wrote:
>>> ---
>>> doc/lua-api/index.rst |  8 
>>> src/hlua_fcn.c| 14 ++
>>> 2 files changed, 22 insertions(+)
>>>
>>>
>>> diff --git a/doc/lua-api/index.rst b/doc/lua-api/index.rst
>>> index 7a77e46ee..cdb285957 100644
>>> --- a/doc/lua-api/index.rst
>>> +++ b/doc/lua-api/index.rst
>>> @@ -925,6 +925,14 @@ Server class
>>>
>>>   This class provides a way for manipulating servers and retrieving 
>>> information.
>>>
>>> +.. js:attribute:: Server.name
>>> +
>>> +  Contain the name of the server.
>>> +
>>> +.. js:attribute:: Server.puid
>>> +
>>> +  Contain the proxy unique identifier of the server.
>>> +
>>> .. js:function:: Server.is_draining(sv)
>>>
>>>   Return true if the server is currently draining sticky connections.
>>> diff --git a/src/hlua_fcn.c b/src/hlua_fcn.c
>>> index a8d53d45b..280d8e5af 100644
>>> --- a/src/hlua_fcn.c
>>> +++ b/src/hlua_fcn.c
>>> @@ -490,6 +490,8 @@ int hlua_listener_get_stats(lua_State *L)
>>>
>>> int hlua_fcn_new_server(lua_State *L, struct server *srv)
>>> {
>>> +   char buffer[10];
>>> +
>>> lua_newtable(L);
>>>
>>> /* Pop a class sesison metatable and affect it to the userdata. */
>>> @@ -498,6 +500,18 @@ int hlua_fcn_new_server(lua_State *L, struct server 
>>> *srv)
>>>
>>> lua_pushlightuserdata(L, srv);
>>> lua_rawseti(L, -2, 0);
>>> +
>>> +   /* Add server name. */
>>> +   lua_pushstring(L, "name");
>>> +   lua_pushstring(L, srv->id);
>>> +   lua_settable(L, -3);
>>> +
>>> +   /* Add server puid. */
>>> +   lua_pushstring(L, "puid");
>>> +   snprintf(buffer, sizeof(buffer), "%d", srv->puid);
>>> +   lua_pushstring(L, buffer);
>>> +   lua_settable(L, -3);
>>> +
>>> return 1;
>>> }
>>>
>>>
>

Re: Priority based queuing

[Patrick Hemmer]

On 2018/5/2 16:29, Patrick Hemmer wrote:
>
>
> On 2018/5/2 13:22, Willy Tarreau wrote:
>> On Wed, May 02, 2018 at 12:44:06PM -0400, Patrick Hemmer wrote:
>>> Can you elaborate on what you're thinking of for a time-based queue?
>>>
>>> What I'm imagining you mean is that you would write a rule to set the
>>> max queue time, and haproxy would insert it into the queue sorting on
>>> TIME_NOW() + MAX_QUEUE_TIME. The main difference I see to this approach
>>> vs scoring, is that you ensure that an item doesn't sit on the queue
>>> forever (or whatever `timeout queue` is set to) if higher priority stuff
>>> keeps getting inserted before it.
>> In part, but mostly that under contention you can still maintain a
>> good quality of service. I remember having had a discussion a very
>> long time ago with a gaming site operator explaining that he wanted
>> to send profile management requests to a dedicated slow server so
>> that people filling in their profile do not disturb the ones playing.
>> For me it's a good example : "how long do you think these people are
>> willing to wait for the server to respond to each click to update
>> their profile?". Let's say 2 seconds is OK, you just add a 2s offset
>> to the position in the queue for such requests. If in the mean time
>> other higher priority requests come in, the delayed one may face the
>> 2 seconds delay. But it will not face more. And obviously if no other
>> request is pending before it in the queue it will be served earlier.
>>
>> A similar example is for certain shopping sites which consider that
>> once you have something in your shopping cart, you're much more likely
>> to finish the process and to pay because you've found the item you
>> were looking for, so you're more willing to tolerate a slightly higher
>> response time, and as such provide a better response time to newcomers.
>> But while this small delay can easily be expressed in milliseconds
>> (probably 50/100), it's almost impossible to define it using positions.
>> What if 50 new visitors suddenly come in ? You don't want the guy with
>> his item to suddenly experience a timeout. The time-based queue however
>> grants you a control over the service degradation you're accepting to
>> inflict to your users.
> There's a key difference in our designs. You're trying to classify
> traffic based on destination. I'm trying to classify traffic based on
> source.
> In my design, the maxconn is set to a level such that the server is
> healthy and every request can be processed fast. With this, and a
> score based queue, and under attack, the good requests will be drained
> fast, leaving only the bad requests. Thus no normal user should be
> getting a timeout, no matter what resource they're going to.
> Your design works when the backend system can't keep up with good
> legitimate traffic, and you need to prioritize one good request over
> another good request. My design is when the backend system can't keep
> up with bad traffic, and so we want to process the good traffic first.
>
>> Another interesting example is when you want ot provide strong service
>> guarantees to some top-level visitors while running under intense load.
>> By using only a position, it's easy to say "I want the Gold class to
>> advance by 1000 positions". But if the site is highly loaded and you
>> have 10k pending requests, these 1000 positions remain irrelevant,
>> because the requests sits there, waiting for 90% of the pollution to
>> be flushed. 
> I think you're misunderstanding my design, as scoring wouldn't work
> like this at all. If you give the gold class a score of 1000 (where
> higher number means higher priority), then the only thing that would
> get processed before gold class would be another class that has a
> score > 1000. If nothing does, and a gold request comes in, it gets
> processed first, no matter how big the queue is.
> Think of scoring as instead having multiple queues. Each queue is only
> processed if the queue before it is empty.
>> If you say "I want these requests to advance by 10 seconds"
>> then no matter how many other requests you have in the queue, it will
>> really advance by 10 seconds and effectively shrink the response time
>> by 10 seconds.
> It may shrink the response time by 10 seconds, but if the normal
> response time is 60 seconds, that's still 50 seconds of waiting. If
> your users are only willing to put up with 30 seconds of wait, this
> means you end up failing *everyone*.
>
>> Overall for user experience it's important to think in time and not
>> positions. The ratio

Re: Priority based queuing

[Patrick Hemmer]

On 2018/5/2 13:22, Willy Tarreau wrote:
> On Wed, May 02, 2018 at 12:44:06PM -0400, Patrick Hemmer wrote:
>> Can you elaborate on what you're thinking of for a time-based queue?
>>
>> What I'm imagining you mean is that you would write a rule to set the
>> max queue time, and haproxy would insert it into the queue sorting on
>> TIME_NOW() + MAX_QUEUE_TIME. The main difference I see to this approach
>> vs scoring, is that you ensure that an item doesn't sit on the queue
>> forever (or whatever `timeout queue` is set to) if higher priority stuff
>> keeps getting inserted before it.
> In part, but mostly that under contention you can still maintain a
> good quality of service. I remember having had a discussion a very
> long time ago with a gaming site operator explaining that he wanted
> to send profile management requests to a dedicated slow server so
> that people filling in their profile do not disturb the ones playing.
> For me it's a good example : "how long do you think these people are
> willing to wait for the server to respond to each click to update
> their profile?". Let's say 2 seconds is OK, you just add a 2s offset
> to the position in the queue for such requests. If in the mean time
> other higher priority requests come in, the delayed one may face the
> 2 seconds delay. But it will not face more. And obviously if no other
> request is pending before it in the queue it will be served earlier.
>
> A similar example is for certain shopping sites which consider that
> once you have something in your shopping cart, you're much more likely
> to finish the process and to pay because you've found the item you
> were looking for, so you're more willing to tolerate a slightly higher
> response time, and as such provide a better response time to newcomers.
> But while this small delay can easily be expressed in milliseconds
> (probably 50/100), it's almost impossible to define it using positions.
> What if 50 new visitors suddenly come in ? You don't want the guy with
> his item to suddenly experience a timeout. The time-based queue however
> grants you a control over the service degradation you're accepting to
> inflict to your users.
There's a key difference in our designs. You're trying to classify
traffic based on destination. I'm trying to classify traffic based on
source.
In my design, the maxconn is set to a level such that the server is
healthy and every request can be processed fast. With this, and a score
based queue, and under attack, the good requests will be drained fast,
leaving only the bad requests. Thus no normal user should be getting a
timeout, no matter what resource they're going to.
Your design works when the backend system can't keep up with good
legitimate traffic, and you need to prioritize one good request over
another good request. My design is when the backend system can't keep up
with bad traffic, and so we want to process the good traffic first.

>
> Another interesting example is when you want ot provide strong service
> guarantees to some top-level visitors while running under intense load.
> By using only a position, it's easy to say "I want the Gold class to
> advance by 1000 positions". But if the site is highly loaded and you
> have 10k pending requests, these 1000 positions remain irrelevant,
> because the requests sits there, waiting for 90% of the pollution to
> be flushed. 
I think you're misunderstanding my design, as scoring wouldn't work like
this at all. If you give the gold class a score of 1000 (where higher
number means higher priority), then the only thing that would get
processed before gold class would be another class that has a score >
1000. If nothing does, and a gold request comes in, it gets processed
first, no matter how big the queue is.
Think of scoring as instead having multiple queues. Each queue is only
processed if the queue before it is empty.
> If you say "I want these requests to advance by 10 seconds"
> then no matter how many other requests you have in the queue, it will
> really advance by 10 seconds and effectively shrink the response time
> by 10 seconds.
It may shrink the response time by 10 seconds, but if the normal
response time is 60 seconds, that's still 50 seconds of waiting. If your
users are only willing to put up with 30 seconds of wait, this means you
end up failing *everyone*.

>
> Overall for user experience it's important to think in time and not
> positions. The rationale behind this is simple : the user will never
> accept as a justification for varying quality of service the number of
> other visitors. And positions just depend on this, it's an average number
> of people you're allowing to pass over. If I'm granted a pass ticket
> allowing me to advance 10 places in the cinema queue and I find a
> 

Re: Priority based queuing

[Patrick Hemmer]

On 2018/5/2 11:04, Willy Tarreau wrote:
> On Tue, May 01, 2018 at 09:34:14PM -0400, Patrick Hemmer wrote:
>> Would it be possible to add priority based queuing to haproxy? By this I
>> mean that when a server/backend is full (maxconn), that incoming
>> requests would be added to the queue in a custom order. The idea here is
>> that when the system is under stress, to make sure the important
>> requests get handled first.
> Hehe that's fun that you mention this, as this has been postponed since
> around 1.2 or 1.3! By then we didn't have the equivalent of HTTP rules
> to add/subtract some priority. Now we have everything to do it, we "just"
> need to replace the lists with priority trees in the queues and that's
> all. It's not a big work if someone is interested in working on this.
>
>> In our exact use case, we're looking to use this to help mitigate DOS
>> attacks. The idea is that if a layer 7 attack is saturating the backend
>> servers, we can add logic to prioritize the requests. This logic might
>> be things like requests that have a valid application cookie go to the
>> front of the queue, or requests that come from a cloud provider (e.g.
>> EC2) go to the back of the queue.
> That's exactly why I wanted them to be manipulated vi http-request rules,
> so that everyone can construct his own conditions. Also I found that for
> most shopping sites, having time-based priority is more important than
> position-based : you often want this type of request to be processed 100ms
> faster than another type of request. With HTTP/2 it will be even more
> interesting because it will allow to send the important objects used for
> rendering before the other ones, which is very similar to the H2 priority
> but more fine-grained if you can adjust it on the fly.
>
>> DOS mitigation is hard because while you can write rules to identify
>> requests that are suspicious, you don't want to block them outright as
>> it is possible they might be legitimate. With prioritization, the
>> requests still get through, and are only affected when the backend is
>> saturated. If maxconn is not reached, the prioritization has no effect
>> at all (since queue is empty).
> I wholeheartly agree with you :-)
>
>> I made the change to haproxy and simulated the conditions in a lab, and
>> the strategy appears to work.
>> The change to haproxy was very minor, ~10 lines in queue.c, using
>> `task->nice` as the prioritization key. However my change is a very
>> rough PoC, and not worthy of submission.
> For a rough PoC it's indeed perfectly fine. But for a final design we
> really need a separate offset. I've really been convinced in field about
> using time rather than position, if you want to experiment with this I
> can give you some insights, it's the same in fact.
Can you elaborate on what you're thinking of for a time-based queue?

What I'm imagining you mean is that you would write a rule to set the
max queue time, and haproxy would insert it into the queue sorting on
TIME_NOW() + MAX_QUEUE_TIME. The main difference I see to this approach
vs scoring, is that you ensure that an item doesn't sit on the queue
forever (or whatever `timeout queue` is set to) if higher priority stuff
keeps getting inserted before it.
I don't think this is necessarily a good thing. If you're under a DOS
attack, the goal is to get the good requests processed before any
possible malicious requests. With a time based queue, those malicious
requests will still get processed and starve out the good requests. For
example lets say you're under attack, a bad request comes in with
max_queue_time=1000ms, and then after 999ms elapse, a good request comes
in with max_queue_time=10ms. You have a good request, and a bad request
on the queue, but HAProxy is going to process the bad request first
because its timer is expiring first. Essentially if haproxy is receiving
X good requests per second, and Y bad requests per second, it's still
going to forward X good per second, and Y bad per second, to the backend
server. The only difference is that they're time shifted.

The other thing I could think you mean by time-based is to insert into
the queue sorting on MAX_QUEUE_TIME, just like a score-based queue, but
you would still record TIME_NOW() + MAX_QUEUE_TIME, and would reject
requests that don't get processed by their deadline. Essentially a
per-request version of the `timeout queue` setting. But I don't see any
real value in this.

Or do you mean something else?

>
>> So before continuing any further down this route, I wanted to see if
>> this is something that could make it into HAProxy, and what any thoughts
>> on it might be.
> Absolutely! I've dreamed of it for over a decade, so I'm glad someone
> is willing to take care of it! Just

Priority based queuing

[Patrick Hemmer]

Would it be possible to add priority based queuing to haproxy? By this I
mean that when a server/backend is full (maxconn), that incoming
requests would be added to the queue in a custom order. The idea here is
that when the system is under stress, to make sure the important
requests get handled first.

In our exact use case, we're looking to use this to help mitigate DOS
attacks. The idea is that if a layer 7 attack is saturating the backend
servers, we can add logic to prioritize the requests. This logic might
be things like requests that have a valid application cookie go to the
front of the queue, or requests that come from a cloud provider (e.g.
EC2) go to the back of the queue.
DOS mitigation is hard because while you can write rules to identify
requests that are suspicious, you don't want to block them outright as
it is possible they might be legitimate. With prioritization, the
requests still get through, and are only affected when the backend is
saturated. If maxconn is not reached, the prioritization has no effect
at all (since queue is empty).

I made the change to haproxy and simulated the conditions in a lab, and
the strategy appears to work.
The change to haproxy was very minor, ~10 lines in queue.c, using
`task->nice` as the prioritization key. However my change is a very
rough PoC, and not worthy of submission.
So before continuing any further down this route, I wanted to see if
this is something that could make it into HAProxy, and what any thoughts
on it might be.

-Patrick

[PATCH 1/1] DOC/MINOR: clean up LUA documentation re: servers & array/table.

[Patrick Hemmer]

* A few typos
* Fix definitions of values which are tables, not arrays.
* Consistent US English naming for "server" instead of "serveur".
---
 doc/lua-api/index.rst | 88
++-
 1 file changed, 45 insertions(+), 43 deletions(-)


diff --git a/doc/lua-api/index.rst b/doc/lua-api/index.rst
index a83bbde01..9b6d18f46 100644
--- a/doc/lua-api/index.rst
+++ b/doc/lua-api/index.rst
@@ -169,9 +169,9 @@ Core class
 
   **context**: task, action, sample-fetch, converter
 
-  This attribute is an array of declared proxies (frontend and backends). Each
-  proxy give an access to his list of listeners and servers. Each entry is of
-  type :ref:`proxy_class`
+  This attribute is a table of declared proxies (frontend and backends). Each
+  proxy give an access to his list of listeners and servers. The table is
+  indexed by proxy name, and each entry is of type :ref:`proxy_class`.
 
   Warning, if you are declared frontend and backend with the same name, only one
   of these are listed.
@@ -183,12 +183,9 @@ Core class
 
   **context**: task, action, sample-fetch, converter
 
-  This attribute is an array of declared proxies with backend capability. Each
-  proxy give an access to his list of listeners and servers. Each entry is of
-  type :ref:`proxy_class`
-
-  Warning, if you are declared frontend and backend with the same name, only one
-  of these are listed.
+  This attribute is a table of declared proxies with backend capability. Each
+  proxy give an access to his list of listeners and servers. The table is
+  indexed by the backend name, and each entry is of type :ref:`proxy_class`.
 
   :see: :js:attr:`core.proxies`
   :see: :js:attr:`core.frontends`
@@ -197,12 +194,9 @@ Core class
 
   **context**: task, action, sample-fetch, converter
 
-  This attribute is an array of declared proxies with frontend capability. Each
-  proxy give an access to his list of listeners and servers. Each entry is of
-  type :ref:`proxy_class`
-
-  Warning, if you are declared frontend and backend with the same name, only one
-  of these are listed.
+  This attribute is a table of declared proxies with frontend capability. Each
+  proxy give an access to his list of listeners and servers. The table is
+  indexed by the frontend name, and each entry is of type :ref:`proxy_class`.
 
   :see: :js:attr:`core.proxies`
   :see: :js:attr:`core.backends`
@@ -336,7 +330,7 @@ Core class
   Lua execution or resume, so two consecutive call to the function "now" will
   probably returns the same result.
 
-  :returns: an array which contains two entries "sec" and "usec". "sec"
+  :returns: a table which contains two entries "sec" and "usec". "sec"
 contains the current at the epoch format, and "usec" contains the
 current microseconds.
 
@@ -439,9 +433,12 @@ Core class
 
   **context**: body, init, task, action, sample-fetch, converter
 
-  proxies is an array containing the list of all proxies declared in the
-  configuration file. Each entry of the proxies array is an object of type
-  :ref:`proxy_class`
+  proxies is a table containing the list of all proxies declared in the
+  configuration file. The table is indexed by the proxy name, and each entry
+  of the proxies table is an object of type :ref:`proxy_class`.
+
+  Warning, if you have declared a frontend and backend with the same name, only
+  one of these are listed.
 
 .. js:function:: core.register_action(name, actions, func [, nb_args])
 
@@ -852,13 +849,14 @@ Proxy class
 
 .. js:attribute:: Proxy.servers
 
-  Contain an array with the attached servers. Each server entry is an object of
-  type :ref:`server_class`.
+  Contain a table with the attached servers. The table is indexed by server
+  name, and each server entry is an object of type :ref:`server_class`.
 
 .. js:attribute:: Proxy.listeners
 
-  Contain an array with the attached listeners. Each listeners entry is an
-  object of type :ref:`listener_class`.
+  Contain a table with the attached listeners. The table is indexed by listener
+  name, and each each listeners entry is an object of type
+  :ref:`listener_class`.
 
 .. js:function:: Proxy.pause(px)
 
@@ -908,21 +906,25 @@ Proxy class
 
 .. js:function:: Proxy.get_stats(px)
 
-  Returns an array containg the proxy statistics. The statistics returned are
+  Returns a table containg the proxy statistics. The statistics returned are
   not the same if the proxy is frontend or a backend.
 
   :param class_proxy px: A :ref:`proxy_class` which indicates the manipulated
 proxy.
-  :returns: a key/value array containing stats
+  :returns: a key/value table containing stats
 
 .. _server_class:
 
 Server class
 
 
+.. js:class:: Server
+
+  This class provides a way for manipulating servers and retrieving information.
+
 .. js:function:: Server.is_draining(sv)
 
-  Return true if the server is currently draining stiky connections.
+  Return true if the server is currently draining sticky connections.
 
   

[PATCH 0/1] Re: DOC/MINOR: clean up LUA documentation re: servers & array/table.

[Patrick Hemmer]

Another rebase on this as I found more table/array issues in the
documentation. I went through the whole doc this time, so this should be
all of it.

Patrick Hemmer (1):
  DOC/MINOR: clean up LUA documentation re: servers & array/table.

 doc/lua-api/index.rst | 88
++-
 1 file changed, 45 insertions(+), 43 deletions(-)

-- 
2.16.3

LUA Converters should be global

[Patrick Hemmer]

Right now in LUA, all the HAProxy converters are accessible through the
`Converters` class. However this class is only accessible through the
TXN class. Can we get this changed so that the Converters class is a global?

My intent is to be able to call a builtin HAProxy converter from within
a custom LUA converter. However the LUA converter prototype does not
receive a TXN, so I have no access to the the Converters class.
Converters are supposed to be stateless, so I see no reason why they
should be accessible only from within the TXN class.

-Patrick

Re: [PATCH 1/2] MINOR: ssl: disable SSL sample fetches when unsupported

[Patrick Hemmer]

On 2018/4/30 04:58, Emeric Brun wrote:
> Hi Patrick,
>
> On 04/29/2018 01:15 AM, Patrick Hemmer wrote:
>> Previously these fetches would return empty results when HAProxy was
>> compiled
>> without the requisite SSL support. This results in confusion and problem
>> reports from people who unexpectedly encounter the behavior.
>> ---
>>  src/ssl_sock.c | 27 +++
>>  1 file changed, 15 insertions(+), 12 deletions(-)
>>
>>
> Are you sure this path will not cause regression using boringssl or libressl 
> because i see some ifdef just based on openssl version.
>
>
> R,
> Emeric
Yes. All this change does is copy/move the ifdefs around. Previously the
ifdefs were inside the functions, so that if the functions were called,
they'd return empty. The change moves the ifdefs to remove the function
from the compiled code entirely. Yes it is possible that people who have
been calling these functions when they don't have support for them will
now get errors. But this is what was requested to happen by Willy.

-Patrick

[PATCH 0/1] *** SUBJECT HERE ***

[Patrick Hemmer]

This is an update of the previous patch. Noticed another error in the
documentation. The following statement was present on `core.frontends`
and `core.backends`:

  Warning, if you are declared frontend and backend with the same name,
  only one of these are listed.

This statement is false. I'm guessing this was meant to be placed on
`core.proxies`, as there the statement is true.

-Patrick


Patrick Hemmer (1):
  DOC/MINOR: clean up LUA documentation re: servers & array/table.

 doc/lua-api/index.rst | 60
++-
 1 file changed, 31 insertions(+), 29 deletions(-)

-- 
2.16.3

[PATCH 1/1] DOC/MINOR: clean up LUA documentation re: servers & array/table.

[Patrick Hemmer]

* A few typos
* Move note about duplicate frontend/backend duplicate naming.
* Consistent US English naming for "server" instead of "serveur".
* Properly identify tables as tables instead of arrays, and provide
information on index. -- LUA defines an array as a table indexed with
integers. By that definition many of the things being called arrays
weren't arrays.

---
 doc/lua-api/index.rst | 60
++-
 1 file changed, 31 insertions(+), 29 deletions(-)


diff --git a/doc/lua-api/index.rst b/doc/lua-api/index.rst
index a83bbde01..8be37e830 100644
--- a/doc/lua-api/index.rst
+++ b/doc/lua-api/index.rst
@@ -169,9 +169,9 @@ Core class
 
   **context**: task, action, sample-fetch, converter
 
-  This attribute is an array of declared proxies (frontend and backends). Each
-  proxy give an access to his list of listeners and servers. Each entry is of
-  type :ref:`proxy_class`
+  This attribute is a table of declared proxies (frontend and backends). Each
+  proxy give an access to his list of listeners and servers. The table is
+  indexed by proxy name, and each entry is of type :ref:`proxy_class`.
 
   Warning, if you are declared frontend and backend with the same name, only one
   of these are listed.
@@ -183,12 +183,9 @@ Core class
 
   **context**: task, action, sample-fetch, converter
 
-  This attribute is an array of declared proxies with backend capability. Each
-  proxy give an access to his list of listeners and servers. Each entry is of
-  type :ref:`proxy_class`
-
-  Warning, if you are declared frontend and backend with the same name, only one
-  of these are listed.
+  This attribute is a table of declared proxies with backend capability. Each
+  proxy give an access to his list of listeners and servers. The table is
+  indexed by the backend name, and each entry is of type :ref:`proxy_class`.
 
   :see: :js:attr:`core.proxies`
   :see: :js:attr:`core.frontends`
@@ -197,12 +194,9 @@ Core class
 
   **context**: task, action, sample-fetch, converter
 
-  This attribute is an array of declared proxies with frontend capability. Each
-  proxy give an access to his list of listeners and servers. Each entry is of
-  type :ref:`proxy_class`
-
-  Warning, if you are declared frontend and backend with the same name, only one
-  of these are listed.
+  This attribute is a table of declared proxies with frontend capability. Each
+  proxy give an access to his list of listeners and servers. The table is
+  indexed by the frontend name, and each entry is of type :ref:`proxy_class`.
 
   :see: :js:attr:`core.proxies`
   :see: :js:attr:`core.backends`
@@ -439,9 +433,12 @@ Core class
 
   **context**: body, init, task, action, sample-fetch, converter
 
-  proxies is an array containing the list of all proxies declared in the
-  configuration file. Each entry of the proxies array is an object of type
-  :ref:`proxy_class`
+  proxies is a table containing the list of all proxies declared in the
+  configuration file. The table is indexed by the proxy name, and each entry
+  of the proxies table is an object of type :ref:`proxy_class`.
+
+  Warning, if you have declared a frontend and backend with the same name, only
+  one of these are listed.
 
 .. js:function:: core.register_action(name, actions, func [, nb_args])
 
@@ -852,13 +849,14 @@ Proxy class
 
 .. js:attribute:: Proxy.servers
 
-  Contain an array with the attached servers. Each server entry is an object of
-  type :ref:`server_class`.
+  Contain a table with the attached servers. The table is indexed by server
+  name, and each server entry is an object of type :ref:`server_class`.
 
 .. js:attribute:: Proxy.listeners
 
-  Contain an array with the attached listeners. Each listeners entry is an
-  object of type :ref:`listener_class`.
+  Contain a table with the attached listeners. The table is indexed by listener
+  name, and each each listeners entry is an object of type
+  :ref:`listener_class`.
 
 .. js:function:: Proxy.pause(px)
 
@@ -920,9 +918,13 @@ Proxy class
 Server class
 
 
+.. js:class:: Server
+
+  This class provides a way for manipulating servers and retrieving information.
+
 .. js:function:: Server.is_draining(sv)
 
-  Return true if the server is currently draining stiky connections.
+  Return true if the server is currently draining sticky connections.
 
   :param class_server sv: A :ref:`server_class` which indicates the manipulated
 server.
@@ -930,7 +932,7 @@ Server class
 
 .. js:function:: Server.set_weight(sv, weight)
 
-  Dynamically change the weight of the serveur. See the management socket
+  Dynamically change the weight of the server. See the management socket
   documentation for more information about the format of the string.
 
   :param class_server sv: A :ref:`server_class` which indicates the manipulated
@@ -939,7 +941,7 @@ Server class
 
 .. js:function:: Server.get_weight(sv)
 
-  This function returns an integer representing the serveur weight.
+  This 

[PATCH] MINOR: add get_maxconn and set_maxconn to LUA Server class.

[Patrick Hemmer]

---
 doc/lua-api/index.rst | 17 +
 src/hlua_fcn.c| 30 ++
 2 files changed, 47 insertions(+)


diff --git a/doc/lua-api/index.rst b/doc/lua-api/index.rst
index cdb285957..b61bf6078 100644
--- a/doc/lua-api/index.rst
+++ b/doc/lua-api/index.rst
@@ -941,6 +941,23 @@ Server class
 server.
   :returns: a boolean
 
+.. js:function:: Server.set_maxconn(sv, weight)
+
+  Dynamically change the maximum connections of the server. See the management
+  socket documentation for more information about the format of the string.
+
+  :param class_server sv: A :ref:`server_class` which indicates the manipulated
+server.
+  :param string maxconn: A string describing the server maximum connections.
+
+.. js:function:: Server.get_maxconn(sv, weight)
+
+  This function returns an integer representing the server maximum connections.
+
+  :param class_server sv: A :ref:`server_class` which indicates the manipulated
+server.
+  :returns: an integer.
+
 .. js:function:: Server.set_weight(sv, weight)
 
   Dynamically change the weight of the server. See the management socket
diff --git a/src/hlua_fcn.c b/src/hlua_fcn.c
index 280d8e5af..f3b50f5d1 100644
--- a/src/hlua_fcn.c
+++ b/src/hlua_fcn.c
@@ -592,6 +592,34 @@ int hlua_server_is_draining(lua_State *L)
 	return 1;
 }
 
+int hlua_server_set_maxconn(lua_State *L)
+{
+	struct server *srv;
+	const char *maxconn;
+	const char *err;
+
+	srv = hlua_check_server(L, 1);
+	maxconn = luaL_checkstring(L, 2);
+
+	HA_SPIN_LOCK(SERVER_LOCK, >lock);
+	err = server_parse_maxconn_change_request(srv, maxconn);
+	HA_SPIN_UNLOCK(SERVER_LOCK, >lock);
+	if (!err)
+		lua_pushnil(L);
+	else
+		hlua_pushstrippedstring(L, err);
+	return 1;
+}
+
+int hlua_server_get_maxconn(lua_State *L)
+{
+	struct server *srv;
+
+	srv = hlua_check_server(L, 1);
+	lua_pushinteger(L, srv->maxconn);
+	return 1;
+}
+
 int hlua_server_set_weight(lua_State *L)
 {
 	struct server *srv;
@@ -1274,6 +1302,8 @@ int hlua_fcn_reg_core_fcn(lua_State *L)
 	lua_pushstring(L, "__index");
 	lua_newtable(L);
 	hlua_class_function(L, "is_draining", hlua_server_is_draining);
+	hlua_class_function(L, "set_maxconn", hlua_server_set_maxconn);
+	hlua_class_function(L, "get_maxconn", hlua_server_get_maxconn);
 	hlua_class_function(L, "set_weight", hlua_server_set_weight);
 	hlua_class_function(L, "get_weight", hlua_server_get_weight);
 	hlua_class_function(L, "set_addr", hlua_server_set_addr);