From 854ab79dbef449c39adf66e3faebb4681359fce4 Mon Sep 17 00:00:00 2001 From: mohit84 Date: Thu, 18 Feb 2021 09:40:44 +0530 Subject: [PATCH 533/538] glusterd: Rebalance cli is not showing correct status after reboot (#2172) Rebalance cli is not showing correct status after reboot. The CLI is not correct status because defrag object is not valid at the time of creating a rpc connection to show the status. The defrag object is not valid because at the time of start a glusterd glusterd_restart_rebalance can be call almost at the same time by two different synctask and glusterd got a disconnect on rpc object and it cleanup the defrag object. Solution: To avoid the defrag object populate a reference count before create a defrag rpc object. >Fixes: #1339 >Signed-off-by: Mohit Agrawal >Change-Id: Ia284015d79beaa3d703ebabb92f26870a5aaafba Upstream Patch : https://github.com/gluster/glusterfs/pull/2172 BUG: 1832306 Change-Id: Ia284015d79beaa3d703ebabb92f26870a5aaafba Signed-off-by: srijan-sivakumar Reviewed-on: https://code.engineering.redhat.com/gerrit/228249 Tested-by: RHGS Build Bot Reviewed-by: Sunil Kumar Heggodu Gopala Acharya --- xlators/mgmt/glusterd/src/glusterd-rebalance.c | 35 ++++++++++----- xlators/mgmt/glusterd/src/glusterd-syncop.c | 1 + xlators/mgmt/glusterd/src/glusterd-utils.c | 59 +++++++++++++++++++++++++- xlators/mgmt/glusterd/src/glusterd-utils.h | 5 +++ xlators/mgmt/glusterd/src/glusterd.h | 1 + 5 files changed, 90 insertions(+), 11 deletions(-) diff --git a/xlators/mgmt/glusterd/src/glusterd-rebalance.c b/xlators/mgmt/glusterd/src/glusterd-rebalance.c index b419a89..fcd5318 100644 --- a/xlators/mgmt/glusterd/src/glusterd-rebalance.c +++ b/xlators/mgmt/glusterd/src/glusterd-rebalance.c @@ -86,6 +86,7 @@ __glusterd_defrag_notify(struct rpc_clnt *rpc, void *mydata, glusterd_conf_t *priv = NULL; xlator_t *this = NULL; int pid = -1; + int refcnt = 0; this = THIS; if (!this) @@ -125,11 +126,12 @@ __glusterd_defrag_notify(struct rpc_clnt *rpc, void *mydata, } case RPC_CLNT_DISCONNECT: { - if (!defrag->connected) - return 0; - LOCK(&defrag->lock); { + if (!defrag->connected) { + UNLOCK(&defrag->lock); + return 0; + } defrag->connected = 0; } UNLOCK(&defrag->lock); @@ -146,11 +148,11 @@ __glusterd_defrag_notify(struct rpc_clnt *rpc, void *mydata, glusterd_defrag_rpc_put(defrag); if (defrag->cbk_fn) defrag->cbk_fn(volinfo, volinfo->rebal.defrag_status); - - GF_FREE(defrag); + refcnt = glusterd_defrag_unref(defrag); gf_msg(this->name, GF_LOG_INFO, 0, GD_MSG_REBALANCE_DISCONNECTED, - "Rebalance process for volume %s has disconnected.", - volinfo->volname); + "Rebalance process for volume %s has disconnected" + " and defrag refcnt is %d.", + volinfo->volname, refcnt); break; } case RPC_CLNT_DESTROY: @@ -309,7 +311,11 @@ glusterd_handle_defrag_start(glusterd_volinfo_t *volinfo, char *op_errstr, gf_msg_debug("glusterd", 0, "rebalance command failed"); goto out; } - + /* Take reference before sleep to save defrag object cleanup while + glusterd_restart_rebalance call for other bricks by syncktask + at the time of restart a glusterd. + */ + glusterd_defrag_ref(defrag); sleep(5); ret = glusterd_rebalance_rpc_create(volinfo); @@ -372,6 +378,7 @@ glusterd_rebalance_rpc_create(glusterd_volinfo_t *volinfo) GF_ASSERT(this); priv = this->private; GF_ASSERT(priv); + struct rpc_clnt *rpc = NULL; // rebalance process is not started if (!defrag) @@ -396,13 +403,21 @@ glusterd_rebalance_rpc_create(glusterd_volinfo_t *volinfo) } glusterd_volinfo_ref(volinfo); - ret = glusterd_rpc_create(&defrag->rpc, options, glusterd_defrag_notify, - volinfo, _gf_true); + ret = glusterd_rpc_create(&rpc, options, glusterd_defrag_notify, volinfo, + _gf_false); if (ret) { gf_msg(THIS->name, GF_LOG_ERROR, 0, GD_MSG_RPC_CREATE_FAIL, "Glusterd RPC creation failed"); goto out; } + LOCK(&defrag->lock); + { + if (!defrag->rpc) + defrag->rpc = rpc; + else + rpc_clnt_unref(rpc); + } + UNLOCK(&defrag->lock); ret = 0; out: if (options) diff --git a/xlators/mgmt/glusterd/src/glusterd-syncop.c b/xlators/mgmt/glusterd/src/glusterd-syncop.c index df78fef..05c9e11 100644 --- a/xlators/mgmt/glusterd/src/glusterd-syncop.c +++ b/xlators/mgmt/glusterd/src/glusterd-syncop.c @@ -1732,6 +1732,7 @@ gd_brick_op_phase(glusterd_op_t op, dict_t *op_ctx, dict_t *req_dict, if (!rpc) { if (pending_node->type == GD_NODE_REBALANCE && pending_node->node) { volinfo = pending_node->node; + glusterd_defrag_ref(volinfo->rebal.defrag); ret = glusterd_rebalance_rpc_create(volinfo); if (ret) { ret = 0; diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.c b/xlators/mgmt/glusterd/src/glusterd-utils.c index bc188a2..9fb8eab 100644 --- a/xlators/mgmt/glusterd/src/glusterd-utils.c +++ b/xlators/mgmt/glusterd/src/glusterd-utils.c @@ -93,6 +93,44 @@ #define NLMV4_VERSION 4 #define NLMV1_VERSION 1 +int +glusterd_defrag_ref(glusterd_defrag_info_t *defrag) +{ + int refcnt = 0; + + if (!defrag) + goto out; + + LOCK(&defrag->lock); + { + refcnt = ++defrag->refcnt; + } + UNLOCK(&defrag->lock); + +out: + return refcnt; +} + +int +glusterd_defrag_unref(glusterd_defrag_info_t *defrag) +{ + int refcnt = -1; + + if (!defrag) + goto out; + + LOCK(&defrag->lock); + { + refcnt = --defrag->refcnt; + if (refcnt <= 0) + GF_FREE(defrag); + } + UNLOCK(&defrag->lock); + +out: + return refcnt; +} + gf_boolean_t is_brick_mx_enabled(void) { @@ -9370,6 +9408,7 @@ glusterd_volume_defrag_restart(glusterd_volinfo_t *volinfo, char *op_errstr, char pidfile[PATH_MAX] = ""; int ret = -1; pid_t pid = 0; + int refcnt = 0; this = THIS; GF_ASSERT(this); @@ -9410,7 +9449,25 @@ glusterd_volume_defrag_restart(glusterd_volinfo_t *volinfo, char *op_errstr, volinfo->volname); goto out; } - ret = glusterd_rebalance_rpc_create(volinfo); + refcnt = glusterd_defrag_ref(volinfo->rebal.defrag); + /* If refcnt value is 1 it means either defrag object is + poulated by glusterd_rebalance_defrag_init or previous + rpc creation was failed.If it is not 1 it means it(defrag) + was populated at the time of start a rebalance daemon. + We need to create a rpc object only while a previous + rpc connection was not established successfully at the + time of restart a rebalance daemon by + glusterd_handle_defrag_start otherwise rebalance cli + does not show correct status after just reboot a node and try + to print the rebalance status because defrag object has been + destroyed during handling of rpc disconnect. + */ + if (refcnt == 1) { + ret = glusterd_rebalance_rpc_create(volinfo); + } else { + ret = 0; + glusterd_defrag_unref(volinfo->rebal.defrag); + } break; } case GF_DEFRAG_STATUS_NOT_STARTED: diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.h b/xlators/mgmt/glusterd/src/glusterd-utils.h index 02d85d2..4541471 100644 --- a/xlators/mgmt/glusterd/src/glusterd-utils.h +++ b/xlators/mgmt/glusterd/src/glusterd-utils.h @@ -886,4 +886,9 @@ int32_t glusterd_check_brick_order(dict_t *dict, char *err_str, int32_t type, int32_t sub_count); +int +glusterd_defrag_ref(glusterd_defrag_info_t *defrag); + +int +glusterd_defrag_unref(glusterd_defrag_info_t *defrag); #endif diff --git a/xlators/mgmt/glusterd/src/glusterd.h b/xlators/mgmt/glusterd/src/glusterd.h index efe4d0e..9de3f28 100644 --- a/xlators/mgmt/glusterd/src/glusterd.h +++ b/xlators/mgmt/glusterd/src/glusterd.h @@ -321,6 +321,7 @@ struct glusterd_defrag_info_ { uint64_t total_data; uint64_t num_files_lookedup; uint64_t total_failures; + int refcnt; gf_lock_t lock; int cmd; pthread_t th; -- 1.8.3.1