cluster/dht: Reorder dir operations in gf_defrag_fix_layout

Earlier, rebalance performed a fix-layout on a directory
before healing its subdirectories. If there were a lot of
subdirs, it could take a while before all subdirs were
created on the newly added bricks. As dht_readdirp only lists
dirs from their hashed subvol, those dirs which hashed to
the newly added bricks but were not yet created on them were
not listed.

Now, the child dirs are listed and processed before the layout
of the parent is fixed. This introduces a change in behaviour
where files in subdirs are migrated before those in parent
directories.

Credit: Shyam <srangana@redhat.com>
Github issue: #239

> BUG: 1248393
> Signed-off-by: N Balachandran <nbalacha@redhat.com>
> Reviewed-on: https://review.gluster.org/18045
> Smoke: Gluster Build System <jenkins@build.gluster.org>
> CentOS-regression: Gluster Build System <jenkins@build.gluster.org>
> Reviewed-by: Raghavendra G <rgowdapp@redhat.com>
(cherry picked from commit 96b33b4b278391ca8a7755cf274931d4f1808cb5)
Change-Id: I8ae7f24a510754cd8d1b31e5d608bcf1928599e2
BUG: 1483402
Signed-off-by: N Balachandran <nbalacha@redhat.com>
Reviewed-on: https://review.gluster.org/18071
Smoke: Gluster Build System <jenkins@build.gluster.org>
Reviewed-by: Raghavendra G <rgowdapp@redhat.com>
CentOS-regression: Gluster Build System <jenkins@build.gluster.org>

2 files changed, 82 insertions, 79 deletions

diff --git a/tests/features/trash.t b/tests/features/trash.t
index 620b84f0da1..472e909e567 100755
--- a/tests/features/trash.t
+++ b/tests/features/trash.t
@@ -2,6 +2,7 @@
 
 . $(dirname $0)/../include.rc
 . $(dirname $0)/../volume.rc
+. $(dirname $0)/../dht.rc
 
 cleanup
 
@@ -183,13 +184,20 @@ TEST file_exists $V0 rebal1 rebal2
 TEST $CLI volume add-brick $V0 $H0:$B0/${V0}3
 TEST [ -d $B0/${V0}3 ]
 
+
 # perform rebalance [36]
 TEST $CLI volume rebalance $V0 start force
+EXPECT_WITHIN $REBALANCE_TIMEOUT "0" rebalance_completed
+
+#Find out which file was migrated to the new brick
+file_name=$(ls $B0/${V0}3/rebal*| xargs basename)
 
 # check whether rebalance was succesful [37-40]
-EXPECT_WITHIN $REBALANCE_TIMEOUT "Y" wildcard_exists $B0/${V0}3/rebal2
-EXPECT_WITHIN $REBALANCE_TIMEOUT "Y" wildcard_exists $B0/${V0}1/.trashcan/internal_op/rebal2*
+EXPECT "Y" wildcard_exists $B0/${V0}3/$file_name*
+EXPECT "Y" wildcard_exists $B0/${V0}1/.trashcan/internal_op/$file_name*
+
 EXPECT_WITHIN $UMOUNT_TIMEOUT "Y" force_umount $M0
+
 # force required in case rebalance is not over
 TEST $CLI volume stop $V0 force
 
@@ -236,7 +244,7 @@ EXPECT_WITHIN $HEAL_TIMEOUT "Y" wildcard_exists $B0/${V1}1/.trashcan/internal_op
 TEST $CLI volume set $V0 trash-dir abc
 TEST start_vol $V0 $M0 $M0/abc
 TEST [ -e $M0/abc -a ! -e $M0/.trashcan ]
-EXPECT "Y" wildcard_exists $B0/${V0}1/abc/internal_op/rebal2*
+EXPECT "Y" wildcard_exists $B0/${V0}1/abc/internal_op/rebal*
 
 # ensure that rename and delete operation on trash directory fails [63-65]
 rm -rf $M0/abc/internal_op
