glusterd: ignore importing volume which is undergoing a delete operation

Problem explanation:

Assuming in a 3 nodes cluster, if N1 originates a delete operation and
while N1's commit phase completes, either glusterd service of N2 or N3
gets disconnected from N1 (before completing the commit phase), N1 will
attempt to end up importing the volume which is in-flight for a delete
in other nodes as a fresh resulting into an incorrect configuration
state.

Fix:

Mark a volume as stage deleted once a volume delete operation passes
it's staging phase and reset this flag during unlock phase. Now during
this intermediate phase if the same volume gets imported to other peers,
it shouldn't considered to be recreated.

An automated .t is quite tough to implement with the current infra.

Test Case:

1. Keep creating and deleting volumes in a loop on a 3 node cluster
2. Simulate n/w failure between the peers (ifdown followed by ifup)
3. Check if output of 'gluster v list | wc -l' is same across all 3
nodes during 1 & 2.

Change-Id: Ifdd5dc39699120258d7fdd42fe2deb9de25c6246
Fixes: bz#1605077
Signed-off-by: Atin Mukherjee <amukherj@redhat.com>

5 files changed, 42 insertions, 6 deletions

diff --git a/xlators/mgmt/glusterd/src/glusterd-locks.c b/xlators/mgmt/glusterd/src/glusterd-locks.c
index cc9c03f5683..1e99117b3a1 100644
--- a/xlators/mgmt/glusterd/src/glusterd-locks.c
+++ b/xlators/mgmt/glusterd/src/glusterd-locks.c
@@ -790,6 +790,7 @@ glusterd_mgmt_v3_unlock (const char *name, uuid_t uuid, char *type)
         int32_t                         ret                 = -1;
         gf_boolean_t                    is_valid            = _gf_true;
         glusterd_conf_t                 *priv               = NULL;
+        glusterd_volinfo_t              *volinfo            = NULL;
         glusterd_mgmt_v3_lock_timer     *mgmt_lock_timer    = NULL;
         uuid_t                          owner               = {0};
         xlator_t                        *this               = NULL;
@@ -888,7 +889,6 @@ glusterd_mgmt_v3_unlock (const char *name, uuid_t uuid, char *type)
                 "Lock for %s %s successfully released",
                 type, name);
 
-        ret = 0;
         /* Release owner reference which was held during lock */
         if (mgmt_lock_timer->timer) {
                 ret = -1;
@@ -905,6 +905,15 @@ glusterd_mgmt_v3_unlock (const char *name, uuid_t uuid, char *type)
                                     mgmt_lock_timer->timer);
                 dict_del (priv->mgmt_v3_lock_timer, key_dup);
         }
+        ret = glusterd_volinfo_find (name, &volinfo);
+        if (volinfo && volinfo->stage_deleted) {
+                /* this indicates a volume still exists and the volume delete
+                 * operation has failed in some of the phases, need to ensure
+                 * stage_deleted flag is set back to false
+                 */
+                volinfo->stage_deleted = _gf_false;
+        }
+        ret = 0;
 out:
 
         gf_msg_trace (this->name, 0, "Returning %d", ret);
diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.c b/xlators/mgmt/glusterd/src/glusterd-utils.c
index 5afeebac78b..362d3b6adc4 100644
--- a/xlators/mgmt/glusterd/src/glusterd-utils.c
+++ b/xlators/mgmt/glusterd/src/glusterd-utils.c
@@ -1718,7 +1718,7 @@ glusterd_volinfo_find_by_volume_id (uuid_t volume_id, glusterd_volinfo_t **volin
 }
 
 int32_t
