BUG/MEDIUM: peers: re-work refcnt on table to protect against flush
In proxy.c, when process is stopping we try to flush tables content
using 'stktable_trash_oldest'. A check on a counter "table->syncing" was
made to verify if there is no pending resync in progress.
But using multiple threads this counter can be increased by an other thread
only after some delay, so the content of some tables can be trashed earlier and
won't be pushed to the new process (after reload, some tables appear reset and
others don't).
This patch re-names the counter "table->syncing" to "table->refcnt" and
the counter is increased during configuration parsing (registering a table to
a peer section) to protect tables during runtime and until resync of a new
process has succeeded or failed.
The inc/dec operations are now made using atomic operations
because multiple peer sections could refer to the same table in futur.
This fix addresses github #1216.
This patch should be backported on all branches multi-thread support (v >= 1.8)
(cherry picked from commit 2cc201f97e409f926b9221fdafe7431877ba1dc6)
Signed-off-by: Emeric Brun <ebrun@haproxy.com>
(cherry picked from commit 167b783bce9c4cd7ca99f09f3a2202ddc72dec4f)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit b8c1aa044a41c57ee46943e2a93c239a4d94db20)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
diff --git a/src/peers.c b/src/peers.c
index e748e81..df9452e 100644
--- a/src/peers.c
+++ b/src/peers.c
@@ -2794,9 +2794,6 @@
/* add DO NOT STOP flag if not present */
_HA_ATOMIC_ADD(&jobs, 1);
peers->flags |= PEERS_F_DONOTSTOP;
- ps = peers->local;
- for (st = ps->tables; st ; st = st->next)
- st->table->syncing++;
/* disconnect all connected peers to process a local sync
* this must be done only the first time we are switching
@@ -2822,7 +2819,7 @@
_HA_ATOMIC_SUB(&jobs, 1);
peers->flags &= ~PEERS_F_DONOTSTOP;
for (st = ps->tables; st ; st = st->next)
- st->table->syncing--;
+ _HA_ATOMIC_SUB(&st->table->refcnt, 1);
}
}
else if (!ps->appctx) {
@@ -2848,7 +2845,7 @@
_HA_ATOMIC_SUB(&jobs, 1);
peers->flags &= ~PEERS_F_DONOTSTOP;
for (st = ps->tables; st ; st = st->next)
- st->table->syncing--;
+ _HA_ATOMIC_SUB(&st->table->refcnt, 1);
}
}
}
@@ -3079,6 +3076,13 @@
id = curpeer->tables->local_id;
st->local_id = id + 1;
+ /* If peer is local we inc table
+ * refcnt to protect against flush
+ * until this process pushed all
+ * table content to the new one
+ */
+ if (curpeer->local)
+ _HA_ATOMIC_ADD(&st->table->refcnt, 1);
curpeer->tables = st;
}
@@ -3252,8 +3256,8 @@
st->last_acked, st->last_pushed, st->last_get,
st->teaching_origin, st->update);
chunk_appendf(&trash, "\n table:%p id=%s update=%u localupdate=%u"
- " commitupdate=%u syncing=%u",
- t, t->id, t->update, t->localupdate, t->commitupdate, t->syncing);
+ " commitupdate=%u refcnt=%u",
+ t, t->id, t->update, t->localupdate, t->commitupdate, t->refcnt);
chunk_appendf(&trash, "\n TX dictionary cache:");
count = 0;
for (i = 0; i < dcache->max_entries; i++) {
diff --git a/src/proxy.c b/src/proxy.c
index 672a49f..d0fc168 100644
--- a/src/proxy.c
+++ b/src/proxy.c
@@ -1011,7 +1011,12 @@
* However we protect tables that are being synced to peers.
*/
if (unlikely(stopping && p->state == PR_STSTOPPED && p->table && p->table->current)) {
- if (!p->table->syncing) {
+ if (!p->table->refcnt) {
+ /* !table->refcnt means there
+ * is no more pending full resync
+ * to push to a new process and
+ * we are free to flush the table.
+ */
stktable_trash_oldest(p->table, p->table->current);
pool_gc(NULL);
}