cluster/afr: Fix locking issues in entry self-heal

Original reporter of the bug & designer of the solution:
        Pranith Kumar K <pkarampu@redhat.com>

Change-Id: I9ed89aa92e4cd0f8049f5f6c7a3701e52989ae5e
BUG: 1128721
Signed-off-by: Krutika Dhananjay <kdhananj@redhat.com>
Reviewed-on: http://review.gluster.org/8837
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Pranith Kumar Karampuri <pkarampu@redhat.com>
Tested-by: Pranith Kumar Karampuri <pkarampu@redhat.com>

1 files changed, 123 insertions, 92 deletions

diff --git a/xlators/cluster/afr/src/afr-self-heal-entry.c b/xlators/cluster/afr/src/afr-self-heal-entry.c
index 63c86d8904e..df6dfaaf396 100644
--- a/xlators/cluster/afr/src/afr-self-heal-entry.c
+++ b/xlators/cluster/afr/src/afr-self-heal-entry.c
@@ -323,32 +323,124 @@ __afr_selfheal_entry_dirent (call_frame_t *frame, xlator_t *this, fd_t *fd,
 	return ret;
 }
 
+static int
+__afr_selfheal_entry_finalize_source (xlator_t *this, unsigned char *sources,
+				      unsigned char *healed_sinks,
+				      unsigned char *locked_on)
+{
+	int i = 0;
+	afr_private_t *priv = NULL;
+	int source = -1;
+	int sources_count = 0;
+
+	priv = this->private;
+
+	sources_count = AFR_COUNT (sources, priv->child_count);
+
+	if ((AFR_CMP (locked_on, healed_sinks, priv->child_count) == 0)
+            || !sources_count) {
+		return -1;
+	}
+
+	for (i = 0; i < priv->child_count; i++) {
+		if (sources[i]) {
+			source = i;
+			break;
+		}
+	}
+
+	return source;
+}
+
 
 static int
