233933
From c2b1c50f06cc59b47c9c834617dff2aed7177a78 Mon Sep 17 00:00:00 2001
233933
From: Ashish Pandey <aspandey@redhat.com>
233933
Date: Mon, 18 Mar 2019 12:54:54 +0530
233933
Subject: [PATCH 164/169] cluster/ec: Fix handling of heal info cases without
233933
 locks
233933
233933
When we use heal info command, it takes lot of time as in
233933
some cases it takes lock on entries to find out if the
233933
entry actually needs heal or not.
233933
233933
There are some cases where we can avoid these locks and
233933
can conclude if the entry needs heal or not.
233933
233933
1 - We do a lookup (without lock) on an entry, which we found in
233933
.glusterfs/indices/xattrop, and find that lock count is
233933
zero. Now if the file contains dirty bit set on all or any
233933
brick, we can say that this entry needs heal.
233933
233933
2 - If the lock count is one and dirty is greater than 1,
233933
then it also means that some fop had left the dirty bit set
233933
which made the dirty count of current fop (which has taken lock)
233933
more than one. At this point also we can definitely say that
233933
this entry needs heal.
233933
233933
This patch is modifying code to take into consideration above two
233933
points.
233933
It is also changing code to not to call ec_heal_inspect if ec_heal_do
233933
was called from client side heal. Client side heal triggeres heal
233933
only when it is sure that it requires heal.
233933
233933
[We have changed the code to not to call heal for lookup]
233933
233933
Upstream patch -
233933
https://review.gluster.org/#/c/glusterfs/+/22372/
233933
233933
Fixes: bz#1716385
233933
Change-Id: I7f09f0ecd12f65a353297aefd57026fd2bebdf9c
233933
Signed-off-by: Ashish Pandey <aspandey@redhat.com>
233933
Reviewed-on: https://code.engineering.redhat.com/gerrit/172579
233933
Reviewed-by: Atin Mukherjee <amukherj@redhat.com>
233933
Tested-by: RHGS Build Bot <nigelb@redhat.com>
233933
---
233933
 xlators/cluster/ec/src/ec-heal.c | 42 ++++++++++++++++------------------------
233933
 1 file changed, 17 insertions(+), 25 deletions(-)
233933
233933
diff --git a/xlators/cluster/ec/src/ec-heal.c b/xlators/cluster/ec/src/ec-heal.c
233933
index 3aa04fb..2fa1f11 100644
233933
--- a/xlators/cluster/ec/src/ec-heal.c
233933
+++ b/xlators/cluster/ec/src/ec-heal.c
233933
@@ -2541,13 +2541,15 @@ ec_heal_do(xlator_t *this, void *data, loc_t *loc, int32_t partial)
233933
 
233933
     /* Mount triggers heal only when it detects that it must need heal, shd
233933
      * triggers heals periodically which need not be thorough*/
