MEDIUM: stick-table: use a distinct lock for the updates tree

Updating an entry in the updates tree is currently performed under the
table's write lock, which causes huge contention with other accesses
such as lookups and free. Aside the updates tree, the update,
localupdate and commitupdate variables, nothing is manipulated, so
let's create a distinct lock (updt_lock) to protect these together
to remove this contention. It required to add an extra lock in the
few places where we delete the update (though only if we're really
going to delete it) to protect the tree. This is very convenient
because now peer_send_teachmsgs() only needs to take this read lock,
and there is very little contention left on the stick-table.

With this alone, the performance jumped from 614k to 1140k/s on a
80-thread machine with a peers section! Stick-table updates with
no peers however now has to stand two locks and slightly regressed
from 4.0-4.1M/s to 3.9-4.0. This is fairly minimal compared to the
significant unlocking of the peers updates and considered totally
acceptable.

diff --git a/include/haproxy/stick_table-t.h b/include/haproxy/stick_table-t.h
index c915edce6c8d3983f570d91b71fe4ae43af5e01f..132422980593f7b59ee1b5dcb1ef1a89c3a1e535 100644 (file)
--- a/include/haproxy/stick_table-t.h
+++ b/include/haproxy/stick_table-t.h
@@ -208,6 +208,7 @@ struct stktable {
        unsigned int update;      /* uses updt_lock */
        unsigned int localupdate; /* uses updt_lock */
        unsigned int commitupdate;/* used to identify the latest local updates pending for sync, uses updt_lock */
+       __decl_thread(HA_RWLOCK_T updt_lock); /* lock protecting the updates part */
 };
 
 extern struct stktable_data_type stktable_data_types[STKTABLE_DATA_TYPES];