-afr_selfheal_entry_dirent (call_frame_t *frame, xlator_t *this, fd_t *fd,
-			   int source, unsigned char *sources,
-			   unsigned char *healed_sinks, char *name)
+__afr_selfheal_entry_prepare (call_frame_t *frame, xlator_t *this, fd_t *fd,
+			      unsigned char *locked_on, unsigned char *sources,
+			      unsigned char *sinks, unsigned char *healed_sinks,
+			      struct afr_reply *replies, int *source_p)
 {
+	int ret = -1;
+	int source = -1;
 	afr_private_t *priv = NULL;
-	int ret = 0;
-	unsigned char *locked_on = NULL;
-	struct afr_reply *replies = NULL;
-	inode_t *inode = NULL;
 
 	priv = this->private;
 
+	ret = afr_selfheal_unlocked_discover (frame, fd->inode, fd->inode->gfid,
+					      replies);
+	if (ret)
+		return ret;
+
+	ret = afr_selfheal_find_direction (this, replies, AFR_ENTRY_TRANSACTION,
+					   locked_on, sources, sinks);
+	if (ret)
+		return ret;
+
+        /* Initialize the healed_sinks[] array optimistically to
+           the intersection of to-be-healed (i.e sinks[]) and
+           the list of servers which are up (i.e locked_on[]).
+
+           As we encounter failures in the healing process, we
+           will unmark the respective servers in the healed_sinks[]
+           array.
+        */
+        AFR_INTERSECT (healed_sinks, sinks, locked_on, priv->child_count);
+
+	source = __afr_selfheal_entry_finalize_source (this, sources,
+                                                       healed_sinks, locked_on);
+	if (source < 0) {
+		/* If source is < 0 (typically split-brain), we perform a
+		   conservative merge of entries rather than erroring out */
+	}
+	*source_p = source;
+
+	return ret;
+}
+
+
+static int
+afr_selfheal_entry_dirent (call_frame_t *frame, xlator_t *this,
+                           fd_t *fd, char *name)
+{
+        int                ret          = 0;
+        int                source       = -1;
+        unsigned char     *locked_on    = NULL;
+        unsigned char     *sources      = NULL;
+        unsigned char     *sinks        = NULL;
+        unsigned char     *healed_sinks = NULL;
+        inode_t           *inode        = NULL;
+        struct afr_reply  *replies      = NULL;
+        struct afr_reply  *par_replies  = NULL;
+        afr_private_t     *priv         = NULL;
+
+	priv = this->private;
+
+        sources = alloca0 (priv->child_count);
+        sinks   = alloca0 (priv->child_count);
+        healed_sinks = alloca0 (priv->child_count);
 	locked_on = alloca0 (priv->child_count);
 
 	replies = alloca0 (priv->child_count * sizeof(*replies));
+	par_replies = alloca0 (priv->child_count * sizeof(*par_replies));
 
-	ret = afr_selfheal_entrylk (frame, this, fd->inode, this->name,
-				    name, locked_on);
+        ret = afr_selfheal_entrylk (frame, this, fd->inode, this->name, NULL,
+                                    locked_on);
 	{
 		if (ret < AFR_SH_MIN_PARTICIPANTS) {
+                        gf_log (this->name, GF_LOG_DEBUG, "%s: Skipping "
+                                "entry self-heal as only %d sub-volumes could "
+                                "be locked in %s domain",
+                                uuid_utoa (fd->inode->gfid), ret, this->name);
 			ret = -ENOTCONN;
 			goto unlock;
 		}
 
+                ret = __afr_selfheal_entry_prepare (frame, this, fd, locked_on,
+                                                    sources, sinks,
+                                                    healed_sinks, par_replies,
+                                                    &source);
+                if (ret < 0)
+                        goto unlock;
+
 		inode = afr_selfheal_unlocked_lookup_on (frame, fd->inode, name,
 							 replies, locked_on,
                                                          NULL);
@@ -362,20 +454,22 @@ afr_selfheal_entry_dirent (call_frame_t *frame, xlator_t *this, fd_t *fd,
 						   locked_on, replies);
 	}
 unlock:
-	afr_selfheal_unentrylk (frame, this, fd->inode, this->name, name,
-				locked_on);
+        afr_selfheal_unentrylk (frame, this, fd->inode, this->name, NULL,
+                                locked_on);
 	if (inode)
 		inode_unref (inode);
         if (replies)
                 afr_replies_wipe (replies, priv->child_count);
+        if (par_replies)
+                afr_replies_wipe (par_replies, priv->child_count);
+
 	return ret;
 }
 
 
 static int
