From 854ab79dbef449c39adf66e3faebb4681359fce4 Mon Sep 17 00:00:00 2001
From: mohit84 <moagrawa@redhat.com>
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 <moagrawa@redhat.com>
>Change-Id: Ia284015d79beaa3d703ebabb92f26870a5aaafba
Upstream Patch : https://github.com/gluster/glusterfs/pull/2172
BUG: 1832306
Change-Id: Ia284015d79beaa3d703ebabb92f26870a5aaafba
Signed-off-by: srijan-sivakumar <ssivakum@redhat.com>
Reviewed-on: https://code.engineering.redhat.com/gerrit/228249
Tested-by: RHGS Build Bot <nigelb@redhat.com>
Reviewed-by: Sunil Kumar Heggodu Gopala Acharya <sheggodu@redhat.com>
---
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