cluster/afr: Modified split-brain handling

RCA
The bug is observed because the decision to mark
a file in split-brain is taken outside appropriate locks.
Lookup gathers xattrs outside any lock. The xattrs being
in split-brain in lookup should only be taken as a hint.
Appropriate inodelks should be taken before confirming
a split-brain. Self-heal confirms this at the moment.
If data/metadata self-heal is turned off, inspecting of
xattrs could not be performed so split-brain behavior
does not work correctly if the self-heal options are turned off.

Fix
Self-heals are launched to inspect xattrs even when the
data/metadata self-heal options are turned off. The decision
to heal data/metadata after the xattrs are inspected is based
on whether the options are turned on/off. So decision to set/reset
split-brain flag is taken inside appropriate locks.

Testcases:
tests 33-36 in
https://github.com/pranithk/gluster-tests/blob/master/afr/self-heal.sh

Change-Id: Ia8aeab08208b50c06609ad35a9d72f3d553ee343
BUG: 833727
Signed-off-by: Pranith Kumar K <pranithk@gluster.com>
Reviewed-on: http://review.gluster.com/3626
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Anand Avati <avati@redhat.com>

1 files changed, 65 insertions, 16 deletions

diff --git a/xlators/cluster/afr/src/afr-common.c b/xlators/cluster/afr/src/afr-common.c
index b6e16074..70a9cd35 100644
--- a/xlators/cluster/afr/src/afr-common.c
+++ b/xlators/cluster/afr/src/afr-common.c
@@ -908,6 +908,8 @@ afr_local_cleanup (afr_local_t *local, xlator_t *this)
                 GF_FREE (local->cont.lookup.success_children);
 
                 GF_FREE (local->cont.lookup.sources);
+                afr_matrix_cleanup (local->cont.lookup.pending_matrix,
+                                    priv->child_count);
         }
 
         { /* getxattr */
@@ -1181,6 +1183,36 @@ afr_lookup_set_self_heal_params_by_xattr (afr_local_t *local, xlator_t *this,
         }
 }
 
+void
+afr_lookup_check_set_metadata_split_brain (afr_local_t *local, xlator_t *this)
+{
+        int32_t                  *sources = NULL;
+        afr_private_t            *priv = NULL;
+        int32_t                  subvol_status = 0;
+        int32_t                  *success_children   = NULL;
+        dict_t                   **xattrs = NULL;
+        struct iatt              *bufs = NULL;
+        int32_t                  **pending_matrix = NULL;
+
+        priv = this->private;
+
+        sources = GF_CALLOC (priv->child_count, sizeof (*sources),
+                             gf_afr_mt_int32_t);
+        if (NULL == sources)
+                goto out;
+        success_children = local->cont.lookup.success_children;
+        xattrs = local->cont.lookup.xattrs;
+        bufs = local->cont.lookup.bufs;
+        pending_matrix = local->cont.lookup.pending_matrix;
+        afr_build_sources (this, xattrs, bufs, pending_matrix,
+                           sources, success_children, AFR_METADATA_TRANSACTION,
+                           &subvol_status, _gf_false);
+        if (subvol_status & SPLIT_BRAIN)
+                local->cont.lookup.possible_spb = _gf_true;
+out:
+        GF_FREE (sources);
+}
+
 static void
 afr_detect_self_heal_by_iatt (afr_local_t *local, xlator_t *this,
                             struct iatt *buf, struct iatt *lookup_buf)
@@ -1214,8 +1246,26 @@ afr_detect_self_heal_by_iatt (afr_local_t *local, xlator_t *this,
 }
 
 static void