diff --git a/xlators/cluster/dht/src/dht-rebalance.c b/xlators/cluster/dht/src/dht-rebalance.c
index e20e392ceb0..101946ab386 100644
--- a/xlators/cluster/dht/src/dht-rebalance.c
+++ b/xlators/cluster/dht/src/dht-rebalance.c
@@ -3581,37 +3581,6 @@ gf_defrag_fix_layout (xlator_t *this, gf_defrag_info_t *defrag, loc_t *loc,
                 }
         }
 
-        if ((defrag->cmd != GF_DEFRAG_CMD_START_TIER) &&
-            (defrag->cmd != GF_DEFRAG_CMD_START_LAYOUT_FIX)) {
-                ret = gf_defrag_process_dir (this, defrag, loc, migrate_data,
-                                             &perrno);
-
-                if (ret && (ret != 2)) {
-                        if (perrno == ENOENT || perrno == ESTALE) {
-                                ret = 0;
-                                goto out;
-                        } else {
-
-                                defrag->total_failures++;
-
-                                gf_msg (this->name, GF_LOG_ERROR, 0,
-                                        DHT_MSG_DEFRAG_PROCESS_DIR_FAILED,
-                                        "gf_defrag_process_dir failed for "
-                                        "directory: %s", loc->path);
-
-                                if (conf->decommission_in_progress) {
-                                        goto out;
-                                }
-
-                                should_commit_hash = 0;
-                        }
-                } else if (ret == 2) {
-                        should_commit_hash = 0;
-                }
-        }
-
-        gf_msg_trace (this->name, 0, "fix layout called on %s", loc->path);
-
         fd = fd_create (loc->inode, defrag->pid);
         if (!fd) {
                 gf_log (this->name, GF_LOG_ERROR, "Failed to create fd");
@@ -3635,10 +3604,10 @@ gf_defrag_fix_layout (xlator_t *this, gf_defrag_info_t *defrag, loc_t *loc,
 
         fd_bind (fd);
         INIT_LIST_HEAD (&entries.list);
+
         while ((ret = syncop_readdirp (this, fd, 131072, offset, &entries,
                                        NULL, NULL)) != 0)
         {
-
                 if (ret < 0) {
                         if (-ret == ENOENT || -ret == ESTALE) {
                                 ret = 0;
@@ -3753,36 +3722,6 @@ gf_defrag_fix_layout (xlator_t *this, gf_defrag_info_t *defrag, loc_t *loc,
                                                 DHT_MSG_DIR_LOOKUP_FAILED,
                                                 "lookup failed for:%s",
                                                 entry_loc.path);
-                                        defrag->total_failures++;
-                                        if (conf->decommission_in_progress) {
-                                                defrag->defrag_status =
-                                                GF_DEFRAG_STATUS_FAILED;
-                                                ret = -1;
-                                                goto out;
-                                        } else {
-                                                should_commit_hash = 0;
-                                                continue;
-                                        }
-                                }
-                        }
-
-                        ret = syncop_setxattr (this, &entry_loc, fix_layout,
-                                               0, NULL, NULL);
-                        if (ret) {
-                                if (-ret == ENOENT || -ret == ESTALE) {
-                                        gf_msg (this->name, GF_LOG_INFO, -ret,
-                                                DHT_MSG_LAYOUT_FIX_FAILED,
-                                                "Setxattr failed. Dir %s "
-                                                "renamed or removed",
-                                                entry_loc.path);
-                                        ret = 0;
-                                        continue;
-                                } else {
-
-                                        gf_msg (this->name, GF_LOG_ERROR, -ret,
-                                                DHT_MSG_LAYOUT_FIX_FAILED,
-                                                "Setxattr failed for %s",
-                                                entry_loc.path);
 
                                         defrag->total_failures++;
 
@@ -3792,6 +3731,7 @@ gf_defrag_fix_layout (xlator_t *this, gf_defrag_info_t *defrag, loc_t *loc,
                                                 ret = -1;
                                                 goto out;
                                         } else {
+                                                should_commit_hash = 0;
                                                 continue;
                                         }
                                 }
@@ -3823,30 +3763,85 @@ gf_defrag_fix_layout (xlator_t *this, gf_defrag_info_t *defrag, loc_t *loc,
                                         continue;
                                 }
                         }
+                }
 
-                        if (ret != 2 &&
-                            gf_defrag_settle_hash (this, defrag, &entry_loc,
-                            fix_layout) != 0) {
-                                defrag->total_failures++;
+                gf_dirent_free (&entries);
+                free_entries = _gf_false;
+                INIT_LIST_HEAD (&entries.list);
+        }
 
-                                gf_msg (this->name, GF_LOG_ERROR, 0,
-                                        DHT_MSG_SETTLE_HASH_FAILED,
-                                        "Settle hash failed for %s",
-                                        entry_loc.path);
+        ret = syncop_setxattr (this, loc, fix_layout, 0, NULL, NULL);
+        if (ret) {
+                if (-ret == ENOENT || -ret == ESTALE) {
+                        gf_msg (this->name, GF_LOG_INFO, -ret,
+                                DHT_MSG_LAYOUT_FIX_FAILED,
+                                "Setxattr failed. Dir %s "
+                                "renamed or removed",
+                                loc->path);
+                        ret = 0;
+                } else {
+                        gf_msg (this->name, GF_LOG_ERROR, -ret,
+                                DHT_MSG_LAYOUT_FIX_FAILED,
+                                "Setxattr failed for %s",
+                                loc->path);
+
+                        defrag->total_failures++;
 
+                        if (conf->decommission_in_progress) {
+                                defrag->defrag_status = GF_DEFRAG_STATUS_FAILED;
                                 ret = -1;
+                                goto out;
+                        }
+                 }
+        }
 
-                                if (conf->decommission_in_progress) {
-                                        defrag->defrag_status =
-                                        GF_DEFRAG_STATUS_FAILED;
+        if ((defrag->cmd != GF_DEFRAG_CMD_START_TIER) &&
+            (defrag->cmd != GF_DEFRAG_CMD_START_LAYOUT_FIX)) {
+                ret = gf_defrag_process_dir (this, defrag, loc, migrate_data,
+                                             &perrno);
+
+                if (ret && (ret != 2)) {
+                        if (perrno == ENOENT || perrno == ESTALE) {
+                                ret = 0;
+                                goto out;
+                        } else {
+
+                                defrag->total_failures++;
 
+                                gf_msg (this->name, GF_LOG_ERROR, 0,
+                                        DHT_MSG_DEFRAG_PROCESS_DIR_FAILED,
+                                        "gf_defrag_process_dir failed for "
+                                        "directory: %s", loc->path);
+
+                                if (conf->decommission_in_progress) {
                                         goto out;
                                 }
+
+                                should_commit_hash = 0;
                         }
+                } else if (ret == 2) {
+                        should_commit_hash = 0;
+                }
+        }
+
+        gf_msg_trace (this->name, 0, "fix layout called on %s", loc->path);
+
+        if (should_commit_hash &&
+            gf_defrag_settle_hash (this, defrag, loc, fix_layout) != 0) {
+
+                defrag->total_failures++;
+
+                gf_msg (this->name, GF_LOG_ERROR, 0,
+                        DHT_MSG_SETTLE_HASH_FAILED,
+                        "Settle hash failed for %s",
+                         loc->path);
+
+                ret = -1;
+
+                if (conf->decommission_in_progress) {
+                        defrag->defrag_status = GF_DEFRAG_STATUS_FAILED;
+                        goto out;
                 }
-                gf_dirent_free (&entries);
-                free_entries = _gf_false;
-                INIT_LIST_HEAD (&entries.list);
         }
 
         ret = 0;