cluster/afr: lookup should set the read-child based on pending xattrs

Signed-off-by: Pranith Kumar K <pranithk@gluster.com>
Signed-off-by: Anand Avati <avati@gluster.com>

BUG: 2840 (files not getting self-healed when the first child goes down)
URL: http://bugs.gluster.com/cgi-bin/bugzilla3/show_bug.cgi?id=2840

3 files changed, 100 insertions, 34 deletions

diff --git a/xlators/cluster/afr/src/afr-common.c b/xlators/cluster/afr/src/afr-common.c
index 3d0ffb1d89e..868dab736b9 100644
--- a/xlators/cluster/afr/src/afr-common.c
+++ b/xlators/cluster/afr/src/afr-common.c
@@ -616,21 +616,84 @@ afr_lookup_self_heal_check (xlator_t *this, afr_local_t *local,
 
 }
 
+int
+afr_is_valid_read_child (int *sources, int32_t child_count, int32_t read_child)
+{
+        int valid = 0;
+
+        if (!sources)
+                goto out;
+
+        if ((read_child < 0) ||
+            (read_child >= child_count))
+                goto out;
+
+        valid = sources[read_child];
+out:
+        return valid;
+}
+
+void
+afr_lookup_set_read_child (xlator_t *this, afr_local_t *local)
+{
+        ia_type_t               ia_type = IA_INVAL;
+        afr_transaction_type    transaction_type = AFR_DATA_TRANSACTION;
+        afr_private_t           *priv = NULL;
+        int32_t                 read_child = -1;
+        afr_self_heal_t         *sh = NULL;
+
+        priv = this->private;
+        sh   = &local->self_heal;
+
+        ia_type = local->cont.lookup.inode->ia_type;
+        if (IA_ISREG (ia_type)) {
+                transaction_type = AFR_DATA_TRANSACTION;
+        } else if IA_ISDIR (ia_type) {
+                transaction_type = AFR_ENTRY_TRANSACTION;
+        } else {
+                transaction_type = AFR_METADATA_TRANSACTION;
+        }
+        afr_self_heal_find_sources (this, local,
+                                    local->cont.lookup.xattrs,
+                                    transaction_type);
+        if (!sh->sources)
+                goto out;
+
+        read_child = priv->read_child;
+        if (afr_is_valid_read_child (sh->sources, priv->child_count,
+                                     read_child))
+                goto out;
+
+        read_child = afr_read_child (this, local->loc.inode);
+        if (afr_is_valid_read_child (sh->sources, priv->child_count,
+                                     read_child))
+                goto out;
+
+        read_child = afr_sh_select_source (sh->sources, priv->child_count);
+out:
+        if (read_child >= 0) {
+                afr_set_read_child (this,
+                                    local->cont.lookup.inode,
+                                    read_child);
+        }
+}
 
 static void
 afr_lookup_done (call_frame_t *frame, xlator_t *this, struct iatt *lookup_buf)
 {
-        int  unwind = 1;
-        int  source = -1;
-        int  up_count = 0;
-        char sh_type_str[256] = {0,};
-
-        afr_private_t *priv  = NULL;
-        afr_local_t   *local = NULL;
+        int                     unwind = 1;
+        int                     up_count = 0;
+        char                    sh_type_str[256] = {0,};
+        afr_private_t           *priv  = NULL;
+        afr_local_t             *local = NULL;
 
         priv  = this->private;
         local = frame->local;
 
+        if (local->op_ret != 0)
+                goto unwind;
+
+
         local->cont.lookup.postparent.ia_ino  = local->cont.lookup.parent_ino;
 
         if (local->cont.lookup.ino) {
@@ -645,6 +708,8 @@ afr_lookup_done (call_frame_t *frame, xlator_t *this, struct iatt *lookup_buf)
                                 local->cont.lookup.inode->ino;
                 }
         }
+
+        afr_lookup_set_read_child (this, local);
         up_count = afr_up_children_count (priv->child_count, priv->child_up);
         if (up_count == 1) {
                 gf_log (this->name, GF_LOG_DEBUG,
@@ -673,27 +738,16 @@ afr_lookup_done (call_frame_t *frame, xlator_t *this, struct iatt *lookup_buf)
                 }
         }
 
