Willy Tarreau [Thu, 4 Dec 2008 08:33:58 +0000 (09:33 +0100)]
[BUG] do not dequeue the backend's pending connections on a dead server
Kai Krueger found that previous patch was incomplete, because there is
an unconditionnal call to process_srv_queue() in session_free() which
still causes a dead server to consume pending connections from the
backend.
This call was made unconditionnal so that we don't leave unserved
connections in the server queue, for instance connections coming
in with "option persist" which can bypass the server status check.
However, the server must not touch the backend's queue if it is down.
Another fear was that some connections might remain unserved when
the server is using a dynamic maxconn if the number of connections
to the backend is too low. Right now, srv_dynamic_maxconn() ensures
this cannot happen, so the call can remain conditionnal.
The fix consists in allowing a server to process it own queue whatever
its state, but not to touch the backend's queue if it is down. Its
queue should normally be empty when the server is down because it is
redistributed when the server goes down. The only remaining cases are
precisely the persistent connections with "option persist" set, coming
in after the queue has been redispatched. Those ones must still be
processed when a connection terminates.
Willy Tarreau [Sun, 30 Nov 2008 20:51:58 +0000 (21:51 +0100)]
[BUG] do not dequeue requests on a dead server
Kai Krueger reported a problem when a server goes down with active
connections. A lot of connections were drained by that server. Kai
did an amazing job at tracking this bug down to the dequeuing
mechanism which forgets to check the server state before allowing
a request to be sent to a server.
The problem occurs more often with long requests, which have a chance
to complete after the server is completely marked down, and to find
requests in the global queue which have not yet been fetched by other
servers.
The fix consists in ensuring that a server is up before sending it
any new request from the queue.
Willy Tarreau [Sun, 16 Nov 2008 06:40:34 +0000 (07:40 +0100)]
[BUG] critical errors should be reported even in daemon mode
Josh Goebel reported that haproxy silently dies when it fails to
chroot. In fact, it does so when in daemon mode, because daemon
mode has been disabling output for ages.
Since the code has been reworked, this could have been changed
because there is no reason for this anymore, hence this patch.
Willy Tarreau [Tue, 4 Nov 2008 09:54:14 +0000 (10:54 +0100)]
[RELEASE] Released version 1.3.15.6
Released version 1.3.15.6 with the following main changes :
- [MINOR] cfgparse: fix off-by 2 in error message size
- [BUG] cookie capture is declared in the frontend but checked on the backend
Willy Tarreau [Fri, 17 Oct 2008 10:01:58 +0000 (12:01 +0200)]
[BUG] cookie capture is declared in the frontend but checked on the backend
Cookie capture would only work by pure luck on the request but did
never work on responses since only the backend was checked. The fix
consists in always checking frontend for cookie captures.
(cherry picked from commit a83c5ba9315a7c47cda2698280b7e49a9d3eb374)
Willy Tarreau [Sun, 12 Oct 2008 20:44:43 +0000 (22:44 +0200)]
[RELEASE] Released version 1.3.15.5
Released version 1.3.15.5 with the following main changes :
- [BUG] do not try to pause backends during reload
- [BUG] ensure that listeners from disabled proxies are correctly unbound.
- [BUG] acl-related keywords are not allowed in defaults sections
Willy Tarreau [Sun, 12 Oct 2008 15:26:37 +0000 (17:26 +0200)]
[BUG] acl-related keywords are not allowed in defaults sections
Using an ACL-related keyword in the defaults section causes a
segfault during parsing because the list headers are not initialized.
We must initialize list headers for default instance and reject
keywords relying on ACLs.
Willy Tarreau [Sun, 12 Oct 2008 10:07:48 +0000 (12:07 +0200)]
[BUG] ensure that listeners from disabled proxies are correctly unbound.
There is a problem when an instance is marked "disabled". Its ports are
still bound but will not be unbound upon termination. This causes processes
to accumulate during soft restarts, and might even cause failures to restart
new ones due to the inability to bind to the same port.
The ideal solution would be to bind all ports at the end of the configuration
parsing. An acceptable workaround is to unbind all listeners of disabled
proxies. This is what the current patch does.
Willy Tarreau [Fri, 10 Oct 2008 15:51:34 +0000 (17:51 +0200)]
[BUG] do not try to pause backends during reload
During a configuration reload, haproxy tried to pause all proxies.
Unfortunately, it also tried to pause backends, which would fail
and cause trouble to the new process since the port was still bound.
Released version 1.3.15.4 with the following main changes :
- [BUG] do not release the connection slot during a retry
- [BUG] dynamic connection throttling could return a max of zero conns
[BUG] dynamic connection throttling could return a max of zero conns
srv_dynamic_maxconn() is clearly documented as returning at least 1
possible connection under throttling. But the computation was wrong,
the minimum 1 was divided and got lost in case of very low maxconns.
Apply the MAX(1, max) before returning the result in order to ensure
that a newly appeared server will get some traffic.
[BUG] do not release the connection slot during a retry
A bug was introduced during last queue management fix. If a server
connection fails, the allocated connection slot is released, but it
will be needed again after the turn-around. This also causes more
connections than expected to go to the server because it appears to
have less connections than real.
Many thanks to Rupert Fiasco, Mark Imbriaco, Cody Fauser, Brian
Gupta and Alexander Staubo for promptly providing configuration
and diagnosis elements to help reproduce this problem easily.
Released version 1.3.15.3 with the following main changes :
- [BUG] disable buffer read timeout when reading stats
- [BUILD] change declaration of base64tab to fix build with Intel C++
- [BUILD] silent a warning in unlikely() with gcc 4.x
- [BUG] use_backend would not correctly consider "unless"
- [CLEANUP] remove dependency on obsolete INTBITS macro
- [BUG] fix segfault with url_param + check_post
- [BUG] server timeout was not considered in some circumstances
- [BUG] ev_sepoll: closed file descriptors could persist in the spec list
- [BUG] maintain_proxies must not disable backends
- [BUG] regparm is broken on gcc < 3
- [OPTIM] force inlining of large functions with gcc >= 3
Willy Tarreau [Fri, 29 Aug 2008 13:48:49 +0000 (15:48 +0200)]
[OPTIM] force inlining of large functions with gcc >= 3
GCC 3 and above do not inline large functions, which is a problem
with ebtree where most core functions are inlined.
This simple patch has both reduced code size and increased speed.
It should be back-ported to ebtree.
(cherry picked from commit 707d3da01f8475d5c172d347a73bd9e947076df6)
Willy Tarreau [Sun, 17 Aug 2008 15:06:37 +0000 (17:06 +0200)]
[BUG] regparm is broken on gcc < 3
Gcc < 3 does not consider regparm declarations for function pointers.
This causes big trouble at least with pollers (and with any function
pointer after all). Disable CONFIG_HAP_USE_REGPARM for gcc < 3.
(cherry picked from commit 61eadc028fb8774ea05d893cd3eca6c671fb511e)
Willy Tarreau [Sat, 16 Aug 2008 16:41:13 +0000 (18:41 +0200)]
[BUG] maintain_proxies must not disable backends
maintain_proxies could disable backends (p->maxconn == 0) which is
wrong (but apparently harmless). Add a check for p->maxconn == 0.
(cherry picked from commit d5382b4aaa099ce5ce2af5828bd4d6dc38e9e8ea)
Willy Tarreau [Sat, 16 Aug 2008 14:06:02 +0000 (16:06 +0200)]
[BUG] ev_sepoll: closed file descriptors could persist in the spec list
If __fd_clo() was called on a file descriptor which was previously
disabled, it was not removed from the spec list. This apparently
could not happen on previous code because the TCP states prevented
this, but now it happens regularly. The effects are spec entries
stuck populated, leading to busy loops.
Willy Tarreau [Mon, 11 Aug 2008 08:35:07 +0000 (10:35 +0200)]
[BUG] server timeout was not considered in some circumstances
Due to a copy-paste typo, the client timeout was refreshed instead
of the server's when waiting for server response. This means that
the server's timeout remained eternity.
Willy Tarreau [Sun, 10 Aug 2008 22:21:56 +0000 (00:21 +0200)]
[BUG] fix segfault with url_param + check_post
If an HTTP/0.9-like POST request is sent to haproxy while
configured with url_param + check_post, it will crash. The
reason is that the total buffer length was computed based
on req->total (which equals the number of bytes read) and
not req->l (number of bytes in the buffer), thus leading
to wrong size calculations when calling memchr().
The affected code does not look like it could have been
exploited to run arbitrary code, only reads were performed
at wrong locations.
(cherry picked from commit fb0528bd56063e9800c7dd6fbd96b3c5c6a687f2)
[CLEANUP] remove dependency on obsolete INTBITS macro
The INTBITS macro was found to be already defined on some platforms,
and to equal 32 (while INTBITS was 5 here). Due to pure luck, there
was no declaration conflict, but it's nonetheless a problem to fix.
Looking at the code showed that this macro was only used for left
shifts and nothing else anymore. So the replacement is obvious. The
new macro, BITS_PER_INT is more obviously correct.
(cherry picked from commit 177e2b012723ef65c6c7f850df3e6e0cd2cca2b4)
[BUG] use_backend would not correctly consider "unless"
A copy-paste typo made use_backend not correctly consider the "unless"
case, depending on the previous "block" rule.
(cherry picked from commit a8cfa34a9c011cecfaedfaf7d91de3e5f7f004a0)
[BUILD] silent a warning in unlikely() with gcc 4.x
The unlikely() implementation for gcc 4.x spits out a warning
when a pointer is passed. Add a cast to unsigned long.
(cherry picked from commit 75875a7c8c7348e90e373c007bafdedcc6253ab4)
Willy Tarreau [Sun, 29 Jun 2008 15:17:38 +0000 (17:17 +0200)]
[BUILD] change declaration of base64tab to fix build with Intel C++
I got a report that Intel C++ complains about the size of the
base64tab in base64.c. Setting it to 65 chars to allow for the
trailing zero fixes the problem.
(cherry picked from commit 69e989ccbcc1d5cbb623493d6c9cca169fb36ff6)
Willy Tarreau [Sun, 29 Jun 2008 14:38:43 +0000 (16:38 +0200)]
[BUG] disable buffer read timeout when reading stats
The buffer read timeouts were not reset when stats were produced. This
caused unneeded wakeups.
(cherry picked from commit 284c7b319566a66d5b742c905072175aac6445e1)
Willy Tarreau [Sat, 21 Jun 2008 19:59:05 +0000 (21:59 +0200)]
[RELEASE] Released version 1.3.15.2
Released version 1.3.15.2 with the following main changes :
- [BUILD] make install should depend on haproxy not "all"
- [BUG] event pollers must not wait if a task exists in the run queue
- [BUG] queue management: wake oldest request in queues
- [BUG] log: reported queue position was offed-by-one
- [BUG] fix the dequeuing logic to ensure that all requests get served
- [DOC] documentation for the "retries" parameter was missing.
Willy Tarreau [Fri, 20 Jun 2008 13:04:11 +0000 (15:04 +0200)]
[BUG] fix the dequeuing logic to ensure that all requests get served
The dequeuing logic was completely wrong. First, a task was assigned
to all servers to process the queue, but this task was never scheduled
and was only woken up on session free. Second, there was no reservation
of server entries when a task was assigned a server. This means that
as long as the task was not connected to the server, its presence was
not accounted for. This was causing trouble when detecting whether or
not a server had reached maxconn. Third, during a redispatch, a session
could lose its place at the server's and get blocked because another
session at the same moment would have stolen the entry. Fourth, the
redispatch option did not work when maxqueue was reached for a server,
and it was not possible to do so without indefinitely hanging a session.
The root cause of all those problems was the lack of pre-reservation of
connections at the server's, and the lack of tracking of servers during
a redispatch. Everything relied on combinations of flags which could
appear similarly in quite distinct situations.
This patch is a major rework but there was no other solution, as the
internal logic was deeply flawed. The resulting code is cleaner, more
understandable, uses less magics and is overall more robust.
As an added bonus, "option redispatch" now works when maxqueue has
been reached on a server.
Willy Tarreau [Fri, 13 Jun 2008 19:48:18 +0000 (21:48 +0200)]
[BUG] log: reported queue position was offed-by-one
The reported queue position in the logs was 0 for the first pending request
in the queue, which is wrong because it means that one request will have to
be completed before the queued one may execute. It caused the undesired side
effect that 0/0 was reported when either 0 or 1 request was pending in the
queue. Thus, we have to increment the queue size before reporting the value.
Willy Tarreau [Fri, 13 Jun 2008 19:12:51 +0000 (21:12 +0200)]
[BUG] queue management: wake oldest request in queues
When a server terminates a connection, the next session in its
own queue was immediately processed. Because of this, if all
server queues are always filled, then no new anonymous request
will be processed. Consider oldest request between global and
server queues to choose from which to pick the request.
An improvement over this will consist in adding a configurable
offset when comparing expiration dates, so that cookie-less
requests can get either less or more priority.
Willy Tarreau [Fri, 13 Jun 2008 19:06:56 +0000 (21:06 +0200)]
[BUG] event pollers must not wait if a task exists in the run queue
Under some circumstances, a task may already lie in the run queue
(eg: inter-task wakeup). It is disastrous to wait for an event in
this case because some processing gets delayed.
Willy Tarreau [Wed, 11 Jun 2008 22:25:46 +0000 (00:25 +0200)]
[BUILD] make install should depend on haproxy not "all"
Reported by Cherife Li : just doing a "make install" fails because it
depends on "all" which is equivalent to "help" if no TARGET was specified.
Make it depend on "haproxy" instead.
Willy Tarreau [Sun, 25 May 2008 19:12:58 +0000 (21:12 +0200)]
[RELEASE] Released version 1.3.15.1
Released version 1.3.15.1 with the following main changes :
- [BUILD] fix build with gcc 4.3
- [TESTS] add a debug patch to help trigger the stats bug
- [BUG] Flush buffers also where there are exactly 0 bytes left
- [DOC] update the README file with new build options
- [MEDIUM] reduce risk of event starvation in ev_sepoll
Willy Tarreau [Sun, 25 May 2008 08:39:02 +0000 (10:39 +0200)]
[MEDIUM] reduce risk of event starvation in ev_sepoll
If too many events are set for spec I/O, those ones can starve the
polled events. Experiments show that when polled events starve, they
quickly turn into spec I/O, making the situation even worse. While
we can reduce the number of polled events processed at once, we
cannot do this on speculative events because most of them are new
ones (avg 2/3 new - 1/3 old from experiments).
The solution against this problem relies on those two factors :
1) one FD registered as a spec event cannot be polled at the same time
2) even during very high loads, we will almost never be interested in
simultaneous read and write streaming on the same FD.
The first point implies that during starvation, we will not have more than
half of our FDs in the poll list, otherwise it means there is less than that
in the spec list, implying there is no starvation.
The second point implies that we're statically only interested in half of
the maximum number of file descriptors at once, because we will unlikely
have simultaneous read and writes for a same buffer during long periods.
So, if we make it possible to drain maxsock/2/2 during peak loads, then we
can ensure that there will be no starvation effect. This means that we must
always allocate maxsock/4 events for the poller.
Last, sepoll uses an optimization consisting in reducing the number of calls
to epoll_wait() to once every too polls. However, when dealing with many
spec events, we can wait very long and skipping epoll_wait() every second
time increases latency. For this reason, we try to detect if we are beyond
a reasonable limit and stop doing so at this stage.
For Fedora 9 gcc 4.3 will be shipping as a feature, and right now haproxy does
not compile with gcc 4.3.
It appears that there is a reordering of headers or something along those lines,
This is the patch that gets haproxy to compile with gcc 4.3. I'm not sure if
this is the correct approach you would want to use, so please correct me.
If this works for you, I'll go ahead and put this patch in the src rpm until a
release of haproxy which compiles with gcc 4.3 is released.
[BUG] Flush buffers also where there are exactly 0 bytes left
I noticed it was possible to get truncated http/csv stats. Sometimes.
Usually the problem disappeared as fast as it appeared, but once it
happend that my http-stats page was truncated for about one hour.
It was quite weird as it happened independently for csv and http
output and it took me some time to track & fix this bug.
Both buffer_write & buffer_write_chunk used to return 0 in two
situations: is case of success or where there was exactly 0 bytes
left. The first one is intentional but I believe the second one
is not as it was not possible to distinguish between successful
write and unsuccessful one, which means that if the buffer was 100%
filled, it was never flushed and it was not possible to write
more data.
Released version 1.3.15 with the following main changes :
- [BUILD] Added support for 'make install'
- [BUILD] Added 'install-man' make target for installing the man page
- [BUILD] Added 'install-bin' make target
- [BUILD] Added 'install-doc' make target
- [BUILD] Removed "/" after '$(DESTDIR)' in install targets
- [BUILD] Changed 'install' target to install the binaries first
- [BUILD] Replace hardcoded 'LD = gcc' with 'LD = $(CC)'
- [MEDIUM]: Inversion for options
- [MEDIUM]: Count retries and redispatches also for servers, fix redistribute_pending, extend logs, %d->%u cleanup
- [BUG]: Restore clearing t->logs.bytes
- [MEDIUM]: rework checks handling
- [DOC] Update a "contrib" file with a hint about a scheme used for formathing subjects
- [MEDIUM] Implement "track [<backend>/]<server>"
- [MINOR] Implement persistent id for proxies and servers
- [BUG] Don't increment server connections too much + fix retries
- [MEDIUM]: Prevent redispatcher from selecting the same server, version #3
- [MAJOR] proto_uxst rework -> SNMP support
- [BUG] appsession lookup in URL does not work
- [BUG] transparent proxy address was ignored in backend
- [BUG] hot reconfiguration failed because of a wrong error check
- [DOC] big update to the configuration manual
- [DOC] large update to the configuration manual
- [DOC] document more options
- [BUILD] major rework of the GNU Makefile
- [STATS] add support for "show info" on the unix socket
- [DOC] document options forwardfor to logasap
- [MINOR] add support for the "backlog" parameter
- [OPTIM] introduce global parameter "tune.maxaccept"
- [MEDIUM] introduce "timeout http-request" in frontends
- [MINOR] tarpit timeout is also allowed in backends
- [BUG] increment server connections for each connect()
- [MEDIUM] add a turn-around state of one second after a connection failure
- [BUG] fix typo in redispatched connection
- [DOC] document options nolinger to ssl-hello-chk
- [DOC] added documentation for "option tcplog" to "use_backend"
- [BUG] connect_server: server might not exist when sending error report
- [MEDIUM] support fully transparent proxy on Linux (USE_LINUX_TPROXY)
- [MEDIUM] add non-local bind to connect() on Linux
- [MINOR] add transparent proxy support for balabit's Tproxy v4
- [BUG] use backend's source and not server's source with tproxy
- [BUG] fix overlapping server flags
- [MEDIUM] fix server health checks source address selection
- [BUG] build failed on CONFIG_HAP_LINUX_TPROXY without CONFIG_HAP_CTTPROXY
- [DOC] added "server", "source" and "stats" keywords
- [DOC] all server parameters have been documented
- [DOC] document all req* and rsp* keywords.
- [DOC] added documentation about HTTP header manipulations
- [BUG] log response byte count, not request
- [BUILD] code did not build in full debug mode
- [BUG] fix truncated responses with sepoll
- [MINOR] use s->frt_addr as the server's address in transparent proxy
- [MINOR] fix configuration hint about timeouts
- [DOC] minor cleanup of the doc and notice to contributors
- [MINOR] report correct section type for unknown keywords.
- [BUILD] update MacOS Makefile to build on newer versions
- [DOC] fix erroneous "useallbackups" option in the doc
- [DOC] applied small fixes from early readers
- [MINOR] add configuration support for "redir" server keyword
- [MEDIUM] completely implement the server redirection method
- [TESTS] add a test case for the server redirection mechanism
- [DOC] add a configuration entry for "server ... redir <prefix>"
- [BUILD] backend.c and checks.c did not build without tproxy !
- Revert "[BUILD] backend.c and checks.c did not build without tproxy !"
- [BUILD] backend.c and checks.c did not build without tproxy !
- [OPTIM] used unsigned ints for HTTP state and message offsets
- [OPTIM] GCC4's builtin_expect() is suboptimal
- [BUG] failed conns were sometimes incremented in the frontend!
- [BUG] timeout.check was not pre-set to eternity
- [TESTS] add test-pollers.cfg to easily report pollers in use
- [BUG] do not apply timeout.connect in checks if unset
- [BUILD] ensure that makefile understands USE_DLMALLOC=1
- [MINOR] silent gcc for a wrong warning
- [CLEANUP] update .gitignore to ignore more temporary files
- [CLEANUP] report dlmalloc's source path only if explictly specified
- [BUG] str2sun could leak a small buffer in case of error during parsing
- [BUG] option allbackups was not working anymore in roundrobin mode
- [MAJOR] implementation of the "leastconn" load balancing algorithm
- [BUILD] ensure that users don't build without setting the target anymore.
- [DOC] document the leastconn LB algo
- [MEDIUM] fix stats socket limitation to 16 kB
- [DOC] fix unescaped space in httpchk example.
- [BUG] fix double-decrement of server connections
- [TESTS] add a test case for port mapping
- [TESTS] add a benchmark for integer hashing
- [TESTS] add new methods in ip-hash test file
- [MAJOR] implement parameter hashing for POST requests
[MAJOR] implement parameter hashing for POST requests
This patch extends the "url_param" load balancing method by introducing
the "check_post" option. Using this option enables analysis of the beginning
of POST requests to search for the specified URL parameter.
The patch also fixes a few minor typos in comments that were discovered
during code review.
If we want to support netmasks for IP address hashing,
we will need something better than a pure modulus, otherwise
people with even numbers of servers will get surprizes.
Bob Jenkins is known for his works on hashing, and his site
has a lot of very interesting researches and algorithms for
integer hashing. He also points to the work of Thomas Wang
who has similar findings.
The program here tests their algorithms in order to find one
well suited for IP address hashing.
Willy Tarreau [Fri, 28 Mar 2008 17:09:38 +0000 (18:09 +0100)]
[BUG] fix double-decrement of server connections
If a client does a sudden dirty close (CL_STCLOSE) during a server
connect turn-around, then the number of server connections is
decremented twice. This causes huge problems on the affected
server because when its connection number becomes negative, it
overflows and prevents the server from accepting new connections
due to an apparent saturation.
The fix consists in not decrementing the counter if the server is
in a turn-around state.
Christian Wiese [Mon, 17 Mar 2008 17:23:12 +0000 (18:23 +0100)]
[BUILD] Replace hardcoded 'LD = gcc' with 'LD = $(CC)'
haproxy relies on linking the binary using gcc, so there is no real need to
hardcode both (CC and LD). Setting 'LD = $(CC)' will make the build system
a bit more cross-compile friendly because only the right cross-compiler has
to be passed via make.
Willy Tarreau [Mon, 17 Mar 2008 20:38:24 +0000 (21:38 +0100)]
[MEDIUM] fix stats socket limitation to 16 kB
Due to the way the stats socket work, it was not possible to
maintain the information related to the command entered, so
after filling a whole buffer, the request was lost and it was
considered that there was nothing to write anymore.
The major reason was that some flags were passed directly
during the first call to stats_dump_raw() instead of being
stored persistently in the session.
To definitely fix this problem, flags were added to the stats
member of the session structure.
A second problem appeared. When the stats were produced, a first
call to client_retnclose() was performed, then one or multiple
subsequent calls to buffer_write_chunks() were done. But once the
stats buffer was full and a reschedule operated, the buffer was
flushed, the write flag cleared from the buffer and nothing was
done to re-arm it.
For this reason, a check was added in the proto_uxst_stats()
function in order to re-call the client FSM when data were added
by stats_dump_raw(). Finally, the whole unix stats dump FSM was
rewritten to avoid all the magics it depended on. It is now
simpler and looks more like the HTTP one.
Christian Wiese [Wed, 12 Mar 2008 13:25:35 +0000 (15:25 +0200)]
[BUILD] Added support for 'make install'
To be flexible while installing haproxy following variables have been
added to the Makefile:
- DESTDIR useful i.e. while installing in a sandbox (not set by default)
- PREFIX defines the default install prefix (default: /usr/local)
- SBINDIR defines the dir the haproxy binary gets installed
(default: $PREFIX/sbin)
Willy Tarreau [Tue, 11 Mar 2008 05:37:39 +0000 (06:37 +0100)]
[BUILD] ensure that users don't build without setting the target anymore.
Too often, people report performance issues on Linux 2.6 because they don't
use the available optimizations. We need to ensure that people are aware of
the available features, and for this, we must force them to choose a target
OS (or "generic"), but at least prevent them from blindly building for a
generic target.
Willy Tarreau [Mon, 10 Mar 2008 21:04:20 +0000 (22:04 +0100)]
[MAJOR] implementation of the "leastconn" load balancing algorithm
The new "leastconn" LB algorithm selects the server which has the
least established or pending connections. The weights are considered,
so that a server with a weight of 20 will get twice as many connections
as the server with a weight of 10.
The algorithm respects the minconn/maxconn settings, as well as the
slowstart since it is a dynamic algorithm. It also correctly supports
backup servers (one and all).
It is generally suited for protocols with long sessions (such as remote
terminals and databases), as it will ensure that upon restart, a server
with no connection will take all new ones until its load is balanced
with others.
A test configuration has been added in order to ease regression testing.
Willy Tarreau [Sat, 8 Mar 2008 20:42:54 +0000 (21:42 +0100)]
[BUG] option allbackups was not working anymore in roundrobin mode
Commit 3168223a7b33a1d5aad1e11b8f2ad917645d7f27 broke option
"allbackups" in roundrobin mode due to an erroneous structure
member replacement in backend.c. The PR_O_USE_ALL_BK flag was
not tested in the right member anymore.
This bug uncoverred another one, by which all backup servers would
be used whatever the option's value, if all of them had been seen
as simultaneously failed at one moment.
This patch fixes the two stupid errors. Correctness has been tested
using the test-fwrr.cfg config example.
Willy Tarreau [Fri, 7 Mar 2008 09:07:04 +0000 (10:07 +0100)]
[BUG] str2sun could leak a small buffer in case of error during parsing
Matt Farnsworth reported a memory leak in str2sun() in case a too large
socket path is passed. The bug is very minor because it only happens
once during config parsing, but has to be fixed nevertheless. The patch
Matt provided could even be improved by completely removing the useless
strdup() in this function.
Currently there is a ~16KB limit for a data size passed via unix socket.
It is caused by a trivial bug ttat is going to fixed soon, however
in most cases there is no need to dump a full stats.
This patch makes possible to select a scope of dumped data by extending
current "show stat" to "show stat [<iid> <type> <sid>]":
- iid is a proxy id, -1 to dump all proxies
- type selects type of dumpable objects: 1 for frontend, 2 for backend, 4 for
server, -1 for all types. Values can be ORed, for example:
1+2=3 -> frontend+backend.
1+2+4=7 -> frontend+backend+server.
- sid is a service id, -1 to dump everything from the selected proxy.
To do this I implemented a new session flag (SN_STAT_BOUND), added three
variables in data_ctx.stats (iid, type, sid), modified dumpstats.c and
completely revorked the process_uxst_stats: now it waits for a "\n"
terminated string, splits args and uses them. BTW: It should be quite easy
to add new commands, for example to enable/disable servers, the only problem
I can see is a not very lucky config name (*stats* socket). :|
During the work I also fixed two bug:
- s->flags were not initialized for proto_uxst
- missing comma if throttling not enabled (caused by a stupid change in
"Implement persistent id for proxies and servers")
Other changes:
- No more magic type valuse, use STATS_TYPE_FE/STATS_TYPE_BE/STATS_TYPE_SV
- Don't memset full s->data_ctx (it was clearing s->data_ctx.stats.{iid/type/sid},
instead initialize stats.sv & stats.sv_st (stats.px and stats.px_st were already
initialized)
With all that changes it was extremely easy to write a short perl plugin
for a perl-enabled net-snmp (also included in this patch).
29385 is my PEN (Private Enterprise Number) and I'm willing to donate
the SNMPv2-SMI::enterprises.29385.106.* OIDs for HAProxy if there
is nothing assigned already.
[MEDIUM]: Prevent redispatcher from selecting the same server, version #3
When haproxy decides that session needs to be redispatched it chose a server,
but there is no guarantee for it to be a different one. So, it often
happens that selected server is exactly the same that it was previously, so
a client ends up with a 503 error anyway, especially when one sever has
much bigger weight than others.
Changes from the previous version:
- drop stupid and unnecessary SN_DIRECT changes
- assign_server(): use srvtoavoid to keep the old server and clear s->srv
so SRV_STATUS_NOSRV guarantees that t->srv == NULL (again)
and get_server_rr_with_conns has chances to work (previously
we were passing a NULL here)
- srv_redispatch_connect(): remove t->srv->cum_sess and t->srv->failed_conns
incrementing as t->srv was guaranteed to be NULL
- add avoididx to get_server_rr_with_conns. I hope I correctly understand this code.
- fix http_flush_cookie_flags() and move it to assign_server_and_queue()
directly. The code here was supposed to set CK_DOWN and clear CK_VALID,
but: (TX_CK_VALID | TX_CK_DOWN) == TX_CK_VALID == TX_CK_MASK so:
if ((txn->flags & TX_CK_MASK) == TX_CK_VALID)
txn->flags ^= (TX_CK_VALID | TX_CK_DOWN);
was really a:
if ((txn->flags & TX_CK_MASK) == TX_CK_VALID)
txn->flags &= TX_CK_VALID
Now haproxy logs "--DI" after redispatching connection.
- defer srv->redispatches++ and s->be->redispatches++ so there
are called only if a conenction was redispatched, not only
supposed to.
- don't increment lbconn if redispatcher selected the same sarver
- don't count unsuccessfully redispatched connections as redispatched
connections
- don't count redispatched connections as errors, so:
- the number of connections effectively served by a server is:
srv->cum_sess - srv->failed_conns - srv->retries - srv->redispatches
and
SUM(servers->failed_conns) == be->failed_conns
- requires the "Don't increment server connections too much + fix retries" patch
- needs little more testing and probably some discussion so reverting to the RFC state
Tests #1:
retries 4
redispatch
i) 1 server(s): b (wght=1, down)
b) sessions=5, lbtot=1, err_conn=1, retr=4, redis=0
-> request failed
ii) server(s): b (wght=1, down), u (wght=1, down)
b) sessions=4, lbtot=1, err_conn=0, retr=3, redis=1
u) sessions=1, lbtot=1, err_conn=1, retr=0, redis=0
-> request FAILED
iii) 2 server(s): b (wght=1, down), u (wght=1, up)
b) sessions=4, lbtot=1, err_conn=0, retr=3, redis=1
u) sessions=1, lbtot=1, err_conn=0, retr=0, redis=0
-> request OK
iv) 2 server(s): b (wght=100, down), u (wght=1, up)
b) sessions=4, lbtot=1, err_conn=0, retr=3, redis=1
u) sessions=1, lbtot=1, err_conn=0, retr=0, redis=0
-> request OK
v) 1 server(s): b (down for first 4 SYNS)
b) sessions=5, lbtot=1, err_conn=0, retr=4, redis=0
-> request OK
Tests #2:
retries 4
i) 1 server(s): b (down)
b) sessions=5, lbtot=1, err_conn=1, retr=4, redis=0
-> request FAILED
[BUG] Don't increment server connections too much + fix retries
Commit 98937b875798e10fac671d109355cde29d2a411a while fixing
one bug introduced another one. With "retries 4" and
"option redispatch" haproxy tries to connect 4 times to
one server server and 1 time to a second one. However
logs showed 5 connections to the first server (the
last one was counted twice) and 2 to the second.
This patch also fixes srv->retries and be->retries increments.
Now I get: 3 retries and 1 error in a first server (4 cum_sess)
and 1 error in a second server (1 cum_sess) with:
retries 4
option redispatch
[MINOR] Implement persistent id for proxies and servers
This patch adds a possibility to set a persistent id for a proxy/server.
Now, even if some proxies/servers are inserted/deleted/moved, iids and
sids can be still used reliable.
Some people add servers with tricky names (BACKEND or FRONTEND for example).
So I also added one more field ('type') to distinguish between a
backend (0), frontend (1) and server (2) without complicated logic:
if name==BACKEND and sid==0 then type is BACKEND else type is SERVER,
etc for a FRONTEND. It also makes possible to have one frontend with more
than one IP (a patch coming soon) with independed stats - for example to
differs between remote and local traffic.
Finally, I added documentation about the CSV format.
This patch depends on '[MEDIUM] Implement "track [<backend>/]<server>"'
This patch implements ability to set the current state of one server
by tracking another one. It:
- adds two variables: *tracknext, *tracked to struct server
- implements findserver(), similar to findproxy()
- adds "track" keyword accepting both "proxy/server" and "server" (assuming current proxy)
- verifies if both checks and tracking is not enabled at the same time
- changes set_server_down() to notify tracking server
- creates set_server_up(), set_server_disabled(), set_server_enabled() by
moving the code from process_chk() and adding notifications
- changes stats to show a name of tracked server instead of Chk/Dwn/Dwntime(html)
or by adding new variable (csv)
Changes from the previuos version:
- it is possibile to track independently of the declaration order
- one extra comma bug is fixed
- new condition to check if there is no disable-on-404 inconsistency
Ryan Warnick [Sun, 17 Feb 2008 10:24:35 +0000 (11:24 +0100)]
[BUG] appsession lookup in URL does not work
We've been trying to use the latest release (1.3.14.2) of haproxy to do
sticky sessions. Cookie insertion is not an option for us, although we
would much rather use it, as we are trying to work around a problem where
cookies are unreliable. The appsession functionality only partially worked
(it wouldn't read the session id out of a query string) until we made the
following code change to the get_srv_from_appsession function in
proto_http.c.
Willy Tarreau [Sun, 27 Jan 2008 01:21:53 +0000 (02:21 +0100)]
[OPTIM] GCC4's builtin_expect() is suboptimal
GCC4 is stupid (unbelievable news!).
When some code uses __builtin_expect(x != 0, 1), it really performs
the check of x != 0 then tests that the result is not zero! This is
a double check when only one was expected. Some performance drops
of 10% in the HTTP parser code have been observed due to this bug.
GCC 3.4 is fine though.
A solution consists in expecting that the tested value is 1. In
this case, it emits the correct code, but it's still not optimal
it seems. Finally the best solution is to ignore likely() and to
pray for the compiler to emit correct code. However, we still have
to fix unlikely() to remove the test there too, and to fix all
code which passed pointers overthere to pass integers instead.
Willy Tarreau [Sat, 26 Jan 2008 23:34:10 +0000 (00:34 +0100)]
[OPTIM] used unsigned ints for HTTP state and message offsets
State and offsets within http_msg were incorrectly set to signed int.
Turning them into unsigned slightly improved performance while reducing
code size.
Willy Tarreau [Tue, 12 Feb 2008 23:45:24 +0000 (00:45 +0100)]
[MEDIUM] completely implement the server redirection method
Now when a server has "redir <prefix>" on its config line, any HEAD or GET
request addressing it will lead to a 302 with Location set to "<prefix>"
immediately followed by the relative URI of the incoming request. This makes
it very easy to send redirect to browsers to check remote static servers, as
well as to provide redirection for remote sites when the local one is down.
Willy Tarreau [Tue, 12 Feb 2008 22:16:33 +0000 (23:16 +0100)]
[MINOR] add configuration support for "redir" server keyword
The servers now support the "redir" keyword, making it possible to
return a 302 with the specified prefix in front of the request instead
of connecting to them. This is generally useful for multi-site load
balancing but may also serve in order to achieve very high traffic
rate.
The keyword has only been added to the config parser and to structures,
it's not used yet.
This patch adds two new variables: fastinter and downinter.
When server state is:
- non-transitionally UP -> inter (no change)
- transitionally UP (going down), unchecked or transitionally DOWN (going up) -> fastinter
- down -> downinter
It allows to set something like:
server sr6 127.0.51.61:80 cookie s6 check inter 10000 downinter 20000 fastinter 500 fall 3 weight 40
In the above example haproxy uses 10000ms between checks but as soon as
one check fails fastinter (500ms) is used. If server is down
downinter (20000) is used or fastinter (500ms) if one check pass.
Fastinter is also used when haproxy starts.
New "timeout.check" variable was added, if set haproxy uses it as an additional
read timeout, but only after a connection has been already established. I was
thinking about using "timeout.server" here but most people set this
with an addition reserve but still want checks to kick out laggy servers.
Please also note that in most cases check request is much simpler
and faster to handle than normal requests so this timeout should be smaller.
I also changed the timeout used for check connections establishing.
Changes from the previous version:
- use tv_isset() to check if the timeout is set,
- use min("timeout connect", "inter") but only if "timeout check" is set
as this min alone may be to short for full (connect + read) check,
- debug code (fprintf) commented/removed
- documentation
Compile tested only (sorry!) as I'm currently traveling but changes
are rather small and trivial.
It should be incremented in session_process_counters while sending data to a
client:
bytes = s->rep->total - s->logs.bytes_out;
s->logs.bytes_out = s->rep->total;
However, if we increment (set) s->logs.bytes_out while processing
"logasap", statistics get wrong values added for headers: 0 or even
negative if haproxy adds some headers itself.
To test it, please enable logasap and download one empty file and look at
stats. Without my fix information available on that page are invalid, for
example:
Willy Tarreau [Sat, 19 Jan 2008 12:46:35 +0000 (13:46 +0100)]
[MINOR] use s->frt_addr as the server's address in transparent proxy
There's no point trying to check original dest addr with only one
method when doing transparent proxy as in full transparent mode,
the real destination address is required. Let's copy the one from
the frontend.
Willy Tarreau [Fri, 18 Jan 2008 16:20:13 +0000 (17:20 +0100)]
[BUG] fix truncated responses with sepoll
Due to the way Linux delivers EPOLLIN and EPOLLHUP, a closed connection
received after some server data sometimes results in truncated responses
if the client disconnects before server starts to respond. The reason
is that the EPOLLHUP flag is processed as an indication of end of
transfer while some data may remain in the system's socket buffers.
This problem could only be triggered with sepoll, although nothing should
prevent it from happening with normal epoll. In fact, the work factoring
performed by sepoll increases the risk that this bug appears.
The fix consists in making FD_POLL_HUP and FD_POLL_ERR sticky and that
they are only checked if FD_POLL_IN is not set, meaning that we have
read all pending data.
That way, the problem is definitely fixed and sepoll still remains about
17% faster than epoll since it can take into account all information
returned by the kernel.
Willy Tarreau [Wed, 16 Jan 2008 15:17:06 +0000 (16:17 +0100)]
[DOC] added "server", "source" and "stats" keywords
The documentation now lists all keywords except the req* and rsp*. The
"server" keyword has been documented for mandatory parameters. Specific
settings are still waiting to be written in a dedicated section.