diff --git a/src/peers.c b/src/peers.c
index 616b6e4f8bef4db230ba119fc4568dbb9edc1589..011c1ed897ac63cdca0fd6f701e15fd769bb5913 100644 (file)
--- a/src/peers.c
+++ b/src/peers.c
@@ -1576,9 +1576,9 @@ static inline int peer_send_teachmsgs(struct appctx *appctx, struct peer *p,
        new_pushed = 1;
 
        if (locked)
-               HA_RWLOCK_WRTORD(STK_TABLE_LOCK, &st->table->lock);
-       else
-               HA_RWLOCK_RDLOCK(STK_TABLE_LOCK, &st->table->lock);
+               HA_RWLOCK_WRUNLOCK(STK_TABLE_LOCK, &st->table->lock);
+
+       HA_RWLOCK_RDLOCK(STK_TABLE_LOCK, &st->table->updt_lock);
 
        while (1) {
                struct stksess *ts;
@@ -1600,10 +1600,10 @@ static inline int peer_send_teachmsgs(struct appctx *appctx, struct peer *p,
                }
 
                HA_ATOMIC_INC(&ts->ref_cnt);
-               HA_RWLOCK_RDUNLOCK(STK_TABLE_LOCK, &st->table->lock);
+               HA_RWLOCK_RDUNLOCK(STK_TABLE_LOCK, &st->table->updt_lock);
 
                ret = peer_send_updatemsg(st, appctx, ts, updateid, new_pushed, use_timed);
-               HA_RWLOCK_RDLOCK(STK_TABLE_LOCK, &st->table->lock);
+               HA_RWLOCK_RDLOCK(STK_TABLE_LOCK, &st->table->updt_lock);
                HA_ATOMIC_DEC(&ts->ref_cnt);
                if (ret <= 0)
                        break;
@@ -1635,7 +1635,7 @@ static inline int peer_send_teachmsgs(struct appctx *appctx, struct peer *p,
        }
 
  out:
-       HA_RWLOCK_RDUNLOCK(STK_TABLE_LOCK, &st->table->lock);
+       HA_RWLOCK_RDUNLOCK(STK_TABLE_LOCK, &st->table->updt_lock);
 
        if (locked)
                HA_RWLOCK_WRLOCK(STK_TABLE_LOCK, &st->table->lock);

diff --git a/src/stick_table.c b/src/stick_table.c
index 6ba26ffa756d5cfac568cb36ecefe37b604ed77f..1e04e7b3721e105de9072872bdb3c01168f521e7 100644 (file)
--- a/src/stick_table.c
+++ b/src/stick_table.c
@@ -119,7 +119,11 @@ int __stksess_kill(struct stktable *t, struct stksess *ts)
                return 0;
 
        eb32_delete(&ts->exp);
-       eb32_delete(&ts->upd);
+       if (ts->upd.node.leaf_p) {
+               HA_RWLOCK_WRLOCK(STK_TABLE_LOCK, &t->updt_lock);
+               eb32_delete(&ts->upd);
+               HA_RWLOCK_WRUNLOCK(STK_TABLE_LOCK, &t->updt_lock);
+       }
        ebmb_delete(&ts->key);
        __stksess_free(t, ts);
        return 1;
@@ -278,7 +282,11 @@ int __stktable_trash_oldest(struct stktable *t, int to_batch)
 
                /* session expired, trash it */
                ebmb_delete(&ts->key);
-               eb32_delete(&ts->upd);
+               if (ts->upd.node.leaf_p) {
+                       HA_RWLOCK_WRLOCK(STK_TABLE_LOCK, &t->updt_lock);
+                       eb32_delete(&ts->upd);
+                       HA_RWLOCK_WRUNLOCK(STK_TABLE_LOCK, &t->updt_lock);
+               }
                __stksess_free(t, ts);
                batched++;
        }
@@ -441,7 +449,7 @@ void stktable_touch_with_exp(struct stktable *t, struct stksess *ts, int local,
                         * or not scheduled for at least one peer.
                         */
                        if (!locked++)
-                               HA_RWLOCK_WRLOCK(STK_TABLE_LOCK, &t->lock);
+                               HA_RWLOCK_WRLOCK(STK_TABLE_LOCK, &t->updt_lock);
 
                        if (!ts->upd.node.leaf_p
                            || (int)(t->commitupdate - ts->upd.key) >= 0
@@ -460,7 +468,7 @@ void stktable_touch_with_exp(struct stktable *t, struct stksess *ts, int local,
                else {
                        /* If this entry is not in the tree */
                        if (!locked++)
-                               HA_RWLOCK_WRLOCK(STK_TABLE_LOCK, &t->lock);
+                               HA_RWLOCK_WRLOCK(STK_TABLE_LOCK, &t->updt_lock);
 
                        if (!ts->upd.node.leaf_p) {
                                ts->upd.key= (++t->update)+(2147483648U);
@@ -474,7 +482,7 @@ void stktable_touch_with_exp(struct stktable *t, struct stksess *ts, int local,
        }
 
        if (locked)
-               HA_RWLOCK_WRUNLOCK(STK_TABLE_LOCK, &t->lock);
+               HA_RWLOCK_WRUNLOCK(STK_TABLE_LOCK, &t->updt_lock);
 
        if (decrefcnt)
                HA_ATOMIC_DEC(&ts->ref_cnt);
@@ -664,6 +672,7 @@ struct task *process_table_expire(struct task *task, void *context, unsigned int
        struct stktable *t = context;
        struct stksess *ts;
        struct eb32_node *eb;
+       int updt_locked = 0;
        int looped = 0;
        int exp_next;
 
@@ -726,7 +735,13 @@ struct task *process_table_expire(struct task *task, void *context, unsigned int
 
                /* session expired, trash it */
                ebmb_delete(&ts->key);
-               eb32_delete(&ts->upd);
+               if (ts->upd.node.leaf_p) {
+                       if (!updt_locked) {
+                               updt_locked = 1;
+                               HA_RWLOCK_WRLOCK(STK_TABLE_LOCK, &t->updt_lock);
+                       }
+                       eb32_delete(&ts->upd);
+               }
                __stksess_free(t, ts);
        }
 
@@ -735,6 +750,8 @@ struct task *process_table_expire(struct task *task, void *context, unsigned int
 
 out_unlock:
        task->expire = exp_next;
+       if (updt_locked)
+               HA_RWLOCK_WRUNLOCK(STK_TABLE_LOCK, &t->updt_lock);
        HA_RWLOCK_WRUNLOCK(STK_TABLE_LOCK, &t->lock);
        return task;
 }