-afr_selfheal_entry_do_subvol (call_frame_t *frame, xlator_t *this, fd_t *fd,
-			      int child, int source, unsigned char *sources,
-			      unsigned char *healed_sinks)
+afr_selfheal_entry_do_subvol (call_frame_t *frame, xlator_t *this,
+                              fd_t *fd, int child)
 {
 	int ret = 0;
 	gf_dirent_t entries;
@@ -409,9 +503,7 @@ afr_selfheal_entry_do_subvol (call_frame_t *frame, xlator_t *this, fd_t *fd,
 				continue;
 
 			ret = afr_selfheal_entry_dirent (iter_frame, this, fd,
-							 source, sources,
-							 healed_sinks,
-							 entry->d_name);
+                                                         entry->d_name);
 			AFR_STACK_RESET (iter_frame);
 
 			if (ret)
@@ -444,8 +536,7 @@ afr_selfheal_entry_do (call_frame_t *frame, xlator_t *this, fd_t *fd,
 	for (i = 0; i < priv->child_count; i++) {
 		if (i != source && !healed_sinks[i])
 			continue;
-		ret = afr_selfheal_entry_do_subvol (frame, this, fd, i, source,
-						    sources, healed_sinks);
+		ret = afr_selfheal_entry_do_subvol (frame, this, fd, i);
 		if (ret)
 			break;
 	}
@@ -453,79 +544,6 @@ afr_selfheal_entry_do (call_frame_t *frame, xlator_t *this, fd_t *fd,
 }
 
 
-static int
-__afr_selfheal_entry_finalize_source (xlator_t *this, unsigned char *sources,
-				      unsigned char *healed_sinks,
-				      unsigned char *locked_on)
-{
-	int i = 0;
-	afr_private_t *priv = NULL;
-	int source = -1;
-	int sources_count = 0;
-
-	priv = this->private;
-
-	sources_count = AFR_COUNT (sources, priv->child_count);
-
-	if ((AFR_CMP (locked_on, healed_sinks, priv->child_count) == 0)
-            || !sources_count) {
-		return -1;
-	}
-
-	for (i = 0; i < priv->child_count; i++) {
-		if (sources[i]) {
-			source = i;
-			break;
-		}
-	}
-
-	return source;
-}
-
-
-static int
-__afr_selfheal_entry_prepare (call_frame_t *frame, xlator_t *this, fd_t *fd,
-			      unsigned char *locked_on, unsigned char *sources,
-			      unsigned char *sinks, unsigned char *healed_sinks,
-			      struct afr_reply *replies, int *source_p)
-{
-	int ret = -1;
-	int source = -1;
-	afr_private_t *priv = NULL;
-
-	priv = this->private;
-
-	ret = afr_selfheal_unlocked_discover (frame, fd->inode, fd->inode->gfid,
-					      replies);
-	if (ret)
-		return ret;
-
-	ret = afr_selfheal_find_direction (this, replies, AFR_ENTRY_TRANSACTION,
-					   locked_on, sources, sinks);
-	if (ret)
-		return ret;
-
-        /* Initialize the healed_sinks[] array optimistically to
-           the intersection of to-be-healed (i.e sinks[]) and
-           the list of servers which are up (i.e locked_on[]).
-
-           As we encounter failures in the healing process, we
-           will unmark the respective servers in the healed_sinks[]
-           array.
-        */
-        AFR_INTERSECT (healed_sinks, sinks, locked_on, priv->child_count);
-
-	source = __afr_selfheal_entry_finalize_source (this, sources,
-                                                       healed_sinks, locked_on);
-	if (source < 0) {
-		/* If source is < 0 (typically split-brain), we perform a
-		   conservative merge of entries rather than erroring out */
-	}
-	*source_p = source;
-
-	return ret;
-}
-
 
 static int
 __afr_selfheal_entry (call_frame_t *frame, xlator_t *this, fd_t *fd,
@@ -553,6 +571,11 @@ __afr_selfheal_entry (call_frame_t *frame, xlator_t *this, fd_t *fd,
 				    data_lock);
 	{
 		if (ret < AFR_SH_MIN_PARTICIPANTS) {
+                        gf_log (this->name, GF_LOG_DEBUG, "%s: Skipping "
+                                "entry self-heal as only %d sub-volumes could "
+                                "be locked in %s domain",
+                                uuid_utoa (fd->inode->gfid), ret,
+                                this->name);
 			ret = -ENOTCONN;
 			goto unlock;
 		}
@@ -576,6 +599,9 @@ unlock:
 					 healed_sinks, AFR_ENTRY_TRANSACTION,
                                          locked_replies, data_lock);
 out:
+        afr_log_selfheal (fd->inode->gfid, this, ret, "entry", source,
+                          healed_sinks);
+
         if (locked_replies)
                 afr_replies_wipe (locked_replies, priv->child_count);
 	return ret;
@@ -629,6 +655,11 @@ afr_selfheal_entry (call_frame_t *frame, xlator_t *this, inode_t *inode)
 				       locked_on);
 	{
 		if (ret < AFR_SH_MIN_PARTICIPANTS) {
+                        gf_log (this->name, GF_LOG_DEBUG, "%s: Skipping "
+                                "entry self-heal as only %d sub-volumes could "
+                                "be locked in %s domain",
+                                uuid_utoa (fd->inode->gfid), ret,
+                                priv->sh_domain);
 			/* Either less than two subvols available, or another
 			   selfheal (from another server) is in progress. Skip
 			   for now in any case there isn't anything to do.