Willy Tarreau [Fri, 7 May 2021 06:42:39 +0000 (08:42 +0200)]
MEDIUM: log: slightly refine the output format of alerts/warnings/etc
For about 20 years we've been emitting cryptic messages on warnings and
alerts, that nobody knows how to parse:
[NOTICE] 126/080118 (3115) : haproxy version is 2.4-dev18-0b7c78-49
[NOTICE] 126/080118 (3115) : path to executable is ./haproxy
[WARNING] 126/080119 (3115) : Server default/srv1 is DOWN via static/srv1. 0 active and 0 backup servers left. 0 sessions active, 0 requeued, 0 remaining in queue.
[ALERT] 126/080119 (3115) : backend 'default' has no server available!
Hint: the first 3-digit number is the day of year, and the 6 digits
after it represent the time of day in format HHMMSS, then the pid in
parenthesis. These are not quite user-friendly and such cryptic into
are not useful at all.
This patch slightly adjusts the output by performing these minimal changes:
- removing the date/time, as they were added very early when haproxy
was meant to be used in foreground as a debugging tool, and they're
provided in more details in logs nowadays ;
- better aligning the fields by padding the severity tag to 10 chars.
The diag output was renamed to "DIAG" only.
Now the output provides this:
[NOTICE] (4563) : haproxy version is 2.4-dev18-75a428-51
[NOTICE] (4563) : path to executable is ./haproxy
[WARNING] (4563) : Server default/srv1 is DOWN via static/srv1. 0 active and 0 backup servers left. 0 sessions active, 0 requeued, 0 remaining in queue.
[ALERT] (4563) : backend 'default' has no server available!
The useless space before the colon was kept so as not to confuse any
possible output parser.
The few entries in the doc referring to this format were adjusted to
reflect the new one.
The change was tagged "MEDIUM" as it may have visible consequences on
home-grown monitoring tools, though it is extremely unlikely due to the
limited extent of these changes.
Willy Tarreau [Fri, 7 May 2021 06:19:30 +0000 (08:19 +0200)]
BUG/MINOR: stream: properly clear the previous error mask on L7 retries
The cleanup of the previous error was incorrect on L7 retries, it would
OR two values while they're part of an enum, leaving some bits set.
Depending on the errors it was possible to occasionally see an internal
error ("I" flag) being logged.
This should be backported as far as 2.0, though the do_l7_retry() function
in in proto_htx.c in older versions.
Willy Tarreau [Fri, 7 May 2021 06:01:35 +0000 (08:01 +0200)]
BUG/MINOR: activity: use the new pointer to calculate the new size in realloc()
When memory profiling is enabled, realloc() can occasionally get the area
size wrong due to the wrong pointer being used to check the new size. When
the old area gets unmapped in the operation, this may even result in a
crash. There's no impact without memory profiling though.
No backport is needed as this is exclusively 2.4-dev.
Willy Tarreau [Thu, 6 May 2021 14:53:26 +0000 (16:53 +0200)]
MINOR: config: add predicates "version_atleast" and "version_before" to cond blocks
These predicates respectively verify that the current version is at least
a given version or is before a specific one. The syntax is exactly the one
reported by "haproxy -v", though each component is optional, so both "1.5"
and "2.4-dev18-88910-48" are supported. Missing components equal zero, and
"dev" is below "pre" or "rc", which are both inferior to no such mention
(i.e. they are negative). Thus "2.4-dev18" is older than "2.4-rc1" which
is older than "2.4".
Willy Tarreau [Thu, 6 May 2021 14:34:23 +0000 (16:34 +0200)]
MINOR: config: add predicate "feature" to detect certain built-in features
The "feature(name)" predicate will return true if <name> corresponds to
a name listed after a '+' in the features list, that is it was enabled at
build time with USE_<name>=1. Typical use cases will include OPENSSL, LUA
and LINUX_SPLICE. But maybe it will also be convenient to use with optional
addons such as PROMEX and the device detection modules to help keeping the
same configs across various deployments.
Willy Tarreau [Thu, 6 May 2021 14:10:09 +0000 (16:10 +0200)]
MINOR: config: add predicates "streq()" and "strneq()" to conditional expressions
"streq(str1,str2)" will return true if the two strings match while
"strneq(str1,str2)" will return true only if they differ. This is
convenient to match an environment variable against a predefined value.
Willy Tarreau [Thu, 6 May 2021 13:49:04 +0000 (15:49 +0200)]
MINOR: config: make cfg_eval_condition() support predicates with arguments
Now we can look up a list of known predicates and pre-parse their
arguments. For now the list is empty. The code needed to be arranged with
a common exit point to release all arguments because there's no default
argument freeing function (it likely only used to exist in the deinit
code). Since we only support simple arguments for now it's no big deal,
only a 2-liner loop.
Let's return the position of the first unparsable character on error,
so that instead of just saying "unparsable conditional expression blah"
we can have:
[ALERT] 125/150618 (13995) : parsing [test-conds2.cfg:1]: unparsable conditional expression '12/blah' in '.if' at position 1:
.if 12/blah
^
This is important because conditions will be made from environment
variables or later from more complex expressions where the error will
not always be easy to locate.
Willy Tarreau [Thu, 6 May 2021 05:43:35 +0000 (07:43 +0200)]
MINOR: global: add version comparison functions
The new function split_version() converts a parsable haproxy version to
an array of integers. The function compare_current_version() compares an
arbitrary version to the current one. These two functions were written
by Thierry Fournier in 2013, and are still usable as-is. They will be
used to write config language predicates.
Willy Tarreau [Thu, 6 May 2021 14:30:32 +0000 (16:30 +0200)]
MINOR: global: export the build features string list
Till now it was only presented in the version output but could not be
consulted outside of haproxy.c, let's export it as a variable, and set
it to an empty string if not defined.
Willy Tarreau [Thu, 6 May 2021 12:50:30 +0000 (14:50 +0200)]
MINOR: arg: improve the error message on missing closing parenthesis
When the closing brace is missing after an argument (acl, ...), the
error may report something like "expected ')' before ''". Let's just
drop "before ''" when the final word is empty to make the message a
bit clearer.
Willy Tarreau [Thu, 6 May 2021 08:25:11 +0000 (10:25 +0200)]
MINOR: config: support some pseudo-variables for file/line/section
The new pseudo-variables ".FILE", ".LINE" and ".SECTION" will be resolved
on the fly by the config parser and will respectively retrieve the current
configuration file name, the current line number and the current section
being parsed. This may help emit logs, errors, and debugging information
(e.g. which rule matched).
The '.' in the first char was reserved for such pseudo-variables and no
other variable is permitted. This will allow to add support for new ones
in the future if they prove to be useful (e.g. randoms/uuid for secret
keying or automatic naming of configuration objects).
Willy Tarreau [Thu, 6 May 2021 08:04:45 +0000 (10:04 +0200)]
MINOR: config: keep up-to-date current file/line/section in the global struct
Let's add a few fields to the global struct to store information about
the current file being processed, the current line number and the current
section. This will be used to retrieve them using special variables.
Willy Tarreau [Thu, 6 May 2021 06:19:48 +0000 (08:19 +0200)]
MINOR: config: centralize the ".if"/".elif" condition parser and evaluator
Instead of duplicating the condition evaluations, let's have a single
function cfg_eval_condition() that returns true/false/error. It takes
less code and will ease its extension.
Willy Tarreau [Thu, 6 May 2021 06:48:09 +0000 (08:48 +0200)]
BUG/MINOR: config: add a missing "ELIF_TAKE" test for ".elif" condition evaluator
This missing state was causing a second elif condition to be evaluated
after a first one succeeded after a .if failed. For example in the test
below the else would be executed:
Willy Tarreau [Thu, 6 May 2021 06:46:11 +0000 (08:46 +0200)]
BUG/MINOR: config: fix uninitialized initial state in ".if" block evaluator
The condition to skip the block in the ".if" evaluator forgot to check
that the level was high enough, resulting in rare cases where a random
value matched one of the 5 values that cause the block to be skipped.
BUG/MINOR: stream: Decrement server current session counter on L7 retry
When a L7 retry is performed, we must not forget to decrement the current
session counter of the assigned server. Of course, it must only be done if
the current session is already counted on the server, thus if SF_CURR_SESS
flag is set on the stream.
This patch is related to the issue #1003. It must be backported as far as
2.0.
MINOR: mux-h1: Manage processing blocking flags on the H1 stream
Because H1C_F_RX_BLK and H1C_F_TX_BLK flags now only concerns data
processing, at the H1 stream level, there is no reason to still manage them
on the H1 connection. Thus, these flags are now set on the H1 stream.
These flags are used to block, respectively, the output and the input
processing. Thus, to be more explicit, H1C_F_WAIT_INPUT is renamed to
H1C_F_TX_BLK and H1C_F_WAIT_OUTPUT is renamed to H1C_F_RX_BLK.
MEDIUM: mux-h1: Wake H1 stream when both sides a synchronized
Instead of subscribing for reads or sends to restart data processing, when
both sides are synchronized, the H1 stream is woken up. This happens when
H1C_F_WAIT_INPUT or H1C_F_WAIT_OUTPUT flags are removed, Indeed, these flags
block the data processing and not raw data sending or receiving.
MINOR: mux-h1: Always subscribe for reads when splicing is disabled
In h1_rcv_pipe(), when the splicing is not possible or disabled at the end
of the fnuction, we make sure to subscribe for reads. It is not a bug but it
avoid an extra call to h1_rcv_pipe() to handle the subscription in some
cases (end of message, end of chunk or read0).
In addition, the condition to detect end of splicing has been simplified. We
now only rely on H1C_F_WANT_SPLICE flags.
MINOR: mux-h1: Subscribe for sends if output buffer is not empty in h1_snd_pipe
In h1_snd_pipe(), before sending spliced data, we take care to flush the
output buffer by subscribing for sends. However, the condition to do so is
not accurate. We test data remaining in the pipe. It works but it also
unnecessarily subscribes H1C for sends when the output buffer is empty if we
are unable to send all spliced data in one time. Instead, H1C is now
subscribed for sends if output buffer is not empty.
MINOR: mux-h1: clean up conditions to enabled and disabled splicing
First, there is no reason to announce the splicing support at the
conn-stream level when it is created, at least for now. GTUNE_USE_SPLICE
option is already handled at the stream level.
Second, in h1_rcv_buf(), there is no reason to test the message state to
switch the H1C in splicing mode (via H1C_F_WANT_SPLICE flag).
h1_process_input() already takes care to set CS_FL_MAY_SPLICE flag on the
conn-stream when appropriate. Thus, in h1_rcv_buf(), we can rely on this
flag to change the H1C state.
Finally, if h1_rcv_pipe() is called, it means the H1C is already in the
splicing mode. H1C_F_WANT_SPLICE flag is necessarily already set. Thus no
reason to force it.
This script test abortonclose option for HTTP/1 client only. It may be
backported as far as 2.0. But on the 2.2 and prior, the syslog part must be
adapted to catch log messages emitted by proxy during HAProxy
startup. Following lines must be added :
BUG/MEDIUM: mux-h1: Properly report client close if abortonclose option is set
On client side, if CO_RFL_KEEP_RECV flags is set when h1_rcv_buf() is
called, we force subscription for reads to be able to catch read0. This way,
the event will be reported to upper layer to let the stream abort the
request.
This patch fixes the abortonclose option for H1 connections. It depends on
following patches :
* MEDIUM: mux-h1: Don't block reads when waiting for the other side
* MINOR: conn-stream: Force mux to wait for read events if abortonclose is set
But to be sure the event is handled by the stream, the following patches are
also required :
* BUG/MINOR: stream-int: Don't block reads in si_update_rx() if chn may receive
* MINOR: channel: Rely on HTX version if appropriate in channel_may_recv()
All the series must be backported with caution as far as 2.0, and only after
a period of observation to be sure nothing broke.
MEDIUM: mux-h1: Don't block reads when waiting for the other side
When we are waiting for the other side to read more data, or to read the
next request, we must only stop the processing of input data and not the
data receipt. This patch don't change anything on the subscribes for
reads. So it should not change anything. The only difference is that the H1
connection will try to read data if it is woken up for an I/O event and if
it was subscribed for reads.
This patch is required to fix abortonclose option for H1 client connections.
MINOR: conn-stream: Force mux to wait for read events if abortonclose is set
When the abortonclose option is enabled, to be sure to be immediately
notified when a shutdown is received from the client, the frontend
conn-stream must be sure the mux will wait for read events. To do so, the
CO_RFL_KEEP_RECV flag is set when mux->rcv_buf() is called. This new flag
instructs the mux to wait for read events, regardless its internal state.
This patch is required to fix abortonclose option for H1 client connections.
BUG/MINOR: stream-int: Don't block reads in si_update_rx() if chn may receive
In si_update_rx() function, the reads may be blocked because we explicitly
don't want to read or because of a lack of room in the input buffer. The
first condition is valid. However the second one only test if the channel is
empty or not. It means the reads are blocked if there are still some output
data in the input channel, in its buffer or its pipe. This condition is not
accurate. The reads must not be blocked if the channel can still receive
data. Thus instead of relying on channel_is_empty() function, we now call
channel_may_recv().
This patch is especially useful to be able to catch read0 on client side
when we are waiting for a connection to the server, when abortonclose option
is enabled. Otherwise, the client abort is not detected.
This patch depends on "MINOR: channel: Rely on HTX version if appropriate in
channel_may_recv()". Both must be backported as far as 2.0 after a period of
observation to be sure nothing broke.
MINOR: channel: Rely on HTX version if appropriate in channel_may_recv()
When channel_may_recv() is called for an HTX stream, the HTX version,
channel_htx_may_recv() is called. This patch is mandatory to fix a bug
related to the abortonclose option.
Willy Tarreau [Wed, 5 May 2021 16:17:39 +0000 (18:17 +0200)]
BUILD: makefile: add new option USE_MEMORY_PROFILING
It is not enabled by default, and may only work on linux-glibc for now,
though maybe other platforms could adopt it, possibly with certain
restrictions.
Willy Tarreau [Wed, 5 May 2021 16:07:02 +0000 (18:07 +0200)]
MINOR: activity: make "show profiling" also dump the memoery usage
Now the memory usage stats are dumped. They are first sorted by total
alloc+free so that the first ones are always the most relevant, and
that most symmetric alloc/free pairs appear next to each other. This
way it becomes convenient to only show a small part of them such as:
show profiling memory 20
It's worth noting that the sorting is performed upon each call to the
iohandler so it is technically possible that an entry could appear
twice or be dropped if the ordering changes between two calls. In
practice it is not an issue but it's worth being mentioned.
Willy Tarreau [Wed, 5 May 2021 15:33:27 +0000 (17:33 +0200)]
MINOR: activity: clean up the show profiling io_handler a little bit
Let's rearrange it to make it more configurable and allow to iterate
over multiple parts (header, tasks, memory etc), to restart from a
given line number (previously it didn't work, though fortunately it
didn't happen), and to support dumping only certain parts and a given
number of lines. A few entries from ctx.cli are now used to store a
restart point and the current step.
Willy Tarreau [Wed, 5 May 2021 15:07:09 +0000 (17:07 +0200)]
MEDIUM: activity: collect memory allocator statistics with USE_MEMORY_PROFILING
When built with USE_MEMORY_PROFILING the main memory allocation functions
are diverted to collect statistics per caller. It is a bit tricky because
the only way to call the original ones is to find their pointer, which
requires dlsym(), and which is not available everywhere.
Thus all functions are designed to call their fallback function (the
original one), which is preset to an initialization function that is
supposed to call dlsym() to resolve the missing symbols, and vanish.
This saves expensive tests in the critical path.
A second problem is that dlsym() calls calloc() to initialize some
error messages. After plenty of tests with posix_memalign(), valloc()
and friends, it turns out that returning NULL still makes it happy.
Thus we currently use a visit counter (in_memprof) to detect if we're
reentering, in which case all allocation functions return NULL.
In order to convert a return address to an entry in the stats, we
perform a cheap hash consisting in multiplying the pointer by a
balanced number (as many zeros as ones) and keeping the middle bits.
The hash is already pretty good like this, achieving to store up to
638 entries in a 2048-entry table without collision. But in order to
further refine this and improve the fill ratio of the table, in case
of collision we move up to 16 adjacent entries to find a free place.
This remains quite cheap and manages to store all of these inside a
1024-entries hash table with even less risk of collision.
Also, free(NULL) does not produce any stats. By doing so we reduce
from 638 to 208 the average number of entries needed for a basic
config using SSL. free(NULL) not only provides no information as it's
a NOP, but keeping it is pure pollution as it happens all the time.
When DEBUG_MEM_STATS is enabled, malloc/calloc/realloc are redefined as
macros, preventing the code from compiling. Thus, when this option is
detected, the macros are undefined as they are pointless there anyway.
The functions are optimized to quickly jump to the fallback and as such
become almost invisible in terms of processing time, execpt an extra
"if" on a read_mostly variable and a jump. Considering that this only
happens for pool misses and library routines, this remains acceptable.
Performance tests in SSL (the most stressful test) shows less than 1%
performance loss when profiling is enabled on 2c4t.
The code was written in a way to ease backporting to modern versions
(2.2+) if needed, so it keeps the long names for integers and doesn't
use the _INC version of the atomic ops.
Willy Tarreau [Wed, 5 May 2021 14:50:40 +0000 (16:50 +0200)]
MINOR: activity: declare the storage for memory usage statistics
We'll need to store for each call place, the pointer to the caller
(the return address to be more exact as with free() it's not uncommon
to see tail calls), the number of calls to alloc/free and the total
alloc/free bytes. realloc() will be counted either as alloc or free
depending on the balance of the size before vs after.
We store 1024+1 entries. The first ones are used as hashes and the
last one for collisions.
When profiling is enabled via the CLI, all the stats are reset.
Willy Tarreau [Wed, 5 May 2021 14:28:31 +0000 (16:28 +0200)]
CLEANUP: activity: mark the profiling and task_profiling_mask __read_mostly
These ones are only read by the scheduler and occasionally written to
by the CLI parser, so let's move them to read_mostly so that they do
not risk to suffer from cache line pollution.
Willy Tarreau [Wed, 5 May 2021 07:06:21 +0000 (09:06 +0200)]
MINOR: tools: add functions to retrieve the address of a symbol
get_sym_curr_addr() will return the address of the first occurrence of
the given symbol while get_sym_next_addr() will return the address of
the next occurrence of the symbol. These ones return NULL on non-linux,
non-ELF, non-USE_DL.
MEDIUM: connection: close front idling connection on soft-stop
Implement a safe mechanism to close front idling connection which
prevents the soft-stop to complete. Every h1/h2 front connection is
added in a new per-thread list instance. On shutdown, a new task is
waking up which calls wake mux operation on every connection still
present in the new list.
A new stopping_list attach point has been added in the connection
structure. As this member is only used for frontend connections, it
shared the same union as the session_list reserved for backend
connections.
MEDIUM: mux_h1: release idling frontend conns on soft-stop
In h1_process, if the proxy of a frontend connection is disabled,
release the connection.
This commit is in preparation to properly close idling front connections
on soft-stop. h1_process must still be called, this will be done via a
dedicated task which monitors the global variable stopping.
MINOR: connection: move session_list member in a union
Move the session_list attach point in an anonymous union. This member is
only used for backend connections. This commit is in preparation for the
support of stopping frontend idling connections which will add another
member to the union.
This change means that a special care must be taken to be sure that only
backend connections manipulate the session_list. A few BUG_ON has been
added as special guard to prevent from misuse.
MINOR: srv: close all idle connections on shutdown
Implement a function to close all server idle connections. This function
is called via a global deinit server handler.
The main objective is to prevents from leaving sockets in TIME_WAIT
state. To limit the set of operations on shutdown and prevents
tasks rescheduling, only the ctrl stack closing is done.
The purpose of this debugging option was to prevent certain pools from
masking other ones when they were shared. For example, task, http_txn,
h2s, h1s, h1c, session, fcgi_strm, and connection are all 192 bytes and
would normally be mergedi, but not with this option. The problem is that
certain pools are declared multiple times with various parameters, which
are often very close, and due to the way the option works, they're not
shared either. Good examples of this are captures and stick tables. Some
configurations have large numbers of stick-tables of pretty similar types
and it's very common to end up with the following when the option is
enabled:
In addition to not being convenient, it can have important effects on the
memory usage because these pools will not share their entries, so one stick
table cannot allocate from another one's pool.
This patch solves this by going back to the initial goal which was not to
have different pools in the same list. Instead of masking the MAP_F_SHARED
flag, it simply adds a test on the pool's name, and disables pool sharing
if the names differ. This way pools are not shared unless they're of the
same name and size, which doesn't hinder debugging. The same test above
now returns this:
Willy Tarreau [Tue, 4 May 2021 16:40:50 +0000 (18:40 +0200)]
MINOR: debug: add a new "debug dev sym" command in expert mode
This command attempts to resolve a pointer to a symbol name. This is
convenient during development as it's easier to get such pointers live
than by issuing a debugger or calling addr2line.
Willy Tarreau [Tue, 4 May 2021 14:27:45 +0000 (16:27 +0200)]
BUG/MEDIUM: cli: prevent memory leak on write errors
Since the introduction of payload support on the CLI in 1.9-dev1 by
commit abbf60710 ("MEDIUM: cli: Add payload support"), a chunk is
temporarily allocated for the CLI to support defragmenting a payload
passed with a command. However it's only released when passing via
the CLI_ST_END state (i.e. on clean shutdown), but not on errors.
Something as trivial as:
$ while :; do ncat --send-only -U /path/to/cli <<< "show stat"; done
with a few hundreds of servers is enough see the number of allocated
trash chunks go through the roof in "show pools".
BUG/MINOR: hlua: Don't rely on top of the stack when using Lua buffers
When the lua buffers are used, a variable number of stack slots may be
used. Thus we cannot assume that we know where the top of the stack is. It
was not an issue for lua < 5.4.3 (at least for small buffers). But
'socket:receive()' now fails with lua 5.4.3 because a light userdata is
systematically pushed on the top of the stack when a buffer is initialized.
To fix the bug, in hlua_socket_receive(), we save the index of the top of
the stack before creating the buffer. This way, we can check the number of
arguments, regardless anything was pushed on the stack or not.
Note that the other buffer usages seem to be safe.
This patch should solve the issue #1240. It should be backport to all stable
branches.
Willy Tarreau [Sat, 1 May 2021 06:25:15 +0000 (08:25 +0200)]
[RELEASE] Released version 2.4-dev18
Released version 2.4-dev18 with the following main changes :
- DOC: Fix indentation for `path-strip-dot` normalizer
- DOC: Fix RFC reference for the percent-to-uppercase normalizer
- DOC: Add RFC references for the path-strip-dot(dot)? normalizers
- MINOR: uri_normalizer: Add a `percent-decode-unreserved` normalizer
- BUG/MINOR: mux-fcgi: Don't send normalized uri to FCGI application
- REORG: htx: Inline htx functions to add HTX blocks in a message
- CLEANUP: assorted typo fixes in the code and comments
- DOC: general: fix white spaces for HTML converter
- BUG/MINOR: ssl: ssl_sock_prepare_ssl_ctx does not return an error code
- BUG/MINOR: cpuset: move include guard at the very beginning
- BUG/MAJOR: fix build on musl with cpu_set_t support
- BUG/MEDIUM: cpuset: fix build on MacOS
- BUG/MINOR: htx: Preserve HTX flags when draining data from an HTX message
- MEDIUM: htx: Refactor htx_xfer_blks() to not rely on hdrs_bytes field
- CLEANUP: htx: Remove unsued hdrs_bytes field from the HTX start-line
- BUG/MINOR: mux-h2: Don't encroach on the reserve when decoding headers
- MEDIUM: http-ana: handle read error on server side if waiting for response
- MINOR: htx: Limit length of headers name/value when a HTX message is dumped
- BUG/MINOR: applet: Notify the other side if data were consumed by an applet
- BUG/MINOR: hlua: Don't consume headers when starting an HTTP lua service
- BUG/MEDIUM: mux-h2: Handle EOM flag when sending a DATA frame with zero-copy
- CLEANUP: channel: No longer notify the producer in co_skip()/co_htx_skip()
- DOC: general: fix example in set-timeout
- CLEANUP: cfgparse: de-uglify early file error handling in readcfgfile()
- MINOR: config: add a new "default-path" global directive
- BUG/MEDIUM: peers: initialize resync timer to get an initial full resync
- BUG/MEDIUM: peers: register last acked value as origin receiving a resync req
- BUG/MEDIUM: peers: stop considering ack messages teaching a full resync
- BUG/MEDIUM: peers: reset starting point if peers appears longly disconnected
- BUG/MEDIUM: peers: reset commitupdate value in new conns
- BUG/MEDIUM: peers: re-work updates lookup during the sync on the fly
- BUG/MEDIUM: peers: reset tables stage flags stages on new conns
- MINOR: peers: add informative flags about resync process for debugging
- BUG/MEDIUM: time: fix updating of global_now upon clock drift
- CLEANUP: freq_ctr: make arguments of freq_ctr_total() const
- CLEANUP: hlua: rename hlua_appctx* appctx to luactx
- MINOR: server: fix doc/trace on lb algo for dynamic server creation
- REGTESTS: server: fix cli_add_server due to previous trace update
- REGTESTS: add minimal CLI "add map" tests
- DOC: management: move "set var" to the proper place
- CLEANUP: map: slightly reorder the add map function
- MINOR: map: get rid of map_add_key_value()
- MINOR: map: show the current and next pattern version in "show map"
- MINOR: map/acl: add the possibility to specify the version in "show map/acl"
- MINOR: pattern: support purging arbitrary ranges of generations
- MINOR: map/acl: add the possibility to specify the version in "clear map/acl"
- MINOR: map/acl: add the "prepare map/acl" CLI command
- MINOR: map/acl: add the "commit map/acl" CLI command
- MINOR: map/acl: make "add map/acl" support an optional version number
- CLEANUP: map/cli: properly align the map/acl help
- BUILD: compiler: do not use already defined __read_mostly on dragonfly
MINOR: map/acl: make "add map/acl" support an optional version number
By passing a version number to "add map/acl", it becomes possible to
atomically replace maps and ACLs. The principle is that a new version
number is first retrieved by calling"prepare map/acl", and this version
number is used with "add map" and "add acl". Newly added entries then
remain invisible to the matching mechanism but are visible in "show
map/acl" when the version number is specified, or may be cleard with
"clear map/acl". Finally when the insertion is complete, a
"commit map/acl" command must be issued, and the version is atomically
updated so that there is no intermediate state with incomplete entries.
MINOR: map/acl: add the "commit map/acl" CLI command
The command is used to atomically replace a map/acl with the pending
contents of the designated version. The new version must have been
allocated by "prepare map/acl" prior to this. At the moment it is not
possible to force the version when adding new entries, so this may only
be used to atomically clear an ACL/map.
MINOR: map/acl: add the "prepare map/acl" CLI command
This command allocates a new version for the map/acl, that will be usable
later to prepare the addition of new values to atomically replace existing
ones. Technically speaking the operation consists in atomically incrementing
the next version. There's no "undo" operation here, if a version is not
committed, it will automatically be trashed when committing a newer version.
MINOR: map/acl: add the possibility to specify the version in "clear map/acl"
This will ease maintenance of versionned maps by allowing to clear old or
failed updates instead of the current version. Nothing was done to allow
clearing everyhing, though if there was a need for this, implementing "@all"
or something equivalent wouldn't require more than 3 lines of code.
MINOR: pattern: support purging arbitrary ranges of generations
Instead of being able to purge only values older than a specific value,
let's support arbitrary ranges and make pat_ref_purge_older() just be
one special case of this one.
MINOR: map/acl: add the possibility to specify the version in "show map/acl"
The maps and ACLs internally all have two versions, the "current" one,
which is the one being matched against, and the "next" one, the one being
filled during an atomic replacement. Till now the "show" commands only used
to show the current one but it can be convenient to be able to show other
ones as well, so let's add the ability to do this with "show map" and
"show acl". The method used here consists in passing the version number
as "@<ver>" before the map/acl name or ID. It would have been better after
it but that could create confusion with keys already using such a format.
MINOR: map: show the current and next pattern version in "show map"
The "show map" command wasn't updated when pattern generations were
added for atomic reloads, let's report them in the "show map" command
that lists all known maps. It will be useful for users.
This function was only used once in cli_parse_add_map(), and half of the
work it used to do was already known from the caller or testable outside
of the lock. Given that we'll need to modify it soon to pass a generation
number, let's remerge it in the caller instead, using pat_ref_load() which
is the one we'll need.
CLEANUP: map: slightly reorder the add map function
The function uses two distinct code paths for single the key/value pair
and multiple pairs inserted as payload, each with a copy-paste of the
error handling. Let's modify the loop to factor them out.
DOC: management: move "set var" to the proper place
Commit b8bd1ee89 ("MEDIUM: cli: add a new experimental "set var" command")
added "get var" and "set var" but "set var" was misplaced in the doc,
breaking the alphabetic ordering.
The map_redirect test already tests for "show map", "del map" and
"clear map" but doesn't have any "add map" command. Let's add some
trivial ones involving one regular entry and two other ones added as
payload, checking they are properly returned.
REGTESTS: server: fix cli_add_server due to previous trace update
Error output for dynamic server creation if invalid lb algo has changed
since previous commit :
MINOR: server: fix doc/trace on lb algo for dynamic server creation
The vtest regex should have been updated has well to match it.
MINOR: server: fix doc/trace on lb algo for dynamic server creation
The text mentionned that only backends with consistent hash method were
supported for dynamic servers. In fact, it is only required that the lb
algorith is dynamic.
CLEANUP: hlua: rename hlua_appctx* appctx to luactx
There is some serious confusion in the lua interface code related to
sockets and services coming from the hlua_appctx structs being called
"appctx" everywhere, and where the real appctx is reached using
appctx->appctx. This part is a bit of a pain to debug so let's rename
all occurrences of this local variable to "luactx".
BUG/MEDIUM: time: fix updating of global_now upon clock drift
During commit 7e4a557f6 ("MINOR: time: change the global timeval and the
the global tick at once") the approach made sure that the new now_ms was
always higher than or equal to global_now_ms, but by forgetting the old
value. This can cause the first update to global_now_ms to fail if it's
already out of sync, going back into the loop, and the subsequent call
would then succeed due to commit 4d01f3dcd ("MINOR: time: avoid
overwriting the same values of global_now").
And if it goes out of sync, it will fail to update forever, as observed
by Ashley Penney in github issue #1194, causing incorrect freq counters
calculations everywhere. One possible trigger for this issue is one thread
spinning for a few milliseconds while the other ones continue to work.
The issue really is that old_now_ms ought not to be modified in the loop
as it's used for the CAS. But we don't need to structurally guarantee that
global_now_ms grows monotonically as it's computed from the new global_now
which is already verified for this via the __tv_islt() test. Thus, dropping
any corrections on global_now_ms in the loop is the correct way to proceed
as long as this one is always updated to follow global_now.
MINOR: peers: add informative flags about resync process for debugging
This patch adds miscellenous informative flags raised during the initial
full resync process performed during the reload for debugging purpose.
0x00000010: Timeout waiting for a full resync from a local node
0x00000020: Timeout waiting for a full resync from a remote node
0x00000040: Session aborted learning from a local node
0x00000080: Session aborted learning from a remote node
0x00000100: A local node teach us and was fully up to date
0x00000200: A remote node teach us and was fully up to date
0x00000400: A local node teach us but was partially up to date
0x00000800: A remote node teach us but was partially up to date
0x00001000: A local node was assigned for a full resync
0x00002000: A remote node was assigned for a full resync
0x00004000: A resync was explicitly requested
This patch could be backported on any supported branch
BUG/MEDIUM: peers: reset tables stage flags stages on new conns
Flags used as context to know current status of each table pushing a
full resync to a peer were correctly reset receiving a new resync
request or confirmation message but in case of local peer sync during
reload the resync request is implicit and those flags were not
correctly reset in this case.
This could result to a partial initial resync of some tables after reload
if the connection with the old process was broken and retried.
This patch reset those flags at the end of the handshake for all new
connections to be sure to push a entire full resync if needed.
This patch should be backported on all supported branches ( v >= 1.6 )
BUG/MEDIUM: peers: re-work updates lookup during the sync on the fly
Only entries between the opposite of the last 'local update' rotating
counter were considered to be pushed. This processing worked in most
cases because updates are continually pushed trying to reach this point
but it remains some cases where updates id are more far away in the past
and appearing in futur and the push of updates is stuck until the head
reach again the tail which could take a very long time.
This patch re-work the lookup to consider that all positions on the
rotating counter is considered in the past until we reach exactly
the 'local update' value. Doing this, the updates push won't be stuck
anymore.
This patch should be backported on all supported branches ( >= 1.6 )
Emeric Brun [Tue, 23 Feb 2021 16:08:08 +0000 (17:08 +0100)]
BUG/MEDIUM: peers: reset commitupdate value in new conns
The commitupdate value of the table is used to check if the update
is still pending for a push for all peers. To be sure to not miss a
push we reset it just after a handshake success.
This patch should be backported on all supported branches ( >= 1.6 )
Emeric Brun [Tue, 23 Feb 2021 15:50:53 +0000 (16:50 +0100)]
BUG/MEDIUM: peers: reset starting point if peers appears longly disconnected
If two peers are disconnected and during this period they continue to
process a large amount of local updates, after a reconnection they
may take a long time before restarting to push their updates. because
the last pushed update would appear internally in futur.
This patch fix this resetting the cursor on acked updates at the maximum
point considered in the past if it appears in futur but it means we
may lost some updates. A clean fix would be to update the protocol to
be able to signal a remote peer that is was not updated for a too long
period and needs a full resync but this is not yet supported by the
protocol.
This patch should be backported on all supported branches ( >= 1.6 )
Emeric Brun [Thu, 4 Mar 2021 09:27:10 +0000 (10:27 +0100)]
BUG/MEDIUM: peers: stop considering ack messages teaching a full resync
The re-con cursor was updated receiving any ack message
even if we are pushing a complete resync to a peer. This cursor
is reset at the end of the resync but if the connection is broken
during resync, we could re-start at an unwanted point.
With this patch, the peer stops to consider ack messages pushing
a resync since the resync process has is own acknowlegement and
is always restarted from the beginning in case of broken connection.
This patch should be backported on all supported branches ( >= 1.6 )
BUG/MEDIUM: peers: register last acked value as origin receiving a resync req
Receiving a resync request, the origins to start the full sync and
to reset after the full resync are mistakenly computed based on
the last update on the table instead of computed based on the
the last update acked by the node requesting the resync.
It could result in disordered or missing updates pushing to the
requester
This patch sets correctly those origins.
This patch should be backported on all supported branches ( >= 1.6 )
BUG/MEDIUM: peers: initialize resync timer to get an initial full resync
If a reload is performed and there is no incoming connections
from the old process to push a full resync, the new process
can be stuck waiting indefinitely for this conn and it never tries a
fallback requesting a full resync from a remote peer because the resync
timer was init to TICK_ETERNITY.
This patch forces a reset of the resync timer to default value (5 secs)
if we detect value is TICK_ETERNITY.
This patch should be backported on all supported branches ( >= 1.6 )
MINOR: config: add a new "default-path" global directive
By default haproxy loads all files designated by a relative path from the
location the process is started in. In some circumstances it might be
desirable to force all relative paths to start from a different location
just as if the process was started from such locations. This is what this
directive is made for. Technically it will perform a temporary chdir() to
the designated location while processing each configuration file, and will
return to the original directory after processing each file. It takes an
argument indicating the policy to use when loading files whose path does
not start with a slash ('/').
A few options are offered, "current" (the default), "config" (files
relative to config file's dir), "parent" (files relative to config file's
parent dir), and "origin" with an absolute path.
CLEANUP: cfgparse: de-uglify early file error handling in readcfgfile()
In readcfgfile() when malloc() fails to allocate a buffer for the
config line, it currently says "parsing[<file>]: out of memory" while
the error is unrelated to the config file and may make one think it has
to do with the file's size. The second test (fopen() returning error)
needs to release the previously allocated line. Both directly return -1
which is not even documented as a valid error code for the function.
Let's simply make sure that the few variables freed at the end are
properly preset, and jump there upon error, after having displayed a
meaningful error message. Now at least we can get this:
$ ./haproxy -f /dev/kmem
[NOTICE] 116/191904 (23233) : haproxy version is 2.4-dev17-c3808c-13
[NOTICE] 116/191904 (23233) : path to executable is ./haproxy
[ALERT] 116/191904 (23233) : Could not open configuration file /dev/kmem : Permission denied
Alex [Tue, 27 Apr 2021 10:57:07 +0000 (12:57 +0200)]
DOC: general: fix example in set-timeout
The alternative arguments are always in curly brackets, let's fix it for
set-timeout.
The Example in set-timeout does not have the one of the required argument.
This commit makes the PR https://github.com/cbonte/haproxy-dconv/pull/34
obsolete.
CLEANUP: channel: No longer notify the producer in co_skip()/co_htx_skip()
Thanks to the commit "BUG/MINOR: applet: Notify the other side if data were
consumed by an applet", it is no longer necessary to notify the producer when an
applet skips output data. Now, it is the default applet handler responsibility
to take care of that.
BUG/MEDIUM: mux-h2: Handle EOM flag when sending a DATA frame with zero-copy
When a DATA frame is sent, we must take care to properly detect the EOM flag
on the HTX message to set ES flag on the frame when necessary, to finish the
stream. But it is only done when data are copied from the HTX message to the
mux buffer and not when the frame are sent via a zero-copy. This patch fixes
this bug.
BUG/MINOR: hlua: Don't consume headers when starting an HTTP lua service
When an HTTP lua service is started, headers are consumed before calling the
script. When it was initialized, the headers were stored in a lua array,
thus they can be removed from the HTX message because the lua service will
no longer access them. But it is a problem with bodyless messages because
the EOM flag is lost. Indeed, once the headers are consumed, the message is
empty and the buffer is reset, included the flags.
Now, the headers are not immediately consumed. We will skip them if
applet:receive() or applet:getline(). This way, the EOM flag is preserved.
At the end, when the script is finished, all output data are consumed, thus
this remains safe.
BUG/MINOR: applet: Notify the other side if data were consumed by an applet
If an applet consumed output data (the amount of output data has changed
between before and after the call to the applet), the producer is
notified. It means CF_WRITE_PARTIAL and CF_WROTE_DATA are set on the output
channel and the opposite stream interface is notified some room was made in
its input buffer. This way, it is no longer the applet responsibility to
take care of it. However, it doesn't matter if the applet does the same.
Said like that, it looks like an improvement not a bug. But it really fixes
a bug in the lua, for HTTP applets. Indeed, applet:receive() and
applet:getline() are buggy for HTTP applets. Data are consumed but the
producer is not notified. It means if the payload is not fully received in
one time, the applet may be blocked because the producer remains blocked (it
is time dependent).
This patch must be backported as far as 2.0 (only for the HTX part).
MEDIUM: http-ana: handle read error on server side if waiting for response
A read error on the server side is also reported as a write error on the
client side. It means some times, a server side error is handled on the
client side. Among others, it is the case when the client side is waiting
for the response while the request processing is already finished. In this
case, the error is not handled as a server error. It is not accurate.
So now, when the request processing is finished but not the response
processing and if a read error was encountered on the server side, the error
is not immediatly processed on the client side, to let a chance to response
analysers to properly catch the error.
BUG/MINOR: mux-h2: Don't encroach on the reserve when decoding headers
Since the input buffer is transferred to the stream when it is created,
there is no longer control on the request size to be sure the buffer's
reserve is still respected. It was automatically performed in h2_rcv_buf()
because the caller took care to provide the correct available space in the
buffer. The control is still there but it is no longer applied on the
request headers. Now, we should take care of the reserve when the headers
are decoded, before the stream creation.
The test is performed for the request and the response.
MEDIUM: htx: Refactor htx_xfer_blks() to not rely on hdrs_bytes field
It is the only function using the hdrs_bytes start-line field. Thus the
function has been refactored to no longer rely on it. To do so, we first
copy HTX blocks to the destination message, without removing them from the
source message. If the copy is interrupted on headers or trailers, we roll
back. Otherwise, data are drained from the source buffer.
Most of time, the copy will succeeds. So the roll back is only performed in
the worst but very rare case.
BUG/MINOR: htx: Preserve HTX flags when draining data from an HTX message
When all data of an HTX message are drained, we rely on htx_reset() to
reinit the message state. However, the flags must be preserved. It is, among
other things, important to preserve processing or parsing errors.
The new global variable cpu_map conflicted with a local variable of the
same name in the code path for the apple platform when setting the
process affinity.
BUG/MAJOR: fix build on musl with cpu_set_t support
Move cpu_map structure outside of the global struct to a global
variable defined in cpuset.c compilation unit. This allows to reorganize
the includes without having to define _GNU_SOURCE everywhere for the
support of the cpu_set_t.
This fixes the compilation with musl libc, most notably used for the
alpine based docker image.
This fixes the github issue #1235.
No need to backport as this feature is new in the current
2.4-dev.
BUG/MINOR: ssl: ssl_sock_prepare_ssl_ctx does not return an error code
The return value check was wrongly based on error codes when the
function actually returns an error number.
This bug was introduced by f3eedfe19592ebcbaa5b97d8c68aa162e7f6f8fa
which is a feature not present before branch 2.4.
REORG: htx: Inline htx functions to add HTX blocks in a message
The HTX functions used to add new HTX blocks in a message have been moved to
the header file to inline them in calling functions. These functions are
small enough.
BUG/MINOR: mux-fcgi: Don't send normalized uri to FCGI application
A normalized URI is the internal term used to specify an URI is stored using
the absolute format (scheme + authority + path). For now, it is only used
for H2 clients. It is the default and recommended format for H2 request.
However, it is unusual for H1 servers to receive such URI. So in this case,
we only send the path of the absolute URI. It is performed for H1 servers,
but not for FCGI applications. This patch fixes the difference.
Note that it is not a real bug, because FCGI applications should support
abosolute URI.
Note also a normalized URI is only detected for H2 clients when a request is
received. There is no such test on the H1 side. It means an absolute URI
received from an H1 client will be sent without modification to an H1 server
or a FCGI application.
To make it possible, a dedicated function has been added to get the H1
URI. This function is called by the H1 and the FCGI multiplexer when a
request is sent to a server.
This patch should fix the issue #1232. It must be backported as far as 2.2.