d1681e
From 9ac423d71b9d9d8875ce2af29e2bcf5770b61d5a Mon Sep 17 00:00:00 2001
d1681e
From: Raghavendra G <rgowdapp@redhat.com>
d1681e
Date: Fri, 16 Mar 2018 12:16:43 +0530
d1681e
Subject: [PATCH 258/260] cluster/dht: log error only if layout healing is
d1681e
 required
d1681e
d1681e
selfhealing of directory is invoked on two conditions:
d1681e
1. no layout on disk or layout has some anomalies (holes/overlaps)
d1681e
2. mds xattr is not set on the directory
d1681e
d1681e
When dht_selfheal_directory is called with a correct layout just to
d1681e
set mds xattr, we see error msgs complaining about "not able to form
d1681e
layout on directory", which is misleading as the layout is
d1681e
correct. So, log this msg only if layout has anomalies.
d1681e
d1681e
>Change-Id: I4af25246fc3a2450c2426e9902d1a5b372eab125
d1681e
>updates: bz#1543279
d1681e
>BUG: 1543279
d1681e
>Signed-off-by: Raghavendra G <rgowdapp@redhat.com>
d1681e
d1681e
upstream patch: https://review.gluster.org/19727
d1681e
BUG: 1567100
d1681e
Change-Id: I169823ec3d4455c43f575e6c4ec0735aceb54e6b
d1681e
Signed-off-by: Raghavendra G <rgowdapp@redhat.com>
d1681e
Reviewed-on: https://code.engineering.redhat.com/gerrit/138149
d1681e
Tested-by: RHGS Build Bot <nigelb@redhat.com>
d1681e
Reviewed-by: Sunil Kumar Heggodu Gopala Acharya <sheggodu@redhat.com>
d1681e
---
d1681e
 xlators/cluster/dht/src/dht-common.h   |   1 +
d1681e
 xlators/cluster/dht/src/dht-selfheal.c | 199 ++-------------------------------
d1681e
 2 files changed, 12 insertions(+), 188 deletions(-)
