From 87138f86b8cb98d1c9d1a4c9a2393e7978d20b1d Mon Sep 17 00:00:00 2001 From: karthik-us Date: Tue, 5 Oct 2021 12:33:01 +0530 Subject: [PATCH 590/610] cluster/afr: Don't check for stale entry-index Problem: In every entry index heal there is a check to see if the index is stale or not. 1. If a file is created when the brick is down this will lead to an extra index lookup because the name is not stale. 2. If a file is deleted when the brick is down this will also lead to and extra index lookup because the name is not stale. 3. If a file is created and deleted when the brick is down then the index is stale and this will save entry-heal i.e. 2 entrylks and 2 lookups Since 1, 2 happen significantly more than 3, this is a bad tradeoff. Fix: Let stale index be removed as part of normal entry heal detecting 'the name is already deleted' code path. > Upstream patch: https://github.com/gluster/glusterfs/pull/2612 > fixes: gluster#2611 > Change-Id: I29bcc07f2480877a83b30dbd7e2e5631a74df8e8 > Signed-off-by: Pranith Kumar K BUG: 1994593 Change-Id: I29bcc07f2480877a83b30dbd7e2e5631a74df8e8 Signed-off-by: karthik-us Reviewed-on: https://code.engineering.redhat.com/gerrit/c/rhs-glusterfs/+/279606 Tested-by: RHGS Build Bot Reviewed-by: Sunil Kumar Heggodu Gopala Acharya --- xlators/cluster/afr/src/afr-self-heal-entry.c | 46 +++++++-------------------- 1 file changed, 11 insertions(+), 35 deletions(-) diff --git a/xlators/cluster/afr/src/afr-self-heal-entry.c b/xlators/cluster/afr/src/afr-self-heal-entry.c index a17dd93..14b7417 100644 --- a/xlators/cluster/afr/src/afr-self-heal-entry.c +++ b/xlators/cluster/afr/src/afr-self-heal-entry.c @@ -933,37 +933,8 @@ afr_selfheal_entry_granular_dirent(xlator_t *subvol, gf_dirent_t *entry, loc_t *parent, void *data) { int ret = 0; - loc_t loc = { - 0, - }; - struct iatt iatt = { - 0, - }; afr_granular_esh_args_t *args = data; - /* Look up the actual inode associated with entry. If the lookup returns - * ESTALE or ENOENT, then it means we have a stale index. Remove it. - * This is analogous to the check in afr_shd_index_heal() except that - * here it is achieved through LOOKUP and in afr_shd_index_heal() through - * a GETXATTR. - */ - - loc.inode = inode_new(args->xl->itable); - loc.parent = inode_ref(args->heal_fd->inode); - gf_uuid_copy(loc.pargfid, loc.parent->gfid); - loc.name = entry->d_name; - - ret = syncop_lookup(args->xl, &loc, &iatt, NULL, NULL, NULL); - if ((ret == -ENOENT) || (ret == -ESTALE)) { - /* The name indices under the pgfid index dir are guaranteed - * to be regular files. Hence the hardcoding. - */ - afr_shd_entry_purge(subvol, parent->inode, entry->d_name, IA_IFREG); - ret = 0; - goto out; - } - /* TBD: afr_shd_zero_xattrop? */ - ret = afr_selfheal_entry_dirent(args->frame, args->xl, args->heal_fd, entry->d_name, parent->inode, subvol, _gf_false); @@ -974,8 +945,6 @@ afr_selfheal_entry_granular_dirent(xlator_t *subvol, gf_dirent_t *entry, if (ret == -1) args->mismatch = _gf_true; -out: - loc_wipe(&loc); return ret; } @@ -1050,7 +1019,9 @@ afr_selfheal_entry_do(call_frame_t *frame, xlator_t *this, fd_t *fd, int source, local = frame->local; gf_msg(this->name, GF_LOG_INFO, 0, AFR_MSG_SELF_HEAL_INFO, - "performing entry selfheal on %s", uuid_utoa(fd->inode->gfid)); + "performing %s entry selfheal on %s", + (local->need_full_crawl ? "full" : "granular"), + uuid_utoa(fd->inode->gfid)); for (i = 0; i < priv->child_count; i++) { /* Expunge */ @@ -1112,6 +1083,7 @@ __afr_selfheal_entry(call_frame_t *frame, xlator_t *this, fd_t *fd, afr_local_t *local = NULL; afr_private_t *priv = NULL; gf_boolean_t did_sh = _gf_true; + char *heal_type = "granular entry"; priv = this->private; local = frame->local; @@ -1194,11 +1166,15 @@ postop_unlock: afr_selfheal_unentrylk(frame, this, fd->inode, this->name, NULL, postop_lock, NULL); out: - if (did_sh) - afr_log_selfheal(fd->inode->gfid, this, ret, "entry", source, sources, + if (did_sh) { + if (local->need_full_crawl) { + heal_type = "full entry"; + } + afr_log_selfheal(fd->inode->gfid, this, ret, heal_type, source, sources, healed_sinks); - else + } else { ret = 1; + } if (locked_replies) afr_replies_wipe(locked_replies, priv->child_count); -- 1.8.3.1