From c209e7656bc3eb80b210924a0f756eebd0befc60 Mon Sep 17 00:00:00 2001 From: Ravishankar N 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 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 Reviewed-on: https://code.engineering.redhat.com/gerrit/143250 Reviewed-by: Atin Mukherjee Tested-by: RHGS Build Bot --- .../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