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, 36 insertions, 14 deletions

diff --git a/xlators/cluster/afr/src/afr-self-heal-metadata.c b/xlators/cluster/afr/src/afr-self-heal-metadata.c
index 147ab98d4..79ce07d00 100644
--- a/xlators/cluster/afr/src/afr-self-heal-metadata.c
+++ b/xlators/cluster/afr/src/afr-self-heal-metadata.c
@@ -50,7 +50,7 @@ afr_sh_metadata_done (call_frame_t *frame, xlator_t *this)
         sh = &local->self_heal;
 
         afr_sh_reset (frame, this);
-        if (local->govinda_gOvinda) {
+        if (sh->mdata_spb) {
                 gf_log (this->name, GF_LOG_INFO,
                         "split-brain detected, aborting selfheal of %s",
                         local->loc.path);
@@ -383,15 +383,6 @@ afr_sh_metadata_fix (call_frame_t *frame, xlator_t *this,
                                       sh->pending_matrix, sh->sources,
                                       sh->success_children,
                                       AFR_METADATA_TRANSACTION, NULL, _gf_false);
-        if (nsources == 0) {
-                gf_log (this->name, GF_LOG_TRACE,
-                        "No self-heal needed for %s",
-                        local->loc.path);
-
-                afr_sh_metadata_finish (frame, this);
-                goto out;
-        }
-
         if ((nsources == -1)
             && (priv->favorite_child != -1)
             && (sh->child_errno[priv->favorite_child] == 0)) {
@@ -413,7 +404,17 @@ afr_sh_metadata_fix (call_frame_t *frame, xlator_t *this,
                         "(possible split-brain). Please fix the file on "
                         "all backend volumes", local->loc.path);
 
-                local->govinda_gOvinda = 1;
+                sh->mdata_spb = _gf_true;
+
+                afr_sh_metadata_finish (frame, this);
+                goto out;
+        }
+
+        sh->mdata_spb = _gf_false;
+        if (nsources == 0) {
+                gf_log (this->name, GF_LOG_TRACE,
+                        "No self-heal needed for %s",
+                        local->loc.path);
 
                 afr_sh_metadata_finish (frame, this);
                 goto out;
@@ -452,7 +453,10 @@ afr_sh_metadata_fix (call_frame_t *frame, xlator_t *this,
                                         sh->fresh_children);
         }
 
-        afr_sh_metadata_sync_prepare (frame, this);
+        if (sh->do_metadata_self_heal && priv->metadata_self_heal)
+                afr_sh_metadata_sync_prepare (frame, this);
+        else
+                afr_sh_metadata_finish (frame, this);
 out:
         return;
 }
@@ -512,17 +516,35 @@ afr_sh_metadata_lock (call_frame_t *frame, xlator_t *this)
         return 0;
 }
 
+gf_boolean_t
+afr_can_start_metadata_self_heal (afr_self_heal_t *sh, afr_private_t *priv)
+{
+        if (sh->force_confirm_spb)
+                return _gf_true;
+        if (sh->do_metadata_self_heal && priv->metadata_self_heal)
+                return _gf_true;
+        return _gf_false;
+}
 
 int
 afr_self_heal_metadata (call_frame_t *frame, xlator_t *this)
 {
         afr_local_t   *local = NULL;
         afr_private_t *priv = this->private;
-
+        afr_self_heal_t *sh = &local->self_heal;
 
         local = frame->local;
+        sh = &local->self_heal;
+
+        /* Self-heal completion cbk changes inode split-brain status based on
+         * govinda_gOvinda, mdata_spb, data_spb value.
+         * Initialize mdata_spb with current split-brain status.
+         * If for some reason self-heal fails(locking phase etc), it makes sure
+         * we retain the split-brain status before this self-heal started.
+         */
+        sh->mdata_spb = afr_is_split_brain (this, sh->inode);
 
-        if (local->self_heal.do_metadata_self_heal && priv->metadata_self_heal) {
+        if (afr_can_start_metadata_self_heal (sh, priv)) {
                 afr_sh_metadata_lock (frame, this);
         } else {
                 afr_sh_metadata_done (frame, this);