14f8ab
From 708c17a8a69b2657f384affaedfcf4ba0a123893 Mon Sep 17 00:00:00 2001
14f8ab
From: karthik-us <ksubrahm@redhat.com>
14f8ab
Date: Wed, 23 Dec 2020 14:45:07 +0530
14f8ab
Subject: [PATCH 513/517] afr: mark pending xattrs as a part of metadata heal
14f8ab
14f8ab
...if pending xattrs are zero for all children.
14f8ab
14f8ab
Problem:
14f8ab
If there are no pending xattrs and a metadata heal needs to be
14f8ab
performed, it can be possible that we end up with xattrs inadvertendly
14f8ab
deleted from all bricks, as explained in the  BZ.
14f8ab
14f8ab
Fix:
14f8ab
After picking one among the sources as the good copy, mark pending xattrs on
14f8ab
all sources to blame the sinks. Now even if this metadata heal fails midway,
14f8ab
a subsequent heal will still choose one of the valid sources that it
14f8ab
picked previously.
14f8ab
14f8ab
Upstream patch details:
14f8ab
> Fixes: #1067
14f8ab
> Change-Id: If1b050b70b0ad911e162c04db4d89b263e2b8d7b
14f8ab
> Signed-off-by: Ravishankar N <ravishankar@redhat.com>
14f8ab
Upstream patch: https://review.gluster.org/#/c/glusterfs/+/21922/
14f8ab
14f8ab
BUG: 1640148
14f8ab
Change-Id: If1b050b70b0ad911e162c04db4d89b263e2b8d7b
14f8ab
Signed-off-by: karthik-us <ksubrahm@redhat.com>
14f8ab
Reviewed-on: https://code.engineering.redhat.com/gerrit/222073
14f8ab
Tested-by: RHGS Build Bot <nigelb@redhat.com>
14f8ab
Reviewed-by: Ravishankar Narayanankutty <ravishankar@redhat.com>
14f8ab
---
14f8ab
 tests/bugs/replicate/mdata-heal-no-xattrs.t      | 59 ++++++++++++++++++++++
14f8ab
 xlators/cluster/afr/src/afr-self-heal-metadata.c | 62 +++++++++++++++++++++++-
14f8ab
 2 files changed, 120 insertions(+), 1 deletion(-)
14f8ab
 create mode 100644 tests/bugs/replicate/mdata-heal-no-xattrs.t