-        if ((local->self_heal.need_metadata_self_heal
+        if (local->self_heal.need_metadata_self_heal
              || local->self_heal.need_data_self_heal
-             || local->self_heal.need_entry_self_heal)
-            && ((!local->cont.lookup.is_revalidate)
-                || (local->op_ret != -1))) {
+             || local->self_heal.need_entry_self_heal) {
 
                 if (local->inodelk_count || local->entrylk_count) {
 
                         /* Someone else is doing self-heal on this file.
-                           So just make a best effort to set the read-subvolume
-                           and return */
-
-                        if (IA_ISREG (local->cont.lookup.inode->ia_type)) {
-                                source = afr_self_heal_get_source (this, local, local->cont.lookup.xattrs);
+                           return */
 
-                                if (source >= 0) {
-                                        afr_set_read_child (this,
-                                                            local->cont.lookup.inode,
-                                                            source);
-                                }
-                        }
+                        goto unwind;
                 } else {
                         if (!local->cont.lookup.inode->ia_type) {
                                 /* fix for RT #602 */
diff --git a/xlators/cluster/afr/src/afr-self-heal-data.c b/xlators/cluster/afr/src/afr-self-heal-data.c
index 263d49fbb2d..5adaba1c974 100644
--- a/xlators/cluster/afr/src/afr-self-heal-data.c
+++ b/xlators/cluster/afr/src/afr-self-heal-data.c
@@ -697,14 +697,15 @@ afr_sh_data_fix (call_frame_t *frame, xlator_t *this)
 }
 
 
-int
-afr_self_heal_get_source (xlator_t *this, afr_local_t *local, dict_t **xattr)
+void
+afr_self_heal_find_sources (xlator_t *this, afr_local_t *local, dict_t **xattr,
+                            afr_transaction_type transaction_type)
 {
 	afr_self_heal_t *sh   = NULL;
 	afr_private_t   *priv = NULL;
 
-	int              source = 0;
 	int              i = 0;
+        afr_self_heal_type sh_type = AFR_SELF_HEAL_DATA;
 
 	sh   = &local->self_heal;
 	priv = this->private;
@@ -721,13 +722,23 @@ afr_self_heal_get_source (xlator_t *this, afr_local_t *local, dict_t **xattr)
                                  gf_afr_mt_int32_t);
 
 	afr_sh_build_pending_matrix (priv, sh->pending_matrix, xattr,
-				     priv->child_count, AFR_DATA_TRANSACTION);
-
-	(void)afr_sh_mark_sources (sh, priv->child_count, AFR_SELF_HEAL_DATA);
-
-	source = afr_sh_select_source (sh->sources, priv->child_count);
-
-	return source;
+                                     priv->child_count, transaction_type);
+
+        switch (transaction_type) {
+        case AFR_DATA_TRANSACTION:
+                sh_type = AFR_SELF_HEAL_DATA;
+                break;
+        case AFR_ENTRY_TRANSACTION:
+                sh_type = AFR_SELF_HEAL_ENTRY;
+                break;
+        case AFR_METADATA_TRANSACTION:
+                sh_type = AFR_SELF_HEAL_METADATA;
+                break;
+        default:
+                sh_type = AFR_SELF_HEAL_METADATA;
+                break;
+        }
+        (void)afr_sh_mark_sources (sh, priv->child_count, sh_type);
 }
 
 
diff --git a/xlators/cluster/afr/src/afr-self-heal.h b/xlators/cluster/afr/src/afr-self-heal.h
index b10ae3fc037..f8f83a01733 100644
--- a/xlators/cluster/afr/src/afr-self-heal.h
+++ b/xlators/cluster/afr/src/afr-self-heal.h
@@ -45,8 +45,9 @@ afr_self_heal_data (call_frame_t *frame, xlator_t *this);
 int
 afr_self_heal_metadata (call_frame_t *frame, xlator_t *this);
 
-int
-afr_self_heal_get_source (xlator_t *this, afr_local_t *local, dict_t **xattr);
+void
+afr_self_heal_find_sources (xlator_t *this, afr_local_t *local, dict_t **xattr,
+                            afr_transaction_type transaction_type);
 
 int
 afr_self_heal (call_frame_t *frame, xlator_t *this);