-afr_detect_self_heal_by_lookup_status (afr_local_t *local, xlator_t *this,
-                                       gf_boolean_t split_brain)
+afr_detect_self_heal_by_split_brain_status (afr_local_t *local, xlator_t *this)
+{
+        gf_boolean_t split_brain = _gf_false;
+        afr_self_heal_t *sh = NULL;
+
+        sh = &local->self_heal;
+
+        split_brain = afr_is_split_brain (this, local->cont.lookup.inode);
+        split_brain = split_brain || local->cont.lookup.possible_spb;
+        if ((local->success_count > 0) && split_brain &&
+            IA_ISREG (local->cont.lookup.inode->ia_type)) {
+                sh->force_confirm_spb = _gf_true;
+                gf_log (this->name, GF_LOG_WARNING,
+                        "split brain detected during lookup of %s.",
+                        local->loc.path);
+        }
+}
+
+static void
+afr_detect_self_heal_by_lookup_status (afr_local_t *local, xlator_t *this)
 {
         GF_ASSERT (local);
         GF_ASSERT (this);
@@ -1233,15 +1283,6 @@ afr_detect_self_heal_by_lookup_status (afr_local_t *local, xlator_t *this,
                 goto out;
         }
 
-        if ((local->success_count > 0) && split_brain &&
-            IA_ISREG (local->cont.lookup.inode->ia_type)) {
-                local->self_heal.do_data_self_heal = _gf_true;
-                local->self_heal.do_gfid_self_heal    = _gf_true;
-                gf_log (this->name, GF_LOG_WARNING,
-                        "split brain detected during lookup of %s.",
-                        local->loc.path);
-        }
-
 out:
         return;
 }
@@ -1252,6 +1293,8 @@ afr_can_self_heal_proceed (afr_self_heal_t *sh, afr_private_t *priv)
         GF_ASSERT (sh);
         GF_ASSERT (priv);
 
+        if (sh->force_confirm_spb)
+                return _gf_true;
         return (sh->do_gfid_self_heal
                 || sh->do_missing_entry_self_heal
                 || (afr_data_self_heal_enabled (priv->data_self_heal) &&
@@ -1516,13 +1559,11 @@ afr_lookup_set_self_heal_params (afr_local_t *local, xlator_t *this)
         int32_t                 child1 = -1;
         int32_t                 child2 = -1;
         afr_self_heal_t         *sh = NULL;
-        gf_boolean_t            split_brain = _gf_false;
 
         priv  = this->private;
         sh = &local->self_heal;
 
-        split_brain = afr_is_split_brain (this, local->cont.lookup.inode);
-        afr_detect_self_heal_by_lookup_status (local, this, split_brain);
+        afr_detect_self_heal_by_lookup_status (local, this);
 
         if (afr_lookup_gfid_missing_count (local, this))
                 local->self_heal.do_gfid_self_heal    = _gf_true;
@@ -1549,9 +1590,11 @@ afr_lookup_set_self_heal_params (afr_local_t *local, xlator_t *this)
                 afr_lookup_set_self_heal_params_by_xattr (local, this,
                                                           xattr[child1]);
         }
-        if (afr_open_only_data_self_heal (priv->data_self_heal)
-            && !split_brain)
+        if (afr_open_only_data_self_heal (priv->data_self_heal))
                 sh->do_data_self_heal = _gf_false;
+        if (sh->do_metadata_self_heal)
+                afr_lookup_check_set_metadata_split_brain (local, this);
+        afr_detect_self_heal_by_split_brain_status (local, this);
 }
 
 int
@@ -2143,6 +2186,7 @@ afr_lookup_cont_init (afr_local_t *local, unsigned int child_count)
         struct iatt       *iatts         = NULL;
         int32_t           *success_children = NULL;
         int32_t           *sources       = NULL;
+        int32_t           **pending_matrix = NULL;
 
         GF_ASSERT (local);
         local->cont.lookup.xattrs = GF_CALLOC (child_count,
@@ -2175,6 +2219,11 @@ afr_lookup_cont_init (afr_local_t *local, unsigned int child_count)
                 goto out;
         local->cont.lookup.sources = sources;
 
+        pending_matrix = afr_matrix_create (child_count, child_count);
+        if (NULL == pending_matrix)
+                goto out;
+        local->cont.lookup.pending_matrix = pending_matrix;
+
         ret = 0;
 out:
         return ret;