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