|
|
887953 |
From d8e094d1bdd2dff5e8f81c0786ca62f6d6dc45ee Mon Sep 17 00:00:00 2001
|
|
|
887953 |
From: Atin Mukherjee <amukherj@redhat.com>
|
|
|
887953 |
Date: Tue, 31 Jul 2018 12:33:49 +0530
|
|
|
887953 |
Subject: [PATCH 365/385] glusterd: ignore importing volume which is undergoing
|
|
|
887953 |
a delete operation
|
|
|
887953 |
|
|
|
887953 |
Problem explanation:
|
|
|
887953 |
|
|
|
887953 |
Assuming in a 3 nodes cluster, if N1 originates a delete operation and
|
|
|
887953 |
while N1's commit phase completes, either glusterd service of N2 or N3
|
|
|
887953 |
gets disconnected from N1 (before completing the commit phase), N1 will
|
|
|
887953 |
attempt to end up importing the volume which is in-flight for a delete
|
|
|
887953 |
in other nodes as a fresh resulting into an incorrect configuration
|
|
|
887953 |
state.
|
|
|
887953 |
|
|
|
887953 |
Fix:
|
|
|
887953 |
|
|
|
887953 |
Mark a volume as stage deleted once a volume delete operation passes
|
|
|
887953 |
it's staging phase and reset this flag during unlock phase. Now during
|
|
|
887953 |
this intermediate phase if the same volume gets imported to other peers,
|
|
|
887953 |
it shouldn't considered to be recreated.
|
|
|
887953 |
|
|
|
887953 |
An automated .t is quite tough to implement with the current infra.
|
|
|
887953 |
|
|
|
887953 |
Test Case:
|
|
|
887953 |
|
|
|
887953 |
1. Keep creating and deleting volumes in a loop on a 3 node cluster
|
|
|
887953 |
2. Simulate n/w failure between the peers (ifdown followed by ifup)
|
|
|
887953 |
3. Check if output of 'gluster v list | wc -l' is same across all 3
|
|
|
887953 |
nodes during 1 & 2.
|
|
|
887953 |
|
|
|
887953 |
>upstream patch : https://review.gluster.org/#/c/glusterfs/+/20592
|
|
|
887953 |
>Change-Id: Ifdd5dc39699120258d7fdd42fe2deb9de25c6246
|
|
|
887953 |
>Fixes: bz#1605077
|
|
|
887953 |
>Signed-off-by: Atin Mukherjee <amukherj@redhat.com>
|
|
|
887953 |
|
|
|
887953 |
Change-Id: Ifdd5dc39699120258d7fdd42fe2deb9de25c6246
|
|
|
887953 |
BUG: 1618221
|
|
|
887953 |
Signed-off-by: Atin Mukherjee <amukherj@redhat.com>
|
|
|
887953 |
Reviewed-on: https://code.engineering.redhat.com/gerrit/149872
|
|
|
887953 |
Tested-by: RHGS Build Bot <nigelb@redhat.com>
|
|
|
887953 |
Reviewed-by: Sunil Kumar Heggodu Gopala Acharya <sheggodu@redhat.com>
|
|
|
887953 |
---
|
|
|
887953 |
xlators/mgmt/glusterd/src/glusterd-locks.c | 13 +++++++++--
|
|
|
887953 |
xlators/mgmt/glusterd/src/glusterd-utils.c | 30 ++++++++++++++++++++++---
|
|
|
887953 |
xlators/mgmt/glusterd/src/glusterd-utils.h | 2 +-
|
|
|
887953 |
xlators/mgmt/glusterd/src/glusterd-volume-ops.c | 2 +-
|
|
|
887953 |
xlators/mgmt/glusterd/src/glusterd.h | 3 +++
|
|
|
887953 |
5 files changed, 43 insertions(+), 7 deletions(-)
|
|
|
887953 |
|
|
|
887953 |
diff --git a/xlators/mgmt/glusterd/src/glusterd-locks.c b/xlators/mgmt/glusterd/src/glusterd-locks.c
|
|
|
887953 |
index 831be20..f4e0225 100644
|
|
|
887953 |
--- a/xlators/mgmt/glusterd/src/glusterd-locks.c
|
|
|
887953 |
+++ b/xlators/mgmt/glusterd/src/glusterd-locks.c
|
|
|
887953 |
@@ -790,6 +790,7 @@ glusterd_mgmt_v3_unlock (const char *name, uuid_t uuid, char *type)
|
|
|
887953 |
int32_t ret = -1;
|
|
|
887953 |
gf_boolean_t is_valid = _gf_true;
|
|
|
887953 |
glusterd_conf_t *priv = NULL;
|
|
|
887953 |
+ glusterd_volinfo_t *volinfo = NULL;
|
|
|
887953 |
glusterd_mgmt_v3_lock_timer *mgmt_lock_timer = NULL;
|
|
|
887953 |
uuid_t owner = {0};
|
|
|
887953 |
xlator_t *this = NULL;
|
|
|
887953 |
@@ -888,8 +889,7 @@ glusterd_mgmt_v3_unlock (const char *name, uuid_t uuid, char *type)
|
|
|
887953 |
"Lock for %s %s successfully released",
|
|
|
887953 |
type, name);
|
|
|
887953 |
|
|
|
887953 |
- ret = 0;
|
|
|
887953 |
- /* Release owner refernce which was held during lock */
|
|
|
887953 |
+ /* Release owner reference which was held during lock */
|
|
|
887953 |
if (mgmt_lock_timer->timer) {
|
|
|
887953 |
ret = -1;
|
|
|
887953 |
mgmt_lock_timer_xl = mgmt_lock_timer->xl;
|
|
|
887953 |
@@ -906,6 +906,15 @@ glusterd_mgmt_v3_unlock (const char *name, uuid_t uuid, char *type)
|
|
|
887953 |
dict_del (priv->mgmt_v3_lock_timer, key_dup);
|
|
|
887953 |
mgmt_lock_timer->timer = NULL;
|
|
|
887953 |
}
|
|
|
887953 |
+ ret = glusterd_volinfo_find (name, &volinfo);
|
|
|
887953 |
+ if (volinfo && volinfo->stage_deleted) {
|
|
|
887953 |
+ /* this indicates a volume still exists and the volume delete
|
|
|
887953 |
+ * operation has failed in some of the phases, need to ensure
|
|
|
887953 |
+ * stage_deleted flag is set back to false
|
|
|
887953 |
+ */
|
|
|
887953 |
+ volinfo->stage_deleted = _gf_false;
|
|
|
887953 |
+ }
|
|
|
887953 |
+ ret = 0;
|
|
|
887953 |
out:
|
|
|
887953 |
|
|
|
887953 |
gf_msg_trace (this->name, 0, "Returning %d", ret);
|
|
|
887953 |
diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.c b/xlators/mgmt/glusterd/src/glusterd-utils.c
|
|
|
887953 |
index 4fd8575..7f52602 100644
|
|
|
887953 |
--- a/xlators/mgmt/glusterd/src/glusterd-utils.c
|
|
|
887953 |
+++ b/xlators/mgmt/glusterd/src/glusterd-utils.c
|
|
|
887953 |
@@ -1699,7 +1699,7 @@ glusterd_volinfo_find_by_volume_id (uuid_t volume_id, glusterd_volinfo_t **volin
|
|
|
887953 |
}
|
|
|
887953 |
|
|
|
887953 |
int32_t
|
|
|
887953 |
-glusterd_volinfo_find (char *volname, glusterd_volinfo_t **volinfo)
|
|
|
887953 |
+glusterd_volinfo_find (const char *volname, glusterd_volinfo_t **volinfo)
|
|
|
887953 |
{
|
|
|
887953 |
glusterd_volinfo_t *tmp_volinfo = NULL;
|
|
|
887953 |
int32_t ret = -1;
|
|
|
887953 |
@@ -2952,6 +2952,11 @@ glusterd_add_volume_to_dict (glusterd_volinfo_t *volinfo,
|
|
|
887953 |
if (ret)
|
|
|
887953 |
goto out;
|
|
|
887953 |
|
|
|
887953 |
+ snprintf (key, sizeof (key), "%s%d.stage_deleted", prefix, count);
|
|
|
887953 |
+ ret = dict_set_uint32 (dict, key, (uint32_t)volinfo->stage_deleted);
|
|
|
887953 |
+ if (ret)
|
|
|
887953 |
+ goto out;
|
|
|
887953 |
+
|
|
|
887953 |
/* tiering related variables */
|
|
|
887953 |
|
|
|
887953 |
memset (key, 0, sizeof (key));
|
|
|
887953 |
@@ -3355,6 +3360,7 @@ glusterd_compare_friend_volume (dict_t *peer_data, int32_t count,
|
|
|
887953 |
uint32_t cksum = 0;
|
|
|
887953 |
uint32_t quota_cksum = 0;
|
|
|
887953 |
uint32_t quota_version = 0;
|
|
|
887953 |
+ uint32_t stage_deleted = 0;
|
|
|
887953 |
int32_t version = 0;
|
|
|
887953 |
xlator_t *this = NULL;
|
|
|
887953 |
|
|
|
887953 |
@@ -3370,9 +3376,15 @@ glusterd_compare_friend_volume (dict_t *peer_data, int32_t count,
|
|
|
887953 |
goto out;
|
|
|
887953 |
|
|
|
887953 |
ret = glusterd_volinfo_find (volname, &volinfo);
|
|
|
887953 |
-
|
|
|
887953 |
if (ret) {
|
|
|
887953 |
- *status = GLUSTERD_VOL_COMP_UPDATE_REQ;
|
|
|
887953 |
+ snprintf (key, sizeof (key), "volume%d.stage_deleted", count);
|
|
|
887953 |
+ ret = dict_get_uint32 (peer_data, key, &stage_deleted);
|
|
|
887953 |
+ /* stage_deleted = 1 means the volume is still in the process of
|
|
|
887953 |
+ * deleting a volume, so we shouldn't be trying to create a
|
|
|
887953 |
+ * fresh volume here which would lead to a stale entry
|
|
|
887953 |
+ */
|
|
|
887953 |
+ if (stage_deleted == 0)
|
|
|
887953 |
+ *status = GLUSTERD_VOL_COMP_UPDATE_REQ;
|
|
|
887953 |
ret = 0;
|
|
|
887953 |
goto out;
|
|
|
887953 |
}
|
|
|
887953 |
@@ -3929,6 +3941,7 @@ glusterd_import_volinfo (dict_t *peer_data, int count,
|
|
|
887953 |
char *rebalance_id_str = NULL;
|
|
|
887953 |
int op_version = 0;
|
|
|
887953 |
int client_op_version = 0;
|
|
|
887953 |
+ uint32_t stage_deleted = 0;
|
|
|
887953 |
|
|
|
887953 |
GF_ASSERT (peer_data);
|
|
|
887953 |
GF_ASSERT (volinfo);
|
|
|
887953 |
@@ -3941,6 +3954,17 @@ glusterd_import_volinfo (dict_t *peer_data, int count,
|
|
|
887953 |
goto out;
|
|
|
887953 |
}
|
|
|
887953 |
|
|
|
887953 |
+ snprintf (key, sizeof (key), "%s%d.stage_deleted", prefix, count);
|
|
|
887953 |
+ ret = dict_get_uint32 (peer_data, key, &stage_deleted);
|
|
|
887953 |
+ /* stage_deleted = 1 means the volume is still in the process of
|
|
|
887953 |
+ * deleting a volume, so we shouldn't be trying to create a
|
|
|
887953 |
+ * fresh volume here which would lead to a stale entry
|
|
|
887953 |
+ */
|
|
|
887953 |
+ if (stage_deleted) {
|
|
|
887953 |
+ ret = 0;
|
|
|
887953 |
+ goto out;
|
|
|
887953 |
+ }
|
|
|
887953 |
+
|
|
|
887953 |
ret = glusterd_volinfo_new (&new_volinfo);
|
|
|
887953 |
if (ret)
|
|
|
887953 |
goto out;
|
|
|
887953 |
diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.h b/xlators/mgmt/glusterd/src/glusterd-utils.h
|
|
|
887953 |
index 4835728..ffcc636 100644
|
|
|
887953 |
--- a/xlators/mgmt/glusterd/src/glusterd-utils.h
|
|
|
887953 |
+++ b/xlators/mgmt/glusterd/src/glusterd-utils.h
|
|
|
887953 |
@@ -159,7 +159,7 @@ glusterd_brickinfo_new_from_brick (char *brick,
|
|
|
887953 |
char **op_errstr);
|
|
|
887953 |
|
|
|
887953 |
int32_t
|
|
|
887953 |
-glusterd_volinfo_find (char *volname, glusterd_volinfo_t **volinfo);
|
|
|
887953 |
+glusterd_volinfo_find (const char *volname, glusterd_volinfo_t **volinfo);
|
|
|
887953 |
|
|
|
887953 |
int
|
|
|
887953 |
glusterd_volinfo_find_by_volume_id (uuid_t volume_id, glusterd_volinfo_t **volinfo);
|
|
|
887953 |
diff --git a/xlators/mgmt/glusterd/src/glusterd-volume-ops.c b/xlators/mgmt/glusterd/src/glusterd-volume-ops.c
|
|
|
887953 |
index 8bb0b6d..94e07cb 100644
|
|
|
887953 |
--- a/xlators/mgmt/glusterd/src/glusterd-volume-ops.c
|
|
|
887953 |
+++ b/xlators/mgmt/glusterd/src/glusterd-volume-ops.c
|
|
|
887953 |
@@ -1828,7 +1828,7 @@ glusterd_op_stage_delete_volume (dict_t *dict, char **op_errstr)
|
|
|
887953 |
snprintf (msg, sizeof(msg), "Some of the peers are down");
|
|
|
887953 |
goto out;
|
|
|
887953 |
}
|
|
|
887953 |
-
|
|
|
887953 |
+ volinfo->stage_deleted = _gf_true;
|
|
|
887953 |
ret = 0;
|
|
|
887953 |
|
|
|
887953 |
out:
|
|
|
887953 |
diff --git a/xlators/mgmt/glusterd/src/glusterd.h b/xlators/mgmt/glusterd/src/glusterd.h
|
|
|
887953 |
index 4ec609f..d4f4f7e 100644
|
|
|
887953 |
--- a/xlators/mgmt/glusterd/src/glusterd.h
|
|
|
887953 |
+++ b/xlators/mgmt/glusterd/src/glusterd.h
|
|
|
887953 |
@@ -475,6 +475,9 @@ struct glusterd_volinfo_ {
|
|
|
887953 |
glusterd_snapdsvc_t snapd;
|
|
|
887953 |
glusterd_tierdsvc_t tierd;
|
|
|
887953 |
int32_t quota_xattr_version;
|
|
|
887953 |
+ gf_boolean_t stage_deleted; /* volume has passed staging
|
|
|
887953 |
+ * for delete operation
|
|
|
887953 |
+ */
|
|
|
887953 |
};
|
|
|
887953 |
|
|
|
887953 |
typedef enum gd_snap_status_ {
|
|
|
887953 |
--
|
|
|
887953 |
1.8.3.1
|
|
|
887953 |
|