On 17 December 2013 00:32, Chris Burroughs <[email protected]>wrote:
> On 12/04/2013 11:24 AM, Chris Burroughs wrote:
>
>> On 12/03/2013 04:07 PM, Chris Burroughs wrote:
>>
>>> This could just be me not being adept at email patches. Sorry if this
>>> is obvious but is this supposed to apply against 1.4 or 1.5?
>>>
>>
>> To answer my own question this applies against 1.5. I'm not sure of the
>> feasibility or desirability of backporting to 1.4.
>>
>> I ran with this in a production load test and as far I can tell it
>> worked as advertised. We were able to run with nbproc and still have
>> useful looking stats sockets for haptop, ganglia etc. At least in our
>> use case, stats sockets with this patch solve the primary objection to
>> running with nbproc.
>>
>
>
> Happpy to test additional patches or variants if further review is needed.
> Is there anything blockng this getting accepted into 1.5?
>
Hi guys,
Following use of this patch in our environments we noted some odd
behaviours...
The steps to reproduce these was to use the multiple sockets patch and then
try to disable and enable a front end.
Here's an excerpt from our internal discussions:
=== START ===
SYNOPSIS:
The desire is to increase nbproc to greater than 1 in order to use more
CPUs to process more SSL data.
SYMPTOMS:
During splash the n-1 of the haproxy processes appear to 'busy spin' and
after splash data connectivity through the proxy is intermittent (1 in 3
give or take). In addition web stats did not appear to work entirely as
expected.
ANALYSIS:
During start of haproxy multiple web-stats front ends get created - it gets
very upset with this and a change of template has already been committed to
provide uniquely named front ends.
Strace on the 'busy spin' showed a EINVALID error after the epoll on the
socket.
ROOT CAUSE:
This gets pretty interesting and is the result of nbproc > 1 as a bit of an
edge case in the haproxy world right now.
When disable frontend is called on a unix socket the following code path is
taken:
1) Call shutdown(fd, SHUT_RD) on any listeners for that frontend to
indicate to the kernel that the fd should not pass data back at the present
time.
2) If 1 is successful then mark the state of the internal data structure
for the frontend as PR_STPAUSED (proxy state paused).
This works fine for the first process but since the process forks and the
fd is shared between PIDs we get some odd behaviour observed.
The EINVALID is due to the other process still wanting to accept() on the
fd but the kernel no longer having it marked for read.
If a disable frontend is then sent to another process 1 above fails (as
there is no RD currently in play so far as the kernel is concerned) but
then haproxy marks the data structure for the proxy as PR_STERROR rather
than PR_STPAUSED due to this.
This stops the busy spin but at an application level haproxy no longer
considers this a working frontend.
When enable frontend is sent to the first control socket it carries out the
following sequence:
1) Checks the state is PR_STPAUSED.
2) Calls listen(fd) to get the kernel to send data again.
3) Marks the internal data structure as state PR_STREADY.
4) Initiates the epoll sequence to start accepting data again.
When the next control socket has an enable frontend sent to it the
following sequence happens:
1) Checks the state and sees it is not PR_STPAUSED (it is PR_STERROR but
the logic is to check for PR_STPAUSED)
2) Since is is not paused it writes to stdout "Frontend is already enabled"
and exits with no other actions.
3) If the kernel tries to send data to this as the fd is shared between
processes and is thus 'open' haproxy at an application level does not have
the proxy in a state of PR_STREADY for this PID and rejects/closes this
attempt.
Iff the 'working PID' is able to get the data then the connection goes
through ... if any of the others PIDs get it then the connection is
rejected.
This problem is effectively hidden since PR_STERROR is not visualised
anywhere. In the stats pages the logic to show the state is as follows:
1) If the state is PR_STREADY then show OPEN
2) If the state is PR_STFULL (ie at connection max) then show FULL
3) If any other state show STOP (ie both PR_STPAUSED and PR_STERROR show
STOP with no indication of the actual state).
=== END ===
Attached is a revised patch that applies to the dev21 release which
resolves this state... Chris you might want to apply this if you are using
the unix sockets to change disable/enable frontends and not just stats
collection.
Specifically only the first fork (releative_pid == 1) manipulates the file
descriptor and the others just manipulate their data structure for the
proxy state. The patch also adds the PR_STERROR state to the stats screens
so that this is surfaced and can be distinguished from PR_STPAUSED ....
The latter might be worth doing anyway given PR_STERROR cannot be recovered
from without restarting haproxy entirely ...
Cheers,
James
diff -ruN haproxy-1.5-dev21/doc/configuration.txt haproxy-1.5-dev21.lmax//doc/configuration.txt
--- haproxy-1.5-dev21/doc/configuration.txt 2013-12-16 23:45:49.000000000 +0000
+++ haproxy-1.5-dev21.lmax//doc/configuration.txt 2014-03-06 16:02:33.397212427 +0000
@@ -632,6 +632,8 @@
All parameters supported by "bind" lines are supported, for instance to
restrict access to some users or their access rights. Please consult
section 5.1 for more information.
+
+ Multiple stats socket lines can be in global if nbproc is greater than 1 controlling each process.
stats timeout <timeout, in milliseconds>
The default timeout on the stats socket is set to 10 seconds. It is possible
diff -ruN haproxy-1.5-dev21/src/cfgparse.c haproxy-1.5-dev21.lmax//src/cfgparse.c
--- haproxy-1.5-dev21/src/cfgparse.c 2013-12-16 23:45:49.000000000 +0000
+++ haproxy-1.5-dev21.lmax//src/cfgparse.c 2014-03-06 16:02:33.405212324 +0000
@@ -7392,7 +7392,7 @@
/* Check multi-process mode compatibility */
if (global.nbproc > 1) {
if (global.stats_fe && !global.stats_fe->bind_proc) {
- Warning("stats socket will not work as expected in multi-process mode (nbproc > 1), you should force process binding using 'stats bind-process'.\n");
+ Warning("stats socket will not work as expected in multi-process mode (nbproc > 1), you should force process binding using 'stats bind-process' or provide multiple stats sockets lines - one for each process.\n");
}
}
diff -ruN haproxy-1.5-dev21/src/dumpstats.c haproxy-1.5-dev21.lmax//src/dumpstats.c
--- haproxy-1.5-dev21/src/dumpstats.c 2013-12-16 23:45:49.000000000 +0000
+++ haproxy-1.5-dev21.lmax//src/dumpstats.c 2014-03-06 16:02:33.409212272 +0000
@@ -2353,6 +2353,7 @@
U2H(px->fe_counters.denied_req), U2H(px->fe_counters.denied_resp),
U2H(px->fe_counters.failed_req),
px->state == PR_STREADY ? "OPEN" :
+ px->state == PR_STERROR ? "ERROR" :
px->state == PR_STFULL ? "FULL" : "STOP");
}
else { /* CSV mode */
@@ -2385,6 +2386,7 @@
px->fe_counters.denied_req, px->fe_counters.denied_resp,
px->fe_counters.failed_req,
px->state == PR_STREADY ? "OPEN" :
+ px->state == PR_STERROR ? "ERROR" :
px->state == PR_STFULL ? "FULL" : "STOP",
relative_pid, px->uuid, STATS_TYPE_FE,
read_freq_ctr(&px->fe_sess_per_sec),
diff -ruN haproxy-1.5-dev21/src/haproxy.c haproxy-1.5-dev21.lmax//src/haproxy.c
--- haproxy-1.5-dev21/src/haproxy.c 2013-12-16 23:45:49.000000000 +0000
+++ haproxy-1.5-dev21.lmax//src/haproxy.c 2014-03-06 16:02:33.409212272 +0000
@@ -1578,6 +1578,25 @@
px = px->next;
}
+ /* unbind unix control sockets leaving one on each process for deterministic control */
+ {
+ int ListenCount = 1;
+ struct proxy * px = proxy;
+
+ while (px != NULL) {
+ if (strcmp(px->id , "GLOBAL") == 0) {
+ struct listener * l;
+ list_for_each_entry(l, &px->conf.listeners, by_fe) {
+ if(ListenCount != relative_pid && !(px->bind_proc)){
+ unbind_listener(l);
+ }
+ ListenCount++;
+ }
+ }
+ px = px->next;
+ }
+ }
+
if (proc == global.nbproc) {
if (global.mode & MODE_SYSTEMD) {
for (proc = 0; proc < global.nbproc; proc++)
diff -ruN haproxy-1.5-dev21/src/listener.c haproxy-1.5-dev21.lmax//src/listener.c
--- haproxy-1.5-dev21/src/listener.c 2013-12-16 23:45:49.000000000 +0000
+++ haproxy-1.5-dev21.lmax//src/listener.c 2014-03-06 17:20:33.767353654 +0000
@@ -76,26 +76,31 @@
*/
int pause_listener(struct listener *l)
{
- if (l->state <= LI_PAUSED)
- return 1;
+ if (l->state <= LI_PAUSED)
+ return 1;
- if (l->proto->sock_prot == IPPROTO_TCP) {
- if (shutdown(l->fd, SHUT_WR) != 0)
- return 0; /* Solaris dies here */
-
- if (listen(l->fd, l->backlog ? l->backlog : l->maxconn) != 0)
- return 0; /* OpenBSD dies here */
-
- if (shutdown(l->fd, SHUT_RD) != 0)
- return 0; /* should always be OK */
- }
-
- if (l->state == LI_LIMITED)
- LIST_DEL(&l->wait_queue);
-
- fd_stop_recv(l->fd);
- l->state = LI_PAUSED;
- return 1;
+ /* If we have more than one process (nbproc>1) then only get one process to
+ * tell the operating system to pause the socket */
+ if (relative_pid == 1) {
+ if (l->proto->sock_prot == IPPROTO_TCP) {
+ if (shutdown(l->fd, SHUT_WR) != 0)
+ return 0; /* Solaris dies here */
+
+ if (listen(l->fd, l->backlog ? l->backlog : l->maxconn) != 0)
+ return 0; /* OpenBSD dies here */
+
+ if (shutdown(l->fd, SHUT_RD) != 0)
+ return 0; /* should always be OK */
+ }
+
+ }
+
+ if (l->state == LI_LIMITED)
+ LIST_DEL(&l->wait_queue);
+
+ fd_stop_recv(l->fd);
+ l->state = LI_PAUSED;
+ return 1;
}
/* This function tries to resume a temporarily disabled listener. Paused, full,
@@ -105,28 +110,31 @@
*/
int resume_listener(struct listener *l)
{
- if (l->state < LI_PAUSED)
- return 0;
-
- if (l->proto->sock_prot == IPPROTO_TCP &&
- l->state == LI_PAUSED &&
- listen(l->fd, l->backlog ? l->backlog : l->maxconn) != 0)
- return 0;
-
- if (l->state == LI_READY)
- return 1;
-
- if (l->state == LI_LIMITED)
- LIST_DEL(&l->wait_queue);
-
- if (l->nbconn >= l->maxconn) {
- l->state = LI_FULL;
- return 1;
- }
-
- fd_want_recv(l->fd);
- l->state = LI_READY;
- return 1;
+ if (l->state < LI_PAUSED)
+ return 0;
+
+ /* Only the first process should tell the OS to reenable the socket */
+ if (relative_pid == 1) {
+ if (l->proto->sock_prot == IPPROTO_TCP &&
+ l->state == LI_PAUSED &&
+ listen(l->fd, l->backlog ? l->backlog : l->maxconn) != 0)
+ return 0;
+ }
+
+ if (l->state == LI_READY)
+ return 1;
+
+ if (l->state == LI_LIMITED)
+ LIST_DEL(&l->wait_queue);
+
+ if (l->nbconn >= l->maxconn) {
+ l->state = LI_FULL;
+ return 1;
+ }
+
+ fd_want_recv(l->fd);
+ l->state = LI_READY;
+ return 1;
}
/* Marks a ready listener as full so that the session code tries to re-enable
