MEDIUM: stick-table: never learn the "conn_cur" value from peers

There have been a large number of issues reported with conn_cur
synchronization because the concept is wrong. In an active-passive
setup, pushing the local connections count from the active node to
the passive one will result in the passive node to have a higher
counter than the real number of connections. Due to this, after a
switchover, it will never be able to close enough connections to
go down to zero. The same commonly happens on reloads since the new
process preloads its values from the old process, and if no connection
happens for a key after the value is learned, it is impossible to reset
the previous ones. In active-active setups it's a bit different, as the
number of connections reflects the number on the peer that pushed last.

This patch solves this by marking the "conn_cur" local and preventing
it from being learned from peers. It is still pushed, however, so that
any monitoring system that collects values from the peers will still
see it.

The patch is tiny and trivially backportable. While a change of behavior
in stable branches is never welcome, it remains possible to fix issues
if reports become frequent.

diff --git a/doc/configuration.txt b/doc/configuration.txt
index d4ce56657c3fabd190f73815d2670240b077d85b..bd404ed32e46149d267a8ab55e4df76af201efa4 100644 (file)
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -3013,12 +3013,21 @@ user <username> [password|insecure-password <password>]
 It is possible to propagate entries of any data-types in stick-tables between
 several HAProxy instances over TCP connections in a multi-master fashion. Each
 instance pushes its local updates and insertions to remote peers. The pushed
-values overwrite remote ones without aggregation. Interrupted exchanges are
-automatically detected and recovered from the last known point.
-In addition, during a soft restart, the old process connects to the new one
-using such a TCP connection to push all its entries before the new process
-tries to connect to other peers. That ensures very fast replication during a
-reload, it typically takes a fraction of a second even for large tables.
+values overwrite remote ones without aggregation. As an exception, the data
+type "conn_cur" is never learned from peers, as it is supposed to reflect local
+values. Earlier versions used to synchronize it and to cause negative values in
+active-active setups, and always-growing values upon reloads or active-passive
+switches because the local value would reflect more connections than locally
+present. This information, however, is pushed so that monitoring systems can
+watch it.
+
+Interrupted exchanges are automatically detected and recovered from the last
+known point. In addition, during a soft restart, the old process connects to
+the new one using such a TCP connection to push all its entries before the new
+process tries to connect to other peers. That ensures very fast replication
+during a reload, it typically takes a fraction of a second even for large
+tables.
+
 Note that Server IDs are used to identify servers remotely, so it is important
 that configurations look similar or at least that the same IDs are forced on
 each server on all participants.

diff --git a/include/haproxy/stick_table-t.h b/include/haproxy/stick_table-t.h
index d8527b0d297b117657e4b0dffb1c2d2b9b6aa030..3b1f2b3ef901278b3c13bb7b8a688226eb57b5bc 100644 (file)
--- a/include/haproxy/stick_table-t.h
+++ b/include/haproxy/stick_table-t.h
@@ -125,7 +125,8 @@ struct stktable_data_type {
        const char *name; /* name of the data type */
        int std_type;     /* standard type we can use for this data, STD_T_* */
        int arg_type;     /* type of optional argument, ARG_T_* */
-       int is_array;
+       int is_array:1;   /* this is an array of gpc/gpt */
+       int is_local:1;   /* this is local only and never learned */
 };
 
 /* stick table keyword type */

diff --git a/src/peers.c b/src/peers.c
index 5a382f863a709af36ecca02577a0367eb76bc253..09aacca32cb172e6fc798b40e51e7599a901ec6e 100644 (file)
--- a/src/peers.c
+++ b/src/peers.c
@@ -1778,6 +1778,10 @@ static int peer_treat_updatemsg(struct appctx *appctx, struct peer *p, int updt,
 
                if (!((1ULL << data_type) & st->remote_data))
                        continue;
+
+               if (stktable_data_types[data_type].is_local)
+                       continue;
+
                if (stktable_data_types[data_type].is_array) {
                        /* in case of array all elements
                         * use the same std_type and they

diff --git a/src/stick_table.c b/src/stick_table.c
index 4bc1a217dfd1c3f10c006eb11ce69c6f552642c9..4936677072ba4f4b5998a485543662b48ca6cfad 100644 (file)
--- a/src/stick_table.c
+++ b/src/stick_table.c
@@ -1145,7 +1145,7 @@ struct stktable_data_type stktable_data_types[STKTABLE_DATA_TYPES] = {
        [STKTABLE_DT_GPC0_RATE]     = { .name = "gpc0_rate",      .std_type = STD_T_FRQP, .arg_type = ARG_T_DELAY  },
        [STKTABLE_DT_CONN_CNT]      = { .name = "conn_cnt",       .std_type = STD_T_UINT  },
        [STKTABLE_DT_CONN_RATE]     = { .name = "conn_rate",      .std_type = STD_T_FRQP, .arg_type = ARG_T_DELAY  },
-       [STKTABLE_DT_CONN_CUR]      = { .name = "conn_cur",       .std_type = STD_T_UINT  },
+       [STKTABLE_DT_CONN_CUR]      = { .name = "conn_cur",       .std_type = STD_T_UINT, .is_local = 1 },
        [STKTABLE_DT_SESS_CNT]      = { .name = "sess_cnt",       .std_type = STD_T_UINT  },
        [STKTABLE_DT_SESS_RATE]     = { .name = "sess_rate",      .std_type = STD_T_FRQP, .arg_type = ARG_T_DELAY  },
        [STKTABLE_DT_HTTP_REQ_CNT]  = { .name = "http_req_cnt",   .std_type = STD_T_UINT  },