-glusterd_volinfo_find (char *volname, glusterd_volinfo_t **volinfo)
+glusterd_volinfo_find (const char *volname, glusterd_volinfo_t **volinfo)
 {
         glusterd_volinfo_t      *tmp_volinfo = NULL;
         int32_t                 ret = -1;
@@ -3016,6 +3016,11 @@ glusterd_add_volume_to_dict (glusterd_volinfo_t *volinfo,
         if (ret)
                 goto out;
 
+        snprintf (key, sizeof (key), "%s%d.stage_deleted", prefix, count);
+        ret = dict_set_uint32 (dict, key, (uint32_t)volinfo->stage_deleted);
+        if (ret)
+                goto out;
+
          /* tiering related variables */
 
         snprintf (key, sizeof (key), "%s%d.cold_brick_count", prefix, count);
@@ -3390,6 +3395,7 @@ glusterd_compare_friend_volume (dict_t *peer_data, int32_t count,
         uint32_t                cksum = 0;
         uint32_t                quota_cksum = 0;
         uint32_t                quota_version = 0;
+        uint32_t                stage_deleted = 0;
         int32_t                 version = 0;
         xlator_t                *this = NULL;
 
@@ -3405,9 +3411,15 @@ glusterd_compare_friend_volume (dict_t *peer_data, int32_t count,
                 goto out;
 
         ret = glusterd_volinfo_find (volname, &volinfo);
-
         if (ret) {
-                *status = GLUSTERD_VOL_COMP_UPDATE_REQ;
+                snprintf (key, sizeof (key), "volume%d.stage_deleted", count);
+                ret = dict_get_uint32 (peer_data, key, &stage_deleted);
+                /* stage_deleted = 1 means the volume is still in the process of
+                 * deleting a volume, so we shouldn't be trying to create a
+                 * fresh volume here which would lead to a stale entry
+                 */
+                if (stage_deleted == 0)
+                        *status = GLUSTERD_VOL_COMP_UPDATE_REQ;
                 ret = 0;
                 goto out;
         }
@@ -3949,6 +3961,7 @@ glusterd_import_volinfo (dict_t *peer_data, int count,
         char               *rebalance_id_str = NULL;
         int                op_version        = 0;
         int                client_op_version = 0;
+        uint32_t           stage_deleted     = 0;
 
         GF_ASSERT (peer_data);
         GF_ASSERT (volinfo);
@@ -3961,6 +3974,17 @@ glusterd_import_volinfo (dict_t *peer_data, int count,
                 goto out;
         }
 
+        snprintf (key, sizeof (key), "%s%d.stage_deleted", prefix, count);
+        ret = dict_get_uint32 (peer_data, key, &stage_deleted);
+        /* stage_deleted = 1 means the volume is still in the process of
+         * deleting a volume, so we shouldn't be trying to create a
+         * fresh volume here which would lead to a stale entry
+        */
+        if (stage_deleted) {
+                ret = 0;
+                goto out;
+        }
+
         ret = glusterd_volinfo_new (&new_volinfo);
         if (ret)
                 goto out;
diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.h b/xlators/mgmt/glusterd/src/glusterd-utils.h
index eded8e231d4..97f3cc6edf0 100644
--- a/xlators/mgmt/glusterd/src/glusterd-utils.h
+++ b/xlators/mgmt/glusterd/src/glusterd-utils.h
@@ -158,7 +158,7 @@ glusterd_brickinfo_new_from_brick (char *brick,
                                    char **op_errstr);
 
 int32_t
-glusterd_volinfo_find (char *volname, glusterd_volinfo_t **volinfo);
+glusterd_volinfo_find (const char *volname, glusterd_volinfo_t **volinfo);
 
 int
 glusterd_volinfo_find_by_volume_id (uuid_t volume_id, glusterd_volinfo_t **volinfo);
diff --git a/xlators/mgmt/glusterd/src/glusterd-volume-ops.c b/xlators/mgmt/glusterd/src/glusterd-volume-ops.c
index 4c1f0be3d06..70a871fcef1 100644
--- a/xlators/mgmt/glusterd/src/glusterd-volume-ops.c
+++ b/xlators/mgmt/glusterd/src/glusterd-volume-ops.c
@@ -1851,7 +1851,7 @@ glusterd_op_stage_delete_volume (dict_t *dict, char **op_errstr)
                 snprintf (msg, sizeof(msg), "Some of the peers are down");
                 goto out;
         }
-
+        volinfo->stage_deleted = _gf_true;
         ret = 0;
 
 out:
diff --git a/xlators/mgmt/glusterd/src/glusterd.h b/xlators/mgmt/glusterd/src/glusterd.h
index 49a1dc6dc08..e0864c04148 100644
--- a/xlators/mgmt/glusterd/src/glusterd.h
+++ b/xlators/mgmt/glusterd/src/glusterd.h
@@ -489,6 +489,9 @@ struct glusterd_volinfo_ {
         glusterd_tierdsvc_t       tierd;
         glusterd_gfproxydsvc_t    gfproxyd;
         int32_t                   quota_xattr_version;
+        gf_boolean_t              stage_deleted; /* volume has passed staging
+                                                  * for delete operation
+                                                  */
 };
 
 typedef enum gd_snap_status_ {