BUG/MEDIUM: stick-tables: don't leave the expire loop with elements deleted

In 3.2, the table expiration latency was improved by commit 994cc58576
("MEDIUM: stick-tables: Limit the number of entries we expire"), however
it introduced an issue by which it's possible to leave the loop after a
certain number of elements were expired, without requeuing the deleted
elements. The issue it causes is that other places with a non-null ref_cnt
will not necessarily delete it themselves, resulting in orphan elements in
the table. These ones will then pollute it and force recycling old ones
more often which in turn results in an increase of the contention.

Let's check for the expiration counter before deleting the element so
that it can be found upon next visit.

This fix must be backported to 3.2. It is directly related to GH
issue #3084. Thanks to Felipe and Ricardo for sharing precious info
and testing a candidate fix.

diff --git a/src/stick_table.c b/src/stick_table.c
index cac2de202e47121718e2a1358d4d3a71f8166983..7d516c8a5210766e56b72f92f41717c7890bdb70 100644 (file)
--- a/src/stick_table.c
+++ b/src/stick_table.c
@@ -943,6 +943,15 @@ struct task *process_table_expire(struct task *task, void *context, unsigned int
                        if (HA_ATOMIC_LOAD(&ts->ref_cnt) != 0)
                                continue;
 
+                       if (updt_locked == 1) {
+                               expired++;
+                               if (expired == STKTABLE_MAX_UPDATES_AT_ONCE) {
+                                       need_resched = 1;
+                                       exp_next = TICK_ETERNITY;
+                                       goto out_unlock;
+                               }
+                       }
+
                        eb32_delete(&ts->exp);
 
                        if (!tick_is_expired(ts->expire, now_ms)) {
@@ -968,14 +977,6 @@ struct task *process_table_expire(struct task *task, void *context, unsigned int
                                continue;
                        }
 
-                       if (updt_locked == 1) {
-                               expired++;
-                               if (expired == STKTABLE_MAX_UPDATES_AT_ONCE) {
-                                       need_resched = 1;
-                                       exp_next = TICK_ETERNITY;
-                                       goto out_unlock;
-                               }
-                       }
                        /* if the entry is in the update list, we must be extremely careful
                         * because peers can see it at any moment and start to use it. Peers
                         * will take the table's updt_lock for reading when doing that, and