14f8ab
14f8ab
diff --git a/tests/bugs/replicate/mdata-heal-no-xattrs.t b/tests/bugs/replicate/mdata-heal-no-xattrs.t
14f8ab
new file mode 100644
14f8ab
index 0000000..d3b0c50
14f8ab
--- /dev/null
14f8ab
+++ b/tests/bugs/replicate/mdata-heal-no-xattrs.t
14f8ab
@@ -0,0 +1,59 @@
14f8ab
+#!/bin/bash
14f8ab
+
14f8ab
+. $(dirname $0)/../../include.rc
14f8ab
+. $(dirname $0)/../../volume.rc
14f8ab
+cleanup;
14f8ab
+
14f8ab
+TEST glusterd
14f8ab
+TEST pidof glusterd
14f8ab
+TEST $CLI volume create $V0 replica 3 $H0:$B0/${V0}{0,1,2};
14f8ab
+TEST $CLI volume set $V0 cluster.self-heal-daemon off
14f8ab
+TEST $CLI volume start $V0
14f8ab
+
14f8ab
+TEST glusterfs --volfile-id=/$V0 --volfile-server=$H0 $M0 --attribute-timeout=0 --entry-timeout=0
14f8ab
+EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" afr_child_up_status $V0 0
14f8ab
+EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" afr_child_up_status $V0 1
14f8ab
+EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" afr_child_up_status $V0 2
14f8ab
+echo "Data">$M0/FILE
14f8ab
+ret=$?
14f8ab
+TEST [ $ret -eq 0 ]
14f8ab
+
14f8ab
+# Change permission on brick-0: simulates the case where there is metadata
14f8ab
+# mismatch but no pending xattrs. This brick will become the source for heal.
14f8ab
+TEST chmod +x $B0/$V0"0"/FILE
14f8ab
+
14f8ab
+# Add gfid to xattrop
14f8ab
+xattrop_b0=$(afr_get_index_path $B0/$V0"0")
14f8ab
+base_entry_b0=`ls $xattrop_b0`
14f8ab
+gfid_str_FILE=$(gf_gfid_xattr_to_str $(gf_get_gfid_xattr $B0/$V0"0"/FILE))
14f8ab
+TEST ln $xattrop_b0/$base_entry_b0 $xattrop_b0/$gfid_str_FILE
14f8ab
+EXPECT_WITHIN $HEAL_TIMEOUT "^1$" get_pending_heal_count $V0
14f8ab
+
14f8ab
+TEST $CLI volume set $V0 cluster.self-heal-daemon on
14f8ab
+EXPECT_WITHIN $CHILD_UP_TIMEOUT "1" afr_child_up_status_in_shd $V0 0
14f8ab
+EXPECT_WITHIN $CHILD_UP_TIMEOUT "1" afr_child_up_status_in_shd $V0 1
14f8ab
+EXPECT_WITHIN $CHILD_UP_TIMEOUT "1" afr_child_up_status_in_shd $V0 2
14f8ab
+TEST $CLI volume heal $V0
14f8ab
+EXPECT_WITHIN $HEAL_TIMEOUT "^0$" get_pending_heal_count $V0
14f8ab
+
14f8ab
+# Brick-0 should contain xattrs blaming other 2 bricks.
14f8ab
+# The values will be zero because heal is over.
14f8ab
+EXPECT "000000000000000000000000" get_hex_xattr trusted.afr.$V0-client-1 $B0/${V0}0/FILE
14f8ab
+EXPECT "000000000000000000000000" get_hex_xattr trusted.afr.$V0-client-2 $B0/${V0}0/FILE
14f8ab
+TEST ! getfattr -n trusted.afr.$V0-client-0 $B0/${V0}0/FILE
14f8ab
+
14f8ab
+# Brick-1 and Brick-2 must not contain any afr xattrs.
14f8ab
+TEST ! getfattr -n trusted.afr.$V0-client-0 $B0/${V0}1/FILE
14f8ab
+TEST ! getfattr -n trusted.afr.$V0-client-1 $B0/${V0}1/FILE
14f8ab
+TEST ! getfattr -n trusted.afr.$V0-client-2 $B0/${V0}1/FILE
14f8ab
+TEST ! getfattr -n trusted.afr.$V0-client-0 $B0/${V0}2/FILE
14f8ab
+TEST ! getfattr -n trusted.afr.$V0-client-1 $B0/${V0}2/FILE
14f8ab
+TEST ! getfattr -n trusted.afr.$V0-client-2 $B0/${V0}2/FILE
14f8ab
+
14f8ab
+# check permission bits.
14f8ab
+EXPECT '755' stat -c %a $B0/${V0}0/FILE
14f8ab
+EXPECT '755' stat -c %a $B0/${V0}1/FILE
14f8ab
+EXPECT '755' stat -c %a $B0/${V0}2/FILE
14f8ab
+
14f8ab
+EXPECT_WITHIN $UMOUNT_TIMEOUT "Y" force_umount $M0
14f8ab
+cleanup;
14f8ab
diff --git a/xlators/cluster/afr/src/afr-self-heal-metadata.c b/xlators/cluster/afr/src/afr-self-heal-metadata.c
14f8ab
index f4e31b6..03f43ba 100644
14f8ab
--- a/xlators/cluster/afr/src/afr-self-heal-metadata.c
14f8ab
+++ b/xlators/cluster/afr/src/afr-self-heal-metadata.c
14f8ab
@@ -190,6 +190,59 @@ out:
14f8ab
     return ret;
14f8ab
 }
14f8ab
 
