path: root/0149-glusterd-Optimize-code-to-copy-dictionary-in-handsha.patch

From 86eee7e829bb33cac9b611da511ecbd2f03fab25 Mon Sep 17 00:00:00 2001
From: Mohit Agrawal <moagrawal@redhat.com>
Date: Fri, 17 May 2019 19:26:48 +0530
Subject: [PATCH 149/169] glusterd: Optimize code to copy dictionary in
 handshake code path

Problem: While high no. of volumes are configured around 2000
         glusterd has bottleneck during handshake at the time
         of copying dictionary

Solution: To avoid the bottleneck serialize a dictionary instead
          of copying key-value pair one by one

> Change-Id: I9fb332f432e4f915bc3af8dcab38bed26bda2b9a
> fixes: bz#1711297
> Cherry picked from commit f8f09178bb890924a8050b466cc2e7a0a30e35a7
> (Reviewed on upstream link https://review.gluster.org/#/c/glusterfs/+/22742/)

BUG: 1711296
Change-Id: I9fb332f432e4f915bc3af8dcab38bed26bda2b9a
Signed-off-by: Mohit Agrawal <moagrawal@redhat.com>
Reviewed-on: https://code.engineering.redhat.com/gerrit/172255
Reviewed-by: Atin Mukherjee <amukherj@redhat.com>
Tested-by: RHGS Build Bot <nigelb@redhat.com>
---
 libglusterfs/src/dict.c                      |   6 +-
 libglusterfs/src/glusterfs/dict.h            |   6 +
 libglusterfs/src/libglusterfs.sym            |   1 +
 xlators/mgmt/glusterd/src/glusterd-rpc-ops.c |  27 ++--
 xlators/mgmt/glusterd/src/glusterd-utils.c   | 187 +++++++++++++++++++++++----
 xlators/mgmt/glusterd/src/glusterd-utils.h   |   3 +-
 xlators/mgmt/glusterd/src/glusterd.h         |   5 +
 7 files changed, 194 insertions(+), 41 deletions(-)

diff --git a/libglusterfs/src/dict.c b/libglusterfs/src/dict.c
index 4cd1fcf..6917df9 100644
--- a/libglusterfs/src/dict.c
+++ b/libglusterfs/src/dict.c
@@ -2799,10 +2799,6 @@ dict_rename_key(dict_t *this, char *key, char *replace_key)
  *     4        4         4       <key len>   <value len>
  */
 
-#define DICT_HDR_LEN 4
-#define DICT_DATA_HDR_KEY_LEN 4
-#define DICT_DATA_HDR_VAL_LEN 4
-
 /**
  * dict_serialized_length_lk - return the length of serialized dict. This
  *                             procedure has to be called with this->lock held.
@@ -2812,7 +2808,7 @@ dict_rename_key(dict_t *this, char *key, char *replace_key)
  *        : failure: -errno
  */
 
-static int
+int
 dict_serialized_length_lk(dict_t *this)
 {
     int ret = -EINVAL;
diff --git a/libglusterfs/src/glusterfs/dict.h b/libglusterfs/src/glusterfs/dict.h
index 52b833f..022f564 100644
--- a/libglusterfs/src/glusterfs/dict.h
+++ b/libglusterfs/src/glusterfs/dict.h
@@ -91,6 +91,9 @@ typedef struct _data_pair data_pair_t;
 #define DICT_MAX_FLAGS 256
 #define DICT_FLAG_SET 1
 #define DICT_FLAG_CLEAR 0
+#define DICT_HDR_LEN 4
+#define DICT_DATA_HDR_KEY_LEN 4
+#define DICT_DATA_HDR_VAL_LEN 4
 
 struct _data {
     char *data;
@@ -412,4 +415,7 @@ are_dicts_equal(dict_t *one, dict_t *two,
                 gf_boolean_t (*value_ignore)(char *k));
 int
 dict_has_key_from_array(dict_t *dict, char **strings, gf_boolean_t *result);
+
+int
+dict_serialized_length_lk(dict_t *this);
 #endif
diff --git a/libglusterfs/src/libglusterfs.sym b/libglusterfs/src/libglusterfs.sym
index cf5757c..ec474e7 100644
--- a/libglusterfs/src/libglusterfs.sym
+++ b/libglusterfs/src/libglusterfs.sym
@@ -405,6 +405,7 @@ dict_rename_key
 dict_reset
 dict_serialize
 dict_serialized_length
+dict_serialized_length_lk
 dict_serialize_value_with_delim
 dict_set
 dict_setn
diff --git a/xlators/mgmt/glusterd/src/glusterd-rpc-ops.c b/xlators/mgmt/glusterd/src/glusterd-rpc-ops.c
index 4ec9700..45f8f17 100644
--- a/xlators/mgmt/glusterd/src/glusterd-rpc-ops.c
+++ b/xlators/mgmt/glusterd/src/glusterd-rpc-ops.c
@@ -1528,11 +1528,9 @@ glusterd_rpc_friend_add(call_frame_t *frame, xlator_t *this, void *data)
 
     RCU_READ_UNLOCK;
 
-    ret = glusterd_add_volumes_to_export_dict(&peer_data);
-    if (ret) {
-        gf_msg(this->name, GF_LOG_ERROR, 0, GD_MSG_DICT_SET_FAILED,
-               "Unable to add list of volumes "
-               "in the peer_data dict for handshake");
+    peer_data = dict_new();
+    if (!peer_data) {
+        errno = ENOMEM;
         goto out;
     }
 
@@ -1563,10 +1561,23 @@ glusterd_rpc_friend_add(call_frame_t *frame, xlator_t *this, void *data)
         }
     }
 
-    ret = dict_allocate_and_serialize(peer_data, &req.vols.vols_val,
-                                      &req.vols.vols_len);
-    if (ret)
+    /* Don't add any key-value in peer_data dictionary after call this function
+     */
+    ret = glusterd_add_volumes_to_export_dict(peer_data, &req.vols.vols_val,
+                                              &req.vols.vols_len);
+    if (ret) {
+        gf_msg(this->name, GF_LOG_ERROR, 0, GD_MSG_DICT_SET_FAILED,
+               "Unable to add list of volumes "
+               "in the peer_data dict for handshake");
         goto out;
+    }
+
+    if (!req.vols.vols_len) {
+        ret = dict_allocate_and_serialize(peer_data, &req.vols.vols_val,
+                                          &req.vols.vols_len);
+        if (ret)
+            goto out;
+    }
 
     ret = glusterd_submit_request(
         peerinfo->rpc, &req, frame, peerinfo->peer, GLUSTERD_FRIEND_ADD, NULL,
diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.c b/xlators/mgmt/glusterd/src/glusterd-utils.c
index 8f1525e..2bc4836 100644
--- a/xlators/mgmt/glusterd/src/glusterd-utils.c
+++ b/xlators/mgmt/glusterd/src/glusterd-utils.c
@@ -3466,11 +3466,118 @@ out:
     return NULL;
 }
 
+int
+glusterd_dict_searialize(dict_t *dict_arr[], int count, int totcount, char *buf)
+{
+    int i = 0;
+    int32_t keylen = 0;
+    int64_t netword = 0;
+    data_pair_t *pair = NULL;
+    int dict_count = 0;
+    int ret = 0;
+
+    netword = hton32(totcount);
+    memcpy(buf, &netword, sizeof(netword));
+    buf += DICT_HDR_LEN;
+
+    for (i = 0; i < count; i++) {
+        if (dict_arr[i]) {
+            dict_count = dict_arr[i]->count;
+            pair = dict_arr[i]->members_list;
+            while (dict_count) {
+                if (!pair) {
+                    gf_msg("glusterd", GF_LOG_ERROR, 0,
+                           LG_MSG_PAIRS_LESS_THAN_COUNT,
+                           "less than count data pairs found!");
+                    ret = -1;
+                    goto out;
+                }
+
+                if (!pair->key) {
+                    gf_msg("glusterd", GF_LOG_ERROR, 0, LG_MSG_NULL_PTR,
+                           "pair->key is null!");
+                    ret = -1;
+                    goto out;
+                }
+
+                keylen = strlen(pair->key);
+                netword = hton32(keylen);
+                memcpy(buf, &netword, sizeof(netword));
+                buf += DICT_DATA_HDR_KEY_LEN;
+                if (!pair->value) {
+                    gf_msg("glusterd", GF_LOG_ERROR, 0, LG_MSG_NULL_PTR,
+                           "pair->value is null!");
+                    ret = -1;
+                    goto out;
+                }
+
+                netword = hton32(pair->value->len);
+                memcpy(buf, &netword, sizeof(netword));
+                buf += DICT_DATA_HDR_VAL_LEN;
+
+                memcpy(buf, pair->key, keylen);
+                buf += keylen;
+                *buf++ = '\0';
+
+                if (pair->value->data) {
+                    memcpy(buf, pair->value->data, pair->value->len);
+                    buf += pair->value->len;
+                }
+
+                pair = pair->next;
+                dict_count--;
+            }
+        }
+    }
+
+out:
+    for (i = 0; i < count; i++) {
+        if (dict_arr[i])
+            dict_unref(dict_arr[i]);
+    }
+    return ret;
+}
+
+int
+glusterd_dict_arr_serialize(dict_t *dict_arr[], int count, char **buf,
+                            u_int *length)
+{
+    ssize_t len = 0;
+    int i = 0;
+    int totcount = 0;
+    int ret = 0;
+
+    for (i = 0; i < count; i++) {
+        if (dict_arr[i]) {
+            len += dict_serialized_length_lk(dict_arr[i]);
+            totcount += dict_arr[i]->count;
+        }
+    }
+
+    // Subtract HDR_LEN except one dictionary
+    len = len - ((count - 1) * DICT_HDR_LEN);
+
+    *buf = GF_MALLOC(len, gf_common_mt_char);
+    if (*buf == NULL) {
+        ret = -ENOMEM;
+        goto out;
+    }
+
+    if (length != NULL) {
+        *length = len;
+    }
+
+    ret = glusterd_dict_searialize(dict_arr, count, totcount, *buf);
+
+out:
+    return ret;
+}
+
 int32_t
-glusterd_add_volumes_to_export_dict(dict_t **peer_data)
+glusterd_add_volumes_to_export_dict(dict_t *peer_data, char **buf,
+                                    u_int *length)
 {
     int32_t ret = -1;
-    dict_t *dict = NULL;
     dict_t *dict_arr[128] = {
         0,
     };
@@ -3496,10 +3603,6 @@ glusterd_add_volumes_to_export_dict(dict_t **peer_data)
     priv = this->private;
     GF_ASSERT(priv);
 
-    dict = dict_new();
-    if (!dict)
-        goto out;
-
     /* Count the total number of volumes */
     cds_list_for_each_entry(volinfo, &priv->volumes, vol_list) volcnt++;
 
@@ -3520,14 +3623,15 @@ glusterd_add_volumes_to_export_dict(dict_t **peer_data)
         cds_list_for_each_entry(volinfo, &priv->volumes, vol_list)
         {
             count++;
-            ret = glusterd_add_volume_to_dict(volinfo, dict, count, "volume");
+            ret = glusterd_add_volume_to_dict(volinfo, peer_data, count,
+                                              "volume");
             if (ret)
                 goto out;
 
             if (!dict_get_sizen(volinfo->dict, VKEY_FEATURES_QUOTA))
                 continue;
 
-            ret = glusterd_vol_add_quota_conf_to_dict(volinfo, dict, count,
+            ret = glusterd_vol_add_quota_conf_to_dict(volinfo, peer_data, count,
                                                       "volume");
             if (ret)
                 goto out;
@@ -3569,34 +3673,34 @@ glusterd_add_volumes_to_export_dict(dict_t **peer_data)
 
         gf_log(this->name, GF_LOG_INFO,
                "Finished dictionary popluation in all threads");
-        for (i = 0; i < totthread; i++) {
-            dict_copy_with_ref(dict_arr[i], dict);
-            dict_unref(dict_arr[i]);
-        }
-        gf_log(this->name, GF_LOG_INFO,
-               "Finished merger of all dictionraies into single one");
     }
 
-    ret = dict_set_int32n(dict, "count", SLEN("count"), volcnt);
+    ret = dict_set_int32n(peer_data, "count", SLEN("count"), volcnt);
     if (ret)
         goto out;
 
-    ctx.dict = dict;
+    ctx.dict = peer_data;
     ctx.prefix = "global";
     ctx.opt_count = 1;
     ctx.key_name = "key";
     ctx.val_name = "val";
     dict_foreach(priv->opts, _add_dict_to_prdict, &ctx);
     ctx.opt_count--;
-    ret = dict_set_int32n(dict, "global-opt-count", SLEN("global-opt-count"),
-                          ctx.opt_count);
+    ret = dict_set_int32n(peer_data, "global-opt-count",
+                          SLEN("global-opt-count"), ctx.opt_count);
     if (ret)
         goto out;
 
-    *peer_data = dict;
+    if (totthread) {
+        gf_log(this->name, GF_LOG_INFO,
+               "Finished merger of all dictionraies into single one");
+        dict_arr[totthread++] = peer_data;
+        ret = glusterd_dict_arr_serialize(dict_arr, totthread, buf, length);
+        gf_log(this->name, GF_LOG_INFO,
+               "Serialize dictionary data return is %d", ret);
+    }
+
 out:
-    if (ret)
-        dict_unref(dict);
 
     gf_msg_trace(this->name, 0, "Returning %d", ret);
     return ret;
@@ -4940,6 +5044,7 @@ glusterd_import_friend_volumes_synctask(void *opaque)
     xlator_t *this = NULL;
     glusterd_conf_t *conf = NULL;
     dict_t *peer_data = NULL;
+    glusterd_friend_synctask_args_t *arg = NULL;
 
     this = THIS;
     GF_ASSERT(this);
@@ -4947,8 +5052,20 @@ glusterd_import_friend_volumes_synctask(void *opaque)
     conf = this->private;
     GF_ASSERT(conf);
 
-    peer_data = (dict_t *)opaque;
-    GF_ASSERT(peer_data);
+    arg = opaque;
+    if (!arg)
+        goto out;
+
+    peer_data = dict_new();
+    if (!peer_data) {
+        goto out;
+    }
+
+    ret = dict_unserialize(arg->dict_buf, arg->dictlen, &peer_data);
+    if (ret) {
+        errno = ENOMEM;
+        goto out;
+    }
 
     ret = dict_get_int32n(peer_data, "count", SLEN("count"), &count);
     if (ret)
@@ -4980,6 +5097,11 @@ glusterd_import_friend_volumes_synctask(void *opaque)
 out:
     if (peer_data)
         dict_unref(peer_data);
+    if (arg) {
+        if (arg->dict_buf)
+            GF_FREE(arg->dict_buf);
+        GF_FREE(arg);
+    }
 
     gf_msg_debug("glusterd", 0, "Returning with %d", ret);
     return ret;
@@ -5146,7 +5268,7 @@ glusterd_compare_friend_data(dict_t *peer_data, int32_t *status, char *hostname)
     gf_boolean_t update = _gf_false;
     xlator_t *this = NULL;
     glusterd_conf_t *priv = NULL;
-    dict_t *peer_data_copy = NULL;
+    glusterd_friend_synctask_args_t *arg = NULL;
 
     this = THIS;
     GF_ASSERT(this);
@@ -5188,12 +5310,23 @@ glusterd_compare_friend_data(dict_t *peer_data, int32_t *status, char *hostname)
          * first brick to come up before attaching the subsequent bricks
          * in case brick multiplexing is enabled
          */
-        peer_data_copy = dict_copy_with_ref(peer_data, NULL);
-        glusterd_launch_synctask(glusterd_import_friend_volumes_synctask,
-                                 peer_data_copy);
+        arg = GF_CALLOC(1, sizeof(*arg), gf_common_mt_char);
+        ret = dict_allocate_and_serialize(peer_data, &arg->dict_buf,
+                                          &arg->dictlen);
+        if (ret < 0) {
+            gf_log(this->name, GF_LOG_ERROR,
+                   "dict_serialize failed while handling "
+                   " import friend volume request");
+            goto out;
+        }
+
+        glusterd_launch_synctask(glusterd_import_friend_volumes_synctask, arg);
     }
 
 out:
+    if (ret && arg) {
+        GF_FREE(arg);
+    }
     gf_msg_debug(this->name, 0, "Returning with ret: %d, status: %d", ret,
                  *status);
     return ret;
diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.h b/xlators/mgmt/glusterd/src/glusterd-utils.h
index 3647c34..6ad8062 100644
--- a/xlators/mgmt/glusterd/src/glusterd-utils.h
+++ b/xlators/mgmt/glusterd/src/glusterd-utils.h
@@ -227,7 +227,8 @@ glusterd_volume_brickinfo_get_by_brick(char *brick, glusterd_volinfo_t *volinfo,
                                        gf_boolean_t construct_real_path);
 
 int32_t
-glusterd_add_volumes_to_export_dict(dict_t **peer_data);
+glusterd_add_volumes_to_export_dict(dict_t *peer_data, char **buf,
+                                    u_int *length);
 
 int32_t
 glusterd_compare_friend_data(dict_t *peer_data, int32_t *status,
diff --git a/xlators/mgmt/glusterd/src/glusterd.h b/xlators/mgmt/glusterd/src/glusterd.h
index 2ea8560..f96bca3 100644
--- a/xlators/mgmt/glusterd/src/glusterd.h
+++ b/xlators/mgmt/glusterd/src/glusterd.h
@@ -240,6 +240,11 @@ typedef struct glusterd_add_dict_args {
     int end;
 } glusterd_add_dict_args_t;
 
+typedef struct glusterd_friend_synctask_args {
+    char *dict_buf;
+    u_int dictlen;
+} glusterd_friend_synctask_args_t;
+
 typedef enum gf_brick_status {
     GF_BRICK_STOPPED,
     GF_BRICK_STARTED,
-- 
1.8.3.1