d1681e
From 6fb1804d113ae996e085ef0f23fa8908d167f006 Mon Sep 17 00:00:00 2001
d1681e
From: Ravishankar N <ravishankar@redhat.com>
d1681e
Date: Thu, 14 Jun 2018 12:59:06 +0530
d1681e
Subject: [PATCH 299/305] afr: heal gfids when file is not present on all
d1681e
 bricks
d1681e
d1681e
Backport of: https://review.gluster.org/#/c/20271/
d1681e
commit f73814ad08d552d94d0139b2592175d206e7a166 (rhgs) introduced a regression
d1681e
wherein if a file is present in only 1 brick of replica *and* doesn't
d1681e
have a gfid associated with it, it doesn't get healed upon the next
d1681e
lookup from the client. Fix it.
d1681e
d1681e
Change-Id: I7d1111dcb45b1b8b8340a7d02558f05df70aa599
d1681e
BUG: 1592666
d1681e
Signed-off-by: Ravishankar N <ravishankar@redhat.com>
d1681e
Reviewed-on: https://code.engineering.redhat.com/gerrit/141899
d1681e
Tested-by: RHGS Build Bot <nigelb@redhat.com>
d1681e
Reviewed-by: Karthik Subrahmanya <ksubrahm@redhat.com>
d1681e
---
d1681e
 .../replicate/bug-1591193-assign-gfid-and-heal.t   | 128 +++++++++++++++++++++
d1681e
 xlators/cluster/afr/src/afr-self-heal-common.c     |  39 ++++++-
d1681e
 xlators/cluster/afr/src/afr-self-heal-data.c       |   8 +-
d1681e
 xlators/cluster/afr/src/afr-self-heal-entry.c      |   4 +-
d1681e
 xlators/cluster/afr/src/afr-self-heal-name.c       |   6 +-
d1681e
 xlators/cluster/afr/src/afr-self-heal.h            |   6 +-
d1681e
 6 files changed, 179 insertions(+), 12 deletions(-)
d1681e
 create mode 100644 tests/bugs/replicate/bug-1591193-assign-gfid-and-heal.t