14f8ab
+static int
14f8ab
+__afr_selfheal_metadata_mark_pending_xattrs(call_frame_t *frame, xlator_t *this,
14f8ab
+                                            inode_t *inode,
14f8ab
+                                            struct afr_reply *replies,
14f8ab
+                                            unsigned char *sources)
14f8ab
+{
14f8ab
+    int ret = 0;
14f8ab
+    int i = 0;
14f8ab
+    int m_idx = 0;
14f8ab
+    afr_private_t *priv = NULL;
14f8ab
+    int raw[AFR_NUM_CHANGE_LOGS] = {0};
14f8ab
+    dict_t *xattr = NULL;
14f8ab
+
14f8ab
+    priv = this->private;
14f8ab
+    m_idx = afr_index_for_transaction_type(AFR_METADATA_TRANSACTION);
14f8ab
+    raw[m_idx] = 1;
14f8ab
+
14f8ab
+    xattr = dict_new();
14f8ab
+    if (!xattr)
14f8ab
+        return -ENOMEM;
14f8ab
+
14f8ab
+    for (i = 0; i < priv->child_count; i++) {
14f8ab
+        if (sources[i])
14f8ab
+            continue;
14f8ab
+        ret = dict_set_static_bin(xattr, priv->pending_key[i], raw,
14f8ab
+                                  sizeof(int) * AFR_NUM_CHANGE_LOGS);
14f8ab
+        if (ret) {
14f8ab
+            ret = -1;
14f8ab
+            goto out;
14f8ab
+        }
14f8ab
+    }
14f8ab
+
14f8ab
+    for (i = 0; i < priv->child_count; i++) {
14f8ab
+        if (!sources[i])
14f8ab
+            continue;
14f8ab
+        ret = afr_selfheal_post_op(frame, this, inode, i, xattr, NULL);
14f8ab
+        if (ret < 0) {
14f8ab
+            gf_msg(this->name, GF_LOG_INFO, -ret, AFR_MSG_SELF_HEAL_INFO,
14f8ab
+                   "Failed to set pending metadata xattr on child %d for %s", i,
14f8ab
+                   uuid_utoa(inode->gfid));
14f8ab
+            goto out;
14f8ab
+        }
14f8ab
+    }
14f8ab
+
14f8ab
+    afr_replies_wipe(replies, priv->child_count);
14f8ab
+    ret = afr_selfheal_unlocked_discover(frame, inode, inode->gfid, replies);
14f8ab
+
14f8ab
+out:
14f8ab
+    if (xattr)
14f8ab
+        dict_unref(xattr);
14f8ab
+    return ret;
14f8ab
+}
14f8ab
+
14f8ab
 /*
14f8ab
  * Look for mismatching uid/gid or mode or user xattrs even if
14f8ab
  * AFR xattrs don't say so, and pick one arbitrarily as winner. */
14f8ab
@@ -210,6 +263,7 @@ __afr_selfheal_metadata_finalize_source(call_frame_t *frame, xlator_t *this,
14f8ab
     };
14f8ab
     int source = -1;
14f8ab
     int sources_count = 0;
14f8ab
+    int ret = 0;
14f8ab
 
14f8ab
     priv = this->private;
14f8ab
 
14f8ab
@@ -300,7 +354,13 @@ __afr_selfheal_metadata_finalize_source(call_frame_t *frame, xlator_t *this,
14f8ab
             healed_sinks[i] = 1;
14f8ab
         }
14f8ab
     }
14f8ab
-
14f8ab
+    if ((sources_count == priv->child_count) && (source > -1) &&
14f8ab
+        (AFR_COUNT(healed_sinks, priv->child_count) != 0)) {
14f8ab
+        ret = __afr_selfheal_metadata_mark_pending_xattrs(frame, this, inode,
14f8ab
+                                                          replies, sources);
14f8ab
+        if (ret < 0)
14f8ab
+            return ret;
14f8ab
+    }
14f8ab
 out:
14f8ab
     afr_mark_active_sinks(this, sources, locked_on, healed_sinks);
14f8ab
     return source;
14f8ab
-- 
14f8ab
1.8.3.1
14f8ab