233933
-    ec_heal_inspect(frame, ec, loc->inode, up_subvols, _gf_false,
233933
-                    !ec->shd.iamshd, &need_heal);
233933
+    if (ec->shd.iamshd) {
233933
+        ec_heal_inspect(frame, ec, loc->inode, up_subvols, _gf_false, _gf_false,
233933
+                        &need_heal);
233933
 
233933
-    if (need_heal == EC_HEAL_NONEED) {
233933
-        gf_msg(ec->xl->name, GF_LOG_DEBUG, 0, EC_MSG_HEAL_FAIL,
233933
-               "Heal is not required for : %s ", uuid_utoa(loc->gfid));
233933
-        goto out;
233933
+        if (need_heal == EC_HEAL_NONEED) {
233933
+            gf_msg(ec->xl->name, GF_LOG_DEBUG, 0, EC_MSG_HEAL_FAIL,
233933
+                   "Heal is not required for : %s ", uuid_utoa(loc->gfid));
233933
+            goto out;
233933
+        }
233933
     }
233933
     sources = alloca0(ec->nodes);
233933
     healed_sinks = alloca0(ec->nodes);
233933
@@ -2902,7 +2904,7 @@ out:
233933
 static int32_t
233933
 _need_heal_calculate(ec_t *ec, uint64_t *dirty, unsigned char *sources,
233933
                      gf_boolean_t self_locked, int32_t lock_count,
233933
-                     ec_heal_need_t *need_heal)
233933
+                     ec_heal_need_t *need_heal, uint64_t *versions)
233933
 {
233933
     int i = 0;
233933
     int source_count = 0;
233933
@@ -2912,7 +2914,7 @@ _need_heal_calculate(ec_t *ec, uint64_t *dirty, unsigned char *sources,
233933
         *need_heal = EC_HEAL_NONEED;
233933
         if (self_locked || lock_count == 0) {
233933
             for (i = 0; i < ec->nodes; i++) {
233933
-                if (dirty[i]) {
233933
+                if (dirty[i] || (versions[i] != versions[0])) {
233933
                     *need_heal = EC_HEAL_MUST;
233933
                     goto out;
233933
                 }
233933
@@ -2928,6 +2930,9 @@ _need_heal_calculate(ec_t *ec, uint64_t *dirty, unsigned char *sources,
233933
                     *need_heal = EC_HEAL_MUST;
233933
                     goto out;
233933
                 }
233933
+                if (dirty[i] != dirty[0] || (versions[i] != versions[0])) {
233933
+                    *need_heal = EC_HEAL_MAYBE;
233933
+                }
233933
             }
233933
         }
233933
     } else {
233933
@@ -2948,7 +2953,6 @@ ec_need_metadata_heal(ec_t *ec, inode_t *inode, default_args_cbk_t *replies,
233933
     unsigned char *healed_sinks = NULL;
233933
     uint64_t *meta_versions = NULL;
233933
     int ret = 0;
233933
-    int i = 0;
233933
 
233933
     sources = alloca0(ec->nodes);
233933
     healed_sinks = alloca0(ec->nodes);
233933
@@ -2961,15 +2965,7 @@ ec_need_metadata_heal(ec_t *ec, inode_t *inode, default_args_cbk_t *replies,
233933
     }
233933
 
233933
     ret = _need_heal_calculate(ec, dirty, sources, self_locked, lock_count,
233933
-                               need_heal);
233933
-    if (ret == ec->nodes && *need_heal == EC_HEAL_NONEED) {
233933
-        for (i = 1; i < ec->nodes; i++) {
233933
-            if (meta_versions[i] != meta_versions[0]) {
233933
-                *need_heal = EC_HEAL_MUST;
233933
-                goto out;
233933
-            }
233933
-        }
233933
-    }
233933
+                               need_heal, meta_versions);
233933
 out:
233933
     return ret;
233933
 }
233933
@@ -3005,7 +3001,7 @@ ec_need_data_heal(ec_t *ec, inode_t *inode, default_args_cbk_t *replies,
233933
     }
233933
 
233933
     ret = _need_heal_calculate(ec, dirty, sources, self_locked, lock_count,
233933
-                               need_heal);
233933
+                               need_heal, data_versions);
233933
 out:
233933
     return ret;
233933
 }
233933
@@ -3033,7 +3029,7 @@ ec_need_entry_heal(ec_t *ec, inode_t *inode, default_args_cbk_t *replies,
233933
     }
233933
 
233933
     ret = _need_heal_calculate(ec, dirty, sources, self_locked, lock_count,
233933
-                               need_heal);
233933
+                               need_heal, data_versions);
233933
 out:
233933
     return ret;
233933
 }
233933
@@ -3131,10 +3127,6 @@ ec_heal_inspect(call_frame_t *frame, ec_t *ec, inode_t *inode,
233933
 need_heal:
233933
     ret = ec_need_heal(ec, inode, replies, lock_count, self_locked, thorough,
233933
                        need_heal);
233933
-
233933
-    if (!self_locked && *need_heal == EC_HEAL_MUST) {
233933
-        *need_heal = EC_HEAL_MAYBE;
233933
-    }
233933
 out:
233933
     cluster_replies_wipe(replies, ec->nodes);
233933
     loc_wipe(&loc;;
233933
@@ -3220,7 +3212,7 @@ ec_get_heal_info(xlator_t *this, loc_t *entry_loc, dict_t **dict_rsp)
233933
 
233933
     ret = ec_heal_inspect(frame, ec, loc.inode, up_subvols, _gf_false,
233933
                           _gf_false, &need_heal);
233933
-    if (ret == ec->nodes && need_heal == EC_HEAL_NONEED) {
233933
+    if (ret == ec->nodes && need_heal != EC_HEAL_MAYBE) {
233933
         goto set_heal;
233933
     }
233933
     need_heal = EC_HEAL_NONEED;
233933
-- 
233933
1.8.3.1
233933