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