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