d1681e
d1681e
diff --git a/tests/bugs/replicate/bug-1591193-assign-gfid-and-heal.t b/tests/bugs/replicate/bug-1591193-assign-gfid-and-heal.t
d1681e
new file mode 100644
d1681e
index 0000000..d3b5f9a
d1681e
--- /dev/null
d1681e
+++ b/tests/bugs/replicate/bug-1591193-assign-gfid-and-heal.t
d1681e
@@ -0,0 +1,128 @@
d1681e
+#!/bin/bash
d1681e
+
d1681e
+. $(dirname $0)/../../include.rc
d1681e
+. $(dirname $0)/../../volume.rc
d1681e
+. $(dirname $0)/../../afr.rc
d1681e
+
d1681e
+cleanup;
d1681e
+
d1681e
+function check_gfid_and_link_count
d1681e
+{
d1681e
+        local file=$1
d1681e
+
d1681e
+        file_gfid_b0=$(gf_get_gfid_xattr $B0/${V0}0/$file)
d1681e
+        TEST [ ! -z $file_gfid_b0 ]
d1681e
+        file_gfid_b1=$(gf_get_gfid_xattr $B0/${V0}1/$file)
d1681e
+        file_gfid_b2=$(gf_get_gfid_xattr $B0/${V0}2/$file)
d1681e
+        EXPECT $file_gfid_b0 echo $file_gfid_b1
d1681e
+        EXPECT $file_gfid_b0 echo $file_gfid_b2
d1681e
+
d1681e
+        EXPECT "2" stat -c %h $B0/${V0}0/$file
d1681e
+        EXPECT "2" stat -c %h $B0/${V0}1/$file
d1681e
+        EXPECT "2" stat -c %h $B0/${V0}2/$file
d1681e
+}
d1681e
+TESTS_EXPECTED_IN_LOOP=30
d1681e
+
d1681e
+##############################################################################
d1681e
+# Test on 1x3 volume
d1681e
+TEST glusterd
d1681e
+TEST pidof glusterd
d1681e
+TEST $CLI volume create $V0 replica 3 $H0:$B0/${V0}{0,1,2};
d1681e
+TEST $CLI volume start $V0;
d1681e
+
d1681e
+EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" brick_up_status $V0 $H0 $B0/${V0}0
d1681e
+EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" brick_up_status $V0 $H0 $B0/${V0}1
d1681e
+EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" brick_up_status $V0 $H0 $B0/${V0}2
d1681e
+
d1681e
+EXPECT_WITHIN $PROCESS_UP_TIMEOUT "Y" glustershd_up_status
d1681e
+EXPECT_WITHIN $CHILD_UP_TIMEOUT "1" afr_child_up_status_in_shd $V0 0
d1681e
+EXPECT_WITHIN $CHILD_UP_TIMEOUT "1" afr_child_up_status_in_shd $V0 1
d1681e
+EXPECT_WITHIN $CHILD_UP_TIMEOUT "1" afr_child_up_status_in_shd $V0 2
d1681e
+
d1681e
+TEST glusterfs --volfile-id=/$V0 --volfile-server=$H0 --attribute-timeout=0 --entry-timeout=0 $M0
d1681e
+EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" afr_child_up_status $V0 0
d1681e
+EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" afr_child_up_status $V0 1
d1681e
+EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" afr_child_up_status $V0 2
d1681e
+
d1681e
+
d1681e
+# Create files directly in the backend on different bricks
d1681e
+echo $RANDOM >> $B0/${V0}0/file1
d1681e
+echo $RANDOM >> $B0/${V0}1/file2
d1681e
+echo $RANDOM >> $B0/${V0}2/file3
d1681e
+
d1681e
+# To prevent is_fresh_file code path
d1681e
+sleep 2
d1681e
+
d1681e
+# Access them from mount to trigger name + gfid heal.
d1681e
+TEST stat $M0/file1
d1681e
+TEST stat $M0/file2
d1681e
+TEST stat $M0/file3
d1681e
+
d1681e
+# Launch index heal to complete any pending data/metadata heals.
d1681e
+TEST $CLI volume heal $V0
d1681e
+EXPECT_WITHIN $HEAL_TIMEOUT "^0$" get_pending_heal_count $V0
d1681e
+
d1681e
+# Check each file has a gfid and the .glusterfs hardlink
d1681e
+check_gfid_and_link_count file1
d1681e
+check_gfid_and_link_count file2
d1681e
+check_gfid_and_link_count file3
d1681e
+
d1681e
+TEST rm $M0/file1
d1681e
+TEST rm $M0/file2
d1681e
+TEST rm $M0/file3
d1681e
+cleanup;
d1681e
+
d1681e
+##############################################################################
d1681e
+# Test on 1x (2+1) volume
d1681e
+TEST glusterd
d1681e
+TEST pidof glusterd
d1681e
+TEST $CLI volume create $V0 replica 3 arbiter 1 $H0:$B0/${V0}{0,1,2};
d1681e
+TEST $CLI volume start $V0;
d1681e
+
d1681e
+EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" brick_up_status $V0 $H0 $B0/${V0}0
d1681e
+EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" brick_up_status $V0 $H0 $B0/${V0}1
d1681e
+EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" brick_up_status $V0 $H0 $B0/${V0}2
d1681e
+
d1681e
+EXPECT_WITHIN $PROCESS_UP_TIMEOUT "Y" glustershd_up_status
d1681e
+EXPECT_WITHIN $CHILD_UP_TIMEOUT "1" afr_child_up_status_in_shd $V0 0
d1681e
+EXPECT_WITHIN $CHILD_UP_TIMEOUT "1" afr_child_up_status_in_shd $V0 1
d1681e
+EXPECT_WITHIN $CHILD_UP_TIMEOUT "1" afr_child_up_status_in_shd $V0 2
d1681e
+
d1681e
+TEST glusterfs --volfile-id=/$V0 --volfile-server=$H0 --attribute-timeout=0 --entry-timeout=0 $M0
d1681e
+EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" afr_child_up_status $V0 0
d1681e
+EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" afr_child_up_status $V0 1
d1681e
+EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" afr_child_up_status $V0 2
d1681e
+
d1681e
+
d1681e
+# Create files directly in the backend on different bricks
d1681e
+echo $RANDOM >> $B0/${V0}0/file1
d1681e
+echo $RANDOM >> $B0/${V0}1/file2
d1681e
+touch $B0/${V0}2/file3
d1681e
+
d1681e
+# To prevent is_fresh_file code path
d1681e
+sleep 2
d1681e
+
d1681e
+# Access them from mount to trigger name + gfid heal.
d1681e
+TEST stat $M0/file1
d1681e
+TEST stat $M0/file2
d1681e
+
d1681e
+# Though file is created on all 3 bricks, lookup will fail as arbiter blames the
d1681e
+# other 2 bricks and ariter is not 'readable'.
d1681e
+# TEST ! stat $M0/file3
d1681e
+# But the checks for failing lookups when quorum is not met is not yet there in
d1681e
+# rhgs-3.4.0, so stat will succeed.
d1681e
+TEST  stat $M0/file3
d1681e
+
d1681e
+# Launch index heal to complete any pending data/metadata heals.
d1681e
+TEST $CLI volume heal $V0
d1681e
+EXPECT_WITHIN $HEAL_TIMEOUT "^0$" get_pending_heal_count $V0
d1681e
+
d1681e
+# Check each file has a gfid and the .glusterfs hardlink
d1681e
+check_gfid_and_link_count file1
d1681e
+check_gfid_and_link_count file2
d1681e
+check_gfid_and_link_count file3
d1681e
+
d1681e
+TEST rm $M0/file1
d1681e
+TEST rm $M0/file2
d1681e
+TEST rm $M0/file3
d1681e
+cleanup;
d1681e
diff --git a/xlators/cluster/afr/src/afr-self-heal-common.c b/xlators/cluster/afr/src/afr-self-heal-common.c
d1681e
index 32fd24a..50989d6 100644
d1681e
--- a/xlators/cluster/afr/src/afr-self-heal-common.c
d1681e
+++ b/xlators/cluster/afr/src/afr-self-heal-common.c
d1681e
@@ -22,7 +22,7 @@ afr_heal_synctask (xlator_t *this, afr_local_t *local);
d1681e
 int
