From c209e7656bc3eb80b210924a0f756eebd0befc60 Mon Sep 17 00:00:00 2001
From: Ravishankar N <ravishankar@redhat.com>
Date: Thu, 5 Jul 2018 14:02:34 +0530
Subject: [PATCH 683/685] glusterd: fix client io-threads option for replicate
volumes
Backport of https://review.gluster.org/#/c/18430/5
...but changes the op_version checks to GD_OP_VERSION_3_11_2
This backport is for honouring 'gluster v set <volname> client-io-threads`
{on/off} on rhgs-3.3.1 async release. After upgrading from an older version
of rhgs to 331 or when upgrading from this version to rhgs-3.4.0, it is
recommended to explicity run the volume set after upgrading all nodes and
bumping up the cluster op-version. As an additional check, please also check
the fuse volfile to see if the io-threads xlator was loaded/unloaded depending
on whether it you did an 'on' or 'off' respectively.
Change-Id: I47d5717bf137b01eea88678cca8624c3aabd8bb5
BUG: 1598416
Signed-off-by: Ravishankar N <ravishankar@redhat.com>
Reviewed-on: https://code.engineering.redhat.com/gerrit/143250
Reviewed-by: Atin Mukherjee <amukherj@redhat.com>
Tested-by: RHGS Build Bot <nigelb@redhat.com>
---
.../replicate/bug-1498570-client-iot-graph-check.t | 49 ++++++++++++++++++++++
xlators/mgmt/glusterd/src/glusterd-brick-ops.c | 34 +++++++++++++++
xlators/mgmt/glusterd/src/glusterd-handler.c | 7 ++--
xlators/mgmt/glusterd/src/glusterd-utils.c | 20 +--------
xlators/mgmt/glusterd/src/glusterd-utils.h | 3 +-
xlators/mgmt/glusterd/src/glusterd-volgen.c | 28 ++++++++-----
xlators/mgmt/glusterd/src/glusterd-volume-ops.c | 34 +++++++++++++++
7 files changed, 141 insertions(+), 34 deletions(-)
create mode 100644 tests/bugs/replicate/bug-1498570-client-iot-graph-check.t
diff --git a/tests/bugs/replicate/bug-1498570-client-iot-graph-check.t b/tests/bugs/replicate/bug-1498570-client-iot-graph-check.t
new file mode 100644
index 0000000..4574ccb
--- /dev/null
+++ b/tests/bugs/replicate/bug-1498570-client-iot-graph-check.t
@@ -0,0 +1,49 @@
+#!/bin/bash
+. $(dirname $0)/../../include.rc
+. $(dirname $0)/../../volume.rc
+. $(dirname $0)/../../afr.rc
+
+TESTS_EXPECTED_IN_LOOP=21
+function reset_cluster
+{
+ cleanup
+ TEST glusterd
+ TEST pidof glusterd
+
+}
+function check_iot_option
+{
+ local enabled=$1
+ local is_loaded_in_graph=$2
+
+ EXPECT "$enabled" volume_get_field $V0 client-io-threads
+ IOT_STRING="volume\ $V0-io-threads"
+ grep "$IOT_STRING" $GLUSTERD_WORKDIR/vols/$V0/trusted-$V0.tcp-fuse.vol
+ TEST ret=$?
+ EXPECT_NOT "$is_loaded_in_graph" echo $ret
+}
+
+reset_cluster
+TEST $CLI volume create $V0 $H0:$B0/${V0}{0,1}
+check_iot_option on 1
+
+reset_cluster
+TEST $CLI volume create $V0 replica 2 $H0:$B0/${V0}{0,1}
+check_iot_option off 0
+
+reset_cluster
+TEST $CLI volume create $V0 $H0:$B0/${V0}0
+TEST $CLI volume start $V0
+TEST $CLI volume add-brick $V0 replica 2 $H0:$B0/${V0}1
+check_iot_option off 0
+TEST $CLI volume remove-brick $V0 replica 1 $H0:$B0/${V0}1 force
+check_iot_option on 1
+
+reset_cluster
+TEST $CLI volume create $V0 replica 3 $H0:$B0/${V0}{0..5}
+TEST $CLI volume set $V0 client-io-threads on
+check_iot_option on 1
+TEST $CLI volume remove-brick $V0 replica 2 $H0:$B0/${V0}2 $H0:$B0/${V0}5 force
+check_iot_option on 1
+
+cleanup
diff --git a/xlators/mgmt/glusterd/src/glusterd-brick-ops.c b/xlators/mgmt/glusterd/src/glusterd-brick-ops.c
index f4cd927..fadbc00 100644
--- a/xlators/mgmt/glusterd/src/glusterd-brick-ops.c
+++ b/xlators/mgmt/glusterd/src/glusterd-brick-ops.c
@@ -1415,6 +1415,24 @@ glusterd_op_perform_add_bricks (glusterd_volinfo_t *volinfo, int32_t count,
/* Gets changed only if the options are given in add-brick cli */
if (type)
volinfo->type = type;
+ /* performance.client-io-threads is turned on by default,
+ * however this has adverse effects on replicate volumes due to
+ * replication design issues, till that get addressed
+ * performance.client-io-threads option is turned off for all
+ * replicate volumes if not already explicitly enabled.
+ */
+ if (type && glusterd_is_volume_replicate (volinfo) &&
+ conf->op_version >= GD_OP_VERSION_3_11_2) {
+ ret = dict_set_str (volinfo->dict,
+ "performance.client-io-threads",
+ "off");
+ if (ret) {
+ gf_msg (this->name, GF_LOG_ERROR, 0,
+ GD_MSG_DICT_SET_FAILED, "Failed to set "
+ "performance.client-io-threads to off");
+ goto out;
+ }
+ }
if (replica_count) {
volinfo->replica_count = replica_count;
@@ -2583,9 +2601,12 @@ glusterd_op_remove_brick (dict_t *dict, char **op_errstr)
char *cold_shd_key = NULL;
char *hot_shd_key = NULL;
int delete_key = 1;
+ glusterd_conf_t *conf = NULL;
this = THIS;
GF_ASSERT (this);
+ conf = this->private;
+ GF_VALIDATE_OR_GOTO (this->name, conf, out);
ret = dict_get_str (dict, "volname", &volname);
@@ -2875,6 +2896,19 @@ glusterd_op_remove_brick (dict_t *dict, char **op_errstr)
volinfo->subvol_count = (volinfo->brick_count /
volinfo->dist_leaf_count);
+ if (!glusterd_is_volume_replicate (volinfo) &&
+ conf->op_version >= GD_OP_VERSION_3_11_2) {
+ ret = dict_set_str (volinfo->dict,
+ "performance.client-io-threads",
+ "on");
+ if (ret) {
+ gf_msg (this->name, GF_LOG_ERROR, 0,
+ GD_MSG_DICT_SET_FAILED, "Failed to set "
+ "performance.client-io-threads to on");
+ goto out;
+ }
+ }
+
ret = glusterd_create_volfiles_and_notify_services (volinfo);
if (ret) {
gf_msg (this->name, GF_LOG_WARNING, 0,
diff --git a/xlators/mgmt/glusterd/src/glusterd-handler.c b/xlators/mgmt/glusterd/src/glusterd-handler.c
index 6d66301..6bcfc6b 100644
--- a/xlators/mgmt/glusterd/src/glusterd-handler.c
+++ b/xlators/mgmt/glusterd/src/glusterd-handler.c
@@ -4886,7 +4886,7 @@ glusterd_get_volume_opts (rpcsvc_request_t *req, dict_t *dict)
(dict,
_gf_false,
key, orig_key,
- volinfo,
+ volinfo->dict,
&rsp.op_errstr);
if (ret && !rsp.op_errstr) {
snprintf (err_str,
@@ -4912,7 +4912,7 @@ glusterd_get_volume_opts (rpcsvc_request_t *req, dict_t *dict)
} else {
/* Handle the "all" volume option request */
ret = glusterd_get_default_val_for_volopt (dict, _gf_true, NULL,
- NULL, volinfo,
+ NULL, volinfo->dict,
&rsp.op_errstr);
if (ret && !rsp.op_errstr) {
snprintf (err_str, sizeof(err_str),
@@ -5505,7 +5505,8 @@ glusterd_get_state (rpcsvc_request_t *req, dict_t *dict)
vol_all_opts = dict_new ();
ret = glusterd_get_default_val_for_volopt (vol_all_opts,
- _gf_true, NULL, NULL, volinfo, &rsp.op_errstr);
+ _gf_true, NULL, NULL, volinfo->dict,
+ &rsp.op_errstr);
if (ret) {
gf_msg (this->name, GF_LOG_ERROR, 0,
GD_MSG_VOL_OPTS_IMPORT_FAIL, "Failed to "
diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.c b/xlators/mgmt/glusterd/src/glusterd-utils.c
index a04ed99..f219fd5 100644
--- a/xlators/mgmt/glusterd/src/glusterd-utils.c
+++ b/xlators/mgmt/glusterd/src/glusterd-utils.c
@@ -12368,8 +12368,7 @@ out:
int
glusterd_get_default_val_for_volopt (dict_t *ctx, gf_boolean_t all_opts,
char *input_key, char *orig_key,
- glusterd_volinfo_t *volinfo,
- char **op_errstr)
+ dict_t *vol_dict, char **op_errstr)
{
struct volopt_map_entry *vme = NULL;
int ret = -1;
@@ -12380,7 +12379,6 @@ glusterd_get_default_val_for_volopt (dict_t *ctx, gf_boolean_t all_opts,
char dict_key[50] = {0,};
gf_boolean_t key_found = _gf_false;
glusterd_conf_t *priv = NULL;
- dict_t *vol_dict = NULL;
this = THIS;
GF_ASSERT (this);
@@ -12388,7 +12386,6 @@ glusterd_get_default_val_for_volopt (dict_t *ctx, gf_boolean_t all_opts,
priv = this->private;
GF_VALIDATE_OR_GOTO (this->name, priv, out);
- vol_dict = volinfo->dict;
GF_VALIDATE_OR_GOTO (this->name, vol_dict, out);
/* Check whether key is passed for a single option */
@@ -12410,20 +12407,6 @@ glusterd_get_default_val_for_volopt (dict_t *ctx, gf_boolean_t all_opts,
if (!def_val) {
ret = dict_get_str (vol_dict, vme->key, &def_val);
if (!def_val) {
- /* For replicate volumes
- * performance.client-io-threads will be set to
- * off by default until explicitly turned on
- */
- if (!strcmp (vme->key,
- "performance.client-io-threads")) {
- if (volinfo->type ==
- GF_CLUSTER_TYPE_REPLICATE ||
- volinfo->type ==
- GF_CLUSTER_TYPE_STRIPE_REPLICATE) {
- def_val = "off";
- goto set_count;
- }
- }
if (vme->value) {
def_val = vme->value;
} else {
@@ -12436,7 +12419,6 @@ glusterd_get_default_val_for_volopt (dict_t *ctx, gf_boolean_t all_opts,
}
}
}
-set_count:
count++;
sprintf (dict_key, "key%d", count);
ret = dict_set_str(ctx, dict_key, vme->key);
diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.h b/xlators/mgmt/glusterd/src/glusterd-utils.h
index 7a5bfd9..259088b 100644
--- a/xlators/mgmt/glusterd/src/glusterd-utils.h
+++ b/xlators/mgmt/glusterd/src/glusterd-utils.h
@@ -695,8 +695,7 @@ glusterd_get_global_options_for_all_vols (dict_t *dict, char **op_errstr);
int
glusterd_get_default_val_for_volopt (dict_t *dict, gf_boolean_t all_opts,
char *key, char *orig_key,
- glusterd_volinfo_t *volinfo,
- char **err_str);
+ dict_t *vol_dict, char **err_str);
int
glusterd_check_client_op_version_support (char *volname, uint32_t op_version,
diff --git a/xlators/mgmt/glusterd/src/glusterd-volgen.c b/xlators/mgmt/glusterd/src/glusterd-volgen.c
index 4198be8..e22c3d2 100644
--- a/xlators/mgmt/glusterd/src/glusterd-volgen.c
+++ b/xlators/mgmt/glusterd/src/glusterd-volgen.c
@@ -2550,9 +2550,15 @@ perfxl_option_handler (volgen_graph_t *graph, struct volopt_map_entry *vme,
{
gf_boolean_t enabled = _gf_false;
glusterd_volinfo_t *volinfo = NULL;
+ xlator_t *this = NULL;
+ glusterd_conf_t *priv = NULL;
- GF_ASSERT (param);
+ GF_VALIDATE_OR_GOTO ("glusterd", param, out);
volinfo = param;
+ this = THIS;
+ GF_VALIDATE_OR_GOTO ("glusterd", this, out);
+ priv = this->private;
+ GF_VALIDATE_OR_GOTO ("glusterd", priv, out);
if (strcmp (vme->option, "!perf") != 0)
return 0;
@@ -2568,13 +2574,15 @@ perfxl_option_handler (volgen_graph_t *graph, struct volopt_map_entry *vme,
(vme->op_version > volinfo->client_op_version))
return 0;
- /* For replicate volumes do not load io-threads as it affects
- * performance
- */
- if (!strcmp (vme->key, "performance.client-io-threads") &&
- (GF_CLUSTER_TYPE_STRIPE_REPLICATE == volinfo->type ||
- GF_CLUSTER_TYPE_REPLICATE == volinfo->type))
- return 0;
+ if (priv->op_version < GD_OP_VERSION_3_11_2) {
+ /* For replicate volumes do not load io-threads as it affects
+ * performance
+ */
+ if (!strcmp (vme->key, "performance.client-io-threads") &&
+ (GF_CLUSTER_TYPE_STRIPE_REPLICATE == volinfo->type ||
+ GF_CLUSTER_TYPE_REPLICATE == volinfo->type))
+ return 0;
+ }
/* if VKEY_READDIR_AHEAD is enabled and parallel readdir is
* not enabled then load readdir-ahead here else it will be
@@ -2585,8 +2593,8 @@ perfxl_option_handler (volgen_graph_t *graph, struct volopt_map_entry *vme,
if (volgen_graph_add (graph, vme->voltype, volinfo->volname))
return 0;
- else
- return -1;
+out:
+ return -1;
}
static int
diff --git a/xlators/mgmt/glusterd/src/glusterd-volume-ops.c b/xlators/mgmt/glusterd/src/glusterd-volume-ops.c
index 4e410ce..f552c83 100644
--- a/xlators/mgmt/glusterd/src/glusterd-volume-ops.c
+++ b/xlators/mgmt/glusterd/src/glusterd-volume-ops.c
@@ -2236,6 +2236,23 @@ glusterd_op_create_volume (dict_t *dict, char **op_errstr)
volinfo->stripe_count = 1;
if (GF_CLUSTER_TYPE_REPLICATE == volinfo->type) {
+ /* performance.client-io-threads is turned on to default,
+ * however this has adverse effects on replicate volumes due to
+ * replication design issues, till that get addressed
+ * performance.client-io-threads option is turned off for all
+ * replicate volumes
+ */
+ if (priv->op_version >= GD_OP_VERSION_3_11_2) {
+ ret = dict_set_str (volinfo->dict,
+ "performance.client-io-threads",
+ "off");
+ if (ret) {
+ gf_msg (this->name, GF_LOG_ERROR, 0,
+ GD_MSG_DICT_SET_FAILED, "Failed to set "
+ "performance.client-io-threads to off");
+ goto out;
+ }
+ }
ret = dict_get_int32 (dict, "replica-count",
&volinfo->replica_count);
if (ret) {
@@ -2256,6 +2273,23 @@ glusterd_op_create_volume (dict_t *dict, char **op_errstr)
goto out;
}
} else if (GF_CLUSTER_TYPE_STRIPE_REPLICATE == volinfo->type) {
+ /* performance.client-io-threads is turned on to default,
+ * however this has adverse effects on replicate volumes due to
+ * replication design issues, till that get addressed
+ * performance.client-io-threads option is turned off for all
+ * replicate volumes
+ */
+ if (priv->op_version >= GD_OP_VERSION_3_11_2) {
+ ret = dict_set_str (volinfo->dict,
+ "performance.client-io-threads",
+ "off");
+ if (ret) {
+ gf_msg (this->name, GF_LOG_ERROR, 0,
+ GD_MSG_DICT_SET_FAILED, "Failed to set "
+ "performance.client-io-threads to off");
+ goto out;
+ }
+ }
ret = dict_get_int32 (dict, "stripe-count",
&volinfo->stripe_count);
if (ret) {
--
1.8.3.1