Blob Blame History Raw
From 87138f86b8cb98d1c9d1a4c9a2393e7978d20b1d Mon Sep 17 00:00:00 2001
From: karthik-us <ksubrahm@redhat.com>
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 <pranith.karampuri@phonepe.com>

BUG: 1994593
Change-Id: I29bcc07f2480877a83b30dbd7e2e5631a74df8e8
Signed-off-by: karthik-us <ksubrahm@redhat.com>
Reviewed-on: https://code.engineering.redhat.com/gerrit/c/rhs-glusterfs/+/279606
Tested-by: RHGS Build Bot <nigelb@redhat.com>
Reviewed-by: Sunil Kumar Heggodu Gopala Acharya <sheggodu@redhat.com>
---
 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