d1681e
 afr_lookup_and_heal_gfid (xlator_t *this, inode_t *parent, const char *name,
d1681e
                           inode_t *inode, struct afr_reply *replies,
d1681e
-                          int source,  void *gfid)
d1681e
+                          int source,  unsigned char *sources, void *gfid)
d1681e
 {
d1681e
         afr_private_t *priv = NULL;
d1681e
         call_frame_t   *frame    = NULL;
d1681e
@@ -37,6 +37,23 @@ afr_lookup_and_heal_gfid (xlator_t *this, inode_t *parent, const char *name,
d1681e
         priv = this->private;
d1681e
         wind_on = alloca0 (priv->child_count);
d1681e
         ia_type = replies[source].poststat.ia_type;
d1681e
+        if ((ia_type == IA_INVAL) &&
d1681e
+            (AFR_COUNT(sources, priv->child_count) == priv->child_count)) {
d1681e
+                /* If a file is present on some bricks of the replica but parent
d1681e
+                 * dir does not have pending xattrs, all bricks are sources and
d1681e
+                 * the 'source' we  selected earlier might be one where the file
d1681e
+                 * is not actually present. Hence check if file is present in
d1681e
+                 * any of the sources.*/
d1681e
+                for (i = 0; i < priv->child_count; i++) {
d1681e
+                        if (i == source)
d1681e
+                                continue;
d1681e
+                        if (sources[i] && replies[i].valid &&
d1681e
+                            replies[i].op_ret == 0) {
d1681e
+                                ia_type = replies[i].poststat.ia_type;
d1681e
+                                break;
d1681e
+                        }
d1681e
+                }
d1681e
+        }
d1681e
 
d1681e
         /* gfid heal on those subvolumes that do not have gfid associated
d1681e
          * with the inode and update those replies.
d1681e
@@ -1250,6 +1267,21 @@ afr_mark_split_brain_source_sinks_by_policy (call_frame_t *frame,
d1681e
         return fav_child;
d1681e
 }
d1681e
 
d1681e
+gf_boolean_t
d1681e
+afr_is_file_empty_on_all_children (afr_private_t *priv,
d1681e
+                                   struct afr_reply *replies)
d1681e
+{
d1681e
+        int i = 0;
d1681e
+
d1681e
+        for (i = 0; i < priv->child_count; i++) {
d1681e
+                if ((!replies[i].valid) || (replies[i].op_ret != 0) ||
d1681e
+                    (replies[i].poststat.ia_size != 0))
d1681e
+                        return _gf_false;
d1681e
+        }
d1681e
+
d1681e
+        return _gf_true;
d1681e
+}
d1681e
+
d1681e
 int
d1681e
 afr_mark_source_sinks_if_file_empty (xlator_t *this, unsigned char *sources,
d1681e
                                      unsigned char *sinks,
d1681e
@@ -1268,11 +1300,8 @@ afr_mark_source_sinks_if_file_empty (xlator_t *this, unsigned char *sources,
d1681e
                 return -1;
d1681e
 
d1681e
         if (type == AFR_DATA_TRANSACTION) {
d1681e
-                for (i = 0; i < priv->child_count; i++) {
d1681e
-                        if (replies[i].poststat.ia_size != 0)
d1681e
+                if (!afr_is_file_empty_on_all_children(priv, replies))
d1681e
                                 return -1;
d1681e
-                }
d1681e
-
d1681e
                 goto mark;
d1681e
         }
d1681e
 
d1681e
diff --git a/xlators/cluster/afr/src/afr-self-heal-data.c b/xlators/cluster/afr/src/afr-self-heal-data.c
d1681e
index f872a98..3ef7376 100644
d1681e
--- a/xlators/cluster/afr/src/afr-self-heal-data.c
d1681e
+++ b/xlators/cluster/afr/src/afr-self-heal-data.c
d1681e
@@ -670,6 +670,7 @@ __afr_selfheal_data (call_frame_t *frame, xlator_t *this, fd_t *fd,
d1681e
 	int source = -1;
d1681e
         gf_boolean_t did_sh = _gf_true;
d1681e
         gf_boolean_t is_arbiter_the_only_sink = _gf_false;
d1681e
+        gf_boolean_t empty_file = _gf_false;
d1681e
 
d1681e
 	priv = this->private;
d1681e
 
d1681e
@@ -710,6 +711,11 @@ __afr_selfheal_data (call_frame_t *frame, xlator_t *this, fd_t *fd,
d1681e
 		source = ret;
d1681e
 
d1681e
                 if (AFR_IS_ARBITER_BRICK(priv, source)) {
d1681e
+                        empty_file = afr_is_file_empty_on_all_children (priv,
d1681e
+                                                                locked_replies);
d1681e
+                        if (empty_file)
d1681e
+                                goto restore_time;
d1681e
+
d1681e
                         did_sh = _gf_false;
d1681e
                         goto unlock;
d1681e
                 }
d1681e
@@ -746,7 +752,7 @@ restore_time:
d1681e
 	afr_selfheal_restore_time (frame, this, fd->inode, source,
d1681e
 					healed_sinks, locked_replies);
d1681e
 
d1681e
-        if (!is_arbiter_the_only_sink) {
d1681e
+        if (!is_arbiter_the_only_sink || !empty_file) {
d1681e
                 ret = afr_selfheal_inodelk (frame, this, fd->inode, this->name,
d1681e
                                             0, 0, data_lock);
d1681e
                 if (ret < priv->child_count) {
d1681e
diff --git a/xlators/cluster/afr/src/afr-self-heal-entry.c b/xlators/cluster/afr/src/afr-self-heal-entry.c
d1681e
index 647dd71..f6d3a8a 100644
d1681e
--- a/xlators/cluster/afr/src/afr-self-heal-entry.c
d1681e
+++ b/xlators/cluster/afr/src/afr-self-heal-entry.c
d1681e
@@ -187,7 +187,7 @@ __afr_selfheal_heal_dirent (call_frame_t *frame, xlator_t *this, fd_t *fd,
d1681e
 
d1681e
         if (replies[source].op_ret == 0) {
d1681e
                 ret = afr_lookup_and_heal_gfid (this, fd->inode, name,
d1681e
-                                                inode, replies, source,
d1681e
+                                                inode, replies, source, sources,
d1681e
                                              &replies[source].poststat.ia_gfid);
d1681e
                 if (ret)
d1681e
                         return ret;
d1681e
@@ -320,7 +320,7 @@ __afr_selfheal_merge_dirent (call_frame_t *frame, xlator_t *this, fd_t *fd,
d1681e
 	}
d1681e
 
d1681e
         ret = afr_lookup_and_heal_gfid (this, fd->inode, name, inode, replies,
d1681e
-                                        source,
d1681e
+                                        source, sources,
d1681e
                                         &replies[source].poststat.ia_gfid);
d1681e
         if (ret)
d1681e
                 return ret;
d1681e
diff --git a/xlators/cluster/afr/src/afr-self-heal-name.c b/xlators/cluster/afr/src/afr-self-heal-name.c
d1681e
index 556d14b..bcd0e60 100644
d1681e
--- a/xlators/cluster/afr/src/afr-self-heal-name.c
d1681e
+++ b/xlators/cluster/afr/src/afr-self-heal-name.c
d1681e
@@ -19,7 +19,7 @@ __afr_selfheal_assign_gfid (xlator_t *this, inode_t *parent, uuid_t pargfid,
d1681e
                             const char *bname, inode_t *inode,
d1681e
                             struct afr_reply *replies, void *gfid,
d1681e
                             unsigned char *locked_on, int source,
d1681e
-                            gf_boolean_t is_gfid_absent)
d1681e
+                            unsigned char *sources, gf_boolean_t is_gfid_absent)
d1681e
 {
d1681e
 	int             ret          = 0;
d1681e
         int             up_count     = 0;
d1681e
@@ -48,7 +48,7 @@ __afr_selfheal_assign_gfid (xlator_t *this, inode_t *parent, uuid_t pargfid,
d1681e
         }
d1681e
 
d1681e
         afr_lookup_and_heal_gfid (this, parent, bname, inode, replies, source,
d1681e
-                                  gfid);
d1681e
+                                  sources, gfid);
d1681e
 
d1681e
 out:
d1681e
 	return ret;
d1681e
@@ -426,7 +426,7 @@ __afr_selfheal_name_do (call_frame_t *frame, xlator_t *this, inode_t *parent,
d1681e
         is_gfid_absent = (gfid_idx == -1) ? _gf_true : _gf_false;
d1681e
 	ret = __afr_selfheal_assign_gfid (this, parent, pargfid, bname, inode,
d1681e
                                           replies, gfid, locked_on, source,
d1681e
-                                          is_gfid_absent);
d1681e
+                                          sources, is_gfid_absent);
d1681e
         if (ret)
d1681e
                 return ret;
d1681e
 
d1681e
diff --git a/xlators/cluster/afr/src/afr-self-heal.h b/xlators/cluster/afr/src/afr-self-heal.h
d1681e
index b015976..cc99d9e 100644
d1681e
--- a/xlators/cluster/afr/src/afr-self-heal.h
d1681e
+++ b/xlators/cluster/afr/src/afr-self-heal.h
d1681e
@@ -113,7 +113,7 @@ afr_selfheal_entry (call_frame_t *frame, xlator_t *this, inode_t *inode);
d1681e
 int
d1681e
 afr_lookup_and_heal_gfid (xlator_t *this, inode_t *parent, const char *name,
d1681e
                           inode_t *inode, struct afr_reply *replies, int source,
d1681e
-                          void *gfid);
d1681e
+                          unsigned char *sources, void *gfid);
d1681e
 
d1681e
 int
d1681e
 afr_selfheal_inodelk (call_frame_t *frame, xlator_t *this, inode_t *inode,
d1681e
@@ -354,4 +354,8 @@ afr_mark_source_sinks_if_file_empty (xlator_t *this, unsigned char *sources,
d1681e
                                      struct afr_reply *replies,
d1681e
                                      afr_transaction_type type);
d1681e
 
d1681e
+gf_boolean_t
d1681e
+afr_is_file_empty_on_all_children (afr_private_t *priv,
d1681e
+                                   struct afr_reply *replies);
d1681e
+
d1681e
 #endif /* !_AFR_SELFHEAL_H */
d1681e
-- 
d1681e
1.8.3.1
d1681e