Bug 1483402

+++ This bug was initially created as a clone of Bug #1248393 +++

Description of problem:
readdirp sometimes fails to reads few directory entries.


Version-Release number of selected component (if applicable):


How reproducible:


Steps to Reproduce:
1.
2.
3.

Actual results:


Expected results:


Additional info:

RCA
readdirp only reads entries if they are present on hashed subvol. 

1) During a parallel lookup and rebalance, if the fix-layout on parent directory changes the hashed subvol of its child directory, and a lookup at that time on parent directory would not read the child directory.

2) Due to some anomalies a directory may not be present on the hashed subvol. Hence this entry will not be read.

3) During a rebalance if the first_up_subvol is the newly added brick, it may not have all directories. Again few directories are not read.



--- Additional comment from Worker Ant on 2017-08-16 04:08:57 EDT ---

REVIEW: https://review.gluster.org/18045 (cluster/dht: Reorder dir operations in gf_defrag_fix_layout) posted (#1) for review on master by N Balachandran (nbalacha)

--- Additional comment from Nithya Balachandran on 2017-08-16 04:22:32 EDT ---

This issue is also tracked in https://github.com/gluster/glusterfs/issues/239


For directory structure dir-1/dir-2/dir-3
Earlier behaviour:

gf_defrag_fix_layout (mydir):
  lookup mydir
  process mydir (migrate files)
  for every directory d in mydir
     setxattr (fix-layout, d)  <-- calculates new layout and saves it to disk. At this point, any child dirs of 'd' which hash to the newly added bricks will not be listed by a dht_readdirp on 'd'
     gf_defrag_fix_layout (d)  
     set_commit_hash (d)
  done  
done
   


New behaviour:

gf_defrag_fix_layout (mydir):
  lookup mydir

  for every directory d in mydir
     gf_defrag_fix_layout (d)  
  done

  setxattr (fix-layout, mydir) 
  process mydir (migrate files)
  set_commit_hash (mydir)

done


With this change, all child dirs will be healed before the parent directory's layout is changed so this issue will not exist.

--- Additional comment from Worker Ant on 2017-08-18 04:14:07 EDT ---

REVIEW: https://review.gluster.org/18045 (cluster/dht: Reorder dir operations in gf_defrag_fix_layout) posted (#2) for review on master by N Balachandran (nbalacha)

--- Additional comment from Worker Ant on 2017-08-18 04:50:49 EDT ---

REVIEW: https://review.gluster.org/18045 (cluster/dht: Reorder dir operations in gf_defrag_fix_layout) posted (#3) for review on master by N Balachandran (nbalacha)

--- Additional comment from Worker Ant on 2017-08-18 12:38:00 EDT ---

REVIEW: https://review.gluster.org/18045 (cluster/dht: Reorder dir operations in gf_defrag_fix_layout) posted (#4) for review on master by N Balachandran (nbalacha)

--- Additional comment from Worker Ant on 2017-08-19 22:47:06 EDT ---

COMMIT: https://review.gluster.org/18045 committed in master by Raghavendra G (rgowdapp) 
------
commit 96b33b4b278391ca8a7755cf274931d4f1808cb5
Author: N Balachandran <nbalacha>
Date:   Wed Aug 16 12:58:52 2017 +0530

    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>
    Github issue: #239
    
    Change-Id: I8ae7f24a510754cd8d1b31e5d608bcf1928599e2
    BUG: 1248393
    Signed-off-by: N Balachandran <nbalacha>
    Reviewed-on: https://review.gluster.org/18045
    Smoke: Gluster Build System <jenkins.org>
    CentOS-regression: Gluster Build System <jenkins.org>
    Reviewed-by: Raghavendra G <rgowdapp>