d1681e
d1681e
diff --git a/xlators/cluster/dht/src/dht-common.h b/xlators/cluster/dht/src/dht-common.h
d1681e
index a785876..9d429af 100644
d1681e
--- a/xlators/cluster/dht/src/dht-common.h
d1681e
+++ b/xlators/cluster/dht/src/dht-common.h
d1681e
@@ -312,6 +312,7 @@ struct dht_local {
d1681e
                 uint32_t                overlaps_cnt;
d1681e
                 uint32_t                down;
d1681e
                 uint32_t                misc;
d1681e
+                uint32_t                missing_cnt;
d1681e
                 dht_selfheal_dir_cbk_t  dir_cbk;
d1681e
                 dht_selfheal_layout_t   healer;
d1681e
                 dht_need_heal_t         should_heal;
d1681e
diff --git a/xlators/cluster/dht/src/dht-selfheal.c b/xlators/cluster/dht/src/dht-selfheal.c
d1681e
index 53c59a2..5812f12 100644
d1681e
--- a/xlators/cluster/dht/src/dht-selfheal.c
d1681e
+++ b/xlators/cluster/dht/src/dht-selfheal.c
d1681e
@@ -359,7 +359,8 @@ dht_should_heal_layout (call_frame_t *frame, dht_layout_t **heal,
d1681e
         ret = dht_layout_anomalies (frame->this, &local->loc, *ondisk,
d1681e
                                     &local->selfheal.hole_cnt,
d1681e
                                     &local->selfheal.overlaps_cnt,
d1681e
-                                    NULL, &local->selfheal.down,
d1681e
+                                    &local->selfheal.missing_cnt,
d1681e
+                                    &local->selfheal.down,
d1681e
                                     &local->selfheal.misc, NULL);
d1681e
 
d1681e
         if (ret < 0)
d1681e
@@ -1015,115 +1016,6 @@ dht_layout_index_from_conf (dht_layout_t *layout, xlator_t *xlator)
d1681e
         return i;
d1681e
 }
d1681e
 
d1681e
-
d1681e
-static int
d1681e
-dht_selfheal_dir_xattr_for_nameless_lookup (call_frame_t *frame, loc_t *loc,
d1681e
-                                            dht_layout_t  *layout)
d1681e
-{
d1681e
-        dht_local_t     *local = NULL;
d1681e
-        int             missing_xattr = 0;
d1681e
-        int             i = 0;
d1681e
-        xlator_t        *this = NULL;
d1681e
-        dht_conf_t      *conf = NULL;
d1681e
-        dht_layout_t    *dummy = NULL;
d1681e
-        int             j = 0;
d1681e
-
d1681e
-        local = frame->local;
d1681e
-        this = frame->this;
d1681e
-        conf = this->private;
d1681e
-
d1681e
-        for (i = 0; i < layout->cnt; i++) {
d1681e
-                if (layout->list[i].err != -1 || !layout->list[i].stop) {
d1681e
-                        /* err != -1 would mean xattr present on the directory
d1681e
-                           or the directory is non existent.
d1681e
-                           !layout->list[i].stop would mean layout absent
d1681e
-                        */
d1681e
-
d1681e
-                        continue;
d1681e
-                }
d1681e
-                missing_xattr++;
d1681e
-        }
d1681e
-
d1681e
-        /* Also account for subvolumes with no-layout. Used for zero'ing out
d1681e
-           the layouts and for setting quota key's if present */
d1681e
-
d1681e
-        /* Send  where either the subvol is not part of layout,
d1681e
-         * or it is part of the layout but error is non-zero but error
d1681e
-         * is not equal to -1 or ENOENT.
d1681e
-         */
d1681e
-
d1681e
-        for (i = 0; i < conf->subvolume_cnt; i++) {
d1681e
-                if (dht_is_subvol_part_of_layout (layout, conf->subvolumes[i])
d1681e
-                    == _gf_false) {
d1681e
-                        missing_xattr++;
d1681e
-                        continue;
d1681e
-                }
d1681e
-
d1681e
-                j = dht_layout_index_from_conf (layout, conf->subvolumes[i]);
d1681e
-
d1681e
-                if ((j != -1) && (layout->list[j].err != -1) &&
d1681e
-                   (layout->list[j].err != 0) &&
d1681e
-                   (layout->list[j].err != ENOENT)) {
d1681e
-                        missing_xattr++;
d1681e
-                }
d1681e
-
d1681e
-        }
d1681e
-
d1681e
-
d1681e
-        gf_msg_trace (this->name, 0,
d1681e
-                      "%d subvolumes missing xattr for %s",
d1681e
-                      missing_xattr, loc->path);
d1681e
-
d1681e
-        if (missing_xattr == 0) {
d1681e
-                dht_selfheal_dir_finish (frame, this, 0, 1);
d1681e
-                return 0;
d1681e
-        }
d1681e
-
d1681e
-        local->call_cnt = missing_xattr;
d1681e
-
d1681e
-        if (gf_log_get_loglevel () >= GF_LOG_DEBUG)
d1681e
-                dht_log_new_layout_for_dir_selfheal (this, loc, layout);
d1681e
-
d1681e
-        for (i = 0; i < layout->cnt; i++) {
d1681e
-                if (layout->list[i].err != -1 || !layout->list[i].stop)
d1681e
-                        continue;
d1681e
-
d1681e
-                dht_selfheal_dir_xattr_persubvol (frame, loc, layout, i, NULL);
d1681e
-
d1681e
-                if (--missing_xattr == 0)
d1681e
-                        break;
d1681e
-        }
d1681e
-
d1681e
-        dummy = dht_layout_new (this, 1);
d1681e
-        if (!dummy)
d1681e
-                goto out;
d1681e
-
d1681e
-        for (i = 0; i < conf->subvolume_cnt && missing_xattr; i++) {
d1681e
-              if (dht_is_subvol_part_of_layout (layout, conf->subvolumes[i])
d1681e
-                  == _gf_false) {
d1681e
-                        dht_selfheal_dir_xattr_persubvol (frame, loc, dummy, 0,
d1681e
-                                                          conf->subvolumes[i]);
d1681e
-                        missing_xattr--;
d1681e
-                        continue;
d1681e
-              }
d1681e
-
d1681e
-                j = dht_layout_index_from_conf (layout, conf->subvolumes[i]);
d1681e
-
d1681e
-                if ((j != -1) && (layout->list[j].err != -1) &&
d1681e
-                    (layout->list[j].err != ENOENT) &&
d1681e
-                    (layout->list[j].err != 0)) {
d1681e
-                        dht_selfheal_dir_xattr_persubvol (frame, loc, dummy, 0,
d1681e
-                                                          conf->subvolumes[i]);
d1681e
-                        missing_xattr--;
d1681e
-                }
d1681e
-        }
d1681e
-
d1681e
-        dht_layout_unref (this, dummy);
d1681e
-out:
d1681e
-        return 0;
d1681e
-
d1681e
-}
d1681e
-
d1681e
 int
d1681e
 dht_selfheal_dir_setattr_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
d1681e
                               int op_ret, int op_errno, struct iatt *statpre,
d1681e
@@ -2296,7 +2188,8 @@ dht_selfheal_directory (call_frame_t *frame, dht_selfheal_dir_cbk_t dir_cbk,
d1681e
         dht_layout_anomalies (this, loc, layout,
d1681e
                               &local->selfheal.hole_cnt,
d1681e
                               &local->selfheal.overlaps_cnt,
d1681e
-                              NULL, &local->selfheal.down,
d1681e
+                              &local->selfheal.missing_cnt,
d1681e
+                              &local->selfheal.down,
d1681e
                               &local->selfheal.misc, NULL);
d1681e
 
d1681e
         down     = local->selfheal.down;
d1681e
@@ -2325,14 +2218,14 @@ dht_selfheal_directory (call_frame_t *frame, dht_selfheal_dir_cbk_t dir_cbk,
d1681e
 
d1681e
         dht_layout_sort_volname (layout);
d1681e
         local->heal_layout = _gf_true;
d1681e
-        ret = dht_selfheal_dir_getafix (frame, loc, layout);
d1681e
 
d1681e
-        if (ret == -1) {
d1681e
-                gf_msg (this->name, GF_LOG_INFO, 0,
d1681e
-                        DHT_MSG_DIR_SELFHEAL_FAILED,
d1681e
-                        "Directory selfheal failed: "
d1681e
-                        "Unable to form layout for directory %s",
d1681e
-                        loc->path);
d1681e
+        /* Ignore return value as it can be inferred from result of
d1681e
+         * dht_layout_anomalies
d1681e
+         */
d1681e
+        dht_selfheal_dir_getafix (frame, loc, layout);
d1681e
+
d1681e
+        if (!(local->selfheal.hole_cnt || local->selfheal.overlaps_cnt ||
d1681e
+              local->selfheal.missing_cnt)) {
d1681e
                 local->heal_layout = _gf_false;
d1681e
         }
d1681e
 
d1681e
@@ -2352,76 +2245,6 @@ sorry_no_fix:
d1681e
 }
d1681e
 
d1681e
 int
d1681e
-dht_selfheal_directory_for_nameless_lookup (call_frame_t *frame,
d1681e
-                                            dht_selfheal_dir_cbk_t dir_cbk,
d1681e
-                                            loc_t *loc, dht_layout_t *layout)
d1681e
-{
d1681e
-        dht_local_t     *local  = NULL;
d1681e
-        uint32_t        down    = 0;
d1681e
-        uint32_t        misc    = 0;
d1681e
-        int             ret     = 0;
d1681e
-        xlator_t        *this   = NULL;
d1681e
-
d1681e
-        local = frame->local;
d1681e
-        this = frame->this;
d1681e
-        dht_layout_anomalies (this, loc, layout,
d1681e
-                              &local->selfheal.hole_cnt,
d1681e
-                              &local->selfheal.overlaps_cnt,
d1681e
-                              NULL, &local->selfheal.down,
d1681e
-                              &local->selfheal.misc, NULL);
d1681e
-
d1681e
-        down     = local->selfheal.down;
d1681e
-        misc     = local->selfheal.misc;
d1681e
-
d1681e
-        local->selfheal.dir_cbk = dir_cbk;
d1681e
-        local->selfheal.layout = dht_layout_ref (this, layout);
d1681e
-
d1681e
-        if (down) {
d1681e
-                gf_msg (this->name, GF_LOG_WARNING, 0,
d1681e
-                        DHT_MSG_SUBVOL_DOWN_ERROR,
d1681e
-                        "%d subvolumes down -- not fixing", down);
d1681e
-                ret = 0;
d1681e
-                goto sorry_no_fix;
d1681e
-        }
d1681e
-
d1681e
-        if (misc) {
d1681e
-                gf_msg (this->name, GF_LOG_WARNING, 0,
d1681e
-                        DHT_MSG_SUBVOL_ERROR,
d1681e
-                        "%d subvolumes have unrecoverable errors", misc);
d1681e
-                ret = 0;
d1681e
-                goto sorry_no_fix;
d1681e
-        }
d1681e
-
d1681e
-        dht_layout_sort_volname (layout);
d1681e
-        ret = dht_selfheal_dir_getafix (frame, loc, layout);
d1681e
-
d1681e
-        if (ret == -1) {
d1681e
-                gf_msg (this->name, GF_LOG_WARNING, 0,
d1681e
-                        DHT_MSG_LAYOUT_FORM_FAILED,
d1681e
-                        "not able to form layout for the directory");
d1681e
-                goto sorry_no_fix;
d1681e
-        }
d1681e
-
d1681e
-        ret = dht_selfheal_layout_lock (frame, layout, _gf_false,
d1681e
-                                     dht_selfheal_dir_xattr_for_nameless_lookup,
d1681e
-                                        dht_should_heal_layout);
d1681e
-
d1681e
-        if (ret < 0) {
d1681e
-                goto sorry_no_fix;
d1681e
-        }
d1681e
-
d1681e
-        return 0;
d1681e
-
d1681e
-sorry_no_fix:
d1681e
-        /* TODO: need to put appropriate local->op_errno */
d1681e
-        dht_selfheal_dir_finish (frame, this, ret, 1);
d1681e
-
d1681e
-        return 0;
d1681e
-
d1681e
-
d1681e
-}
d1681e
-
d1681e
-int
d1681e
 dht_selfheal_restore (call_frame_t *frame, dht_selfheal_dir_cbk_t dir_cbk,
d1681e
                       loc_t *loc, dht_layout_t *layout)
d1681e
 {
d1681e
-- 
d1681e
1.8.3.1
d1681e