Cleanup ufs descendant sync type

core/server/master/src/main/java/alluxio/master/file/DefaultFileSystemMaster.java

@@ -70,6 +70,7 @@
 import alluxio.master.file.options.CreateDirectoryOptions;
 import alluxio.master.file.options.CreateFileOptions;
 import alluxio.master.file.options.DeleteOptions;
+import alluxio.master.file.options.DescendantType;
 import alluxio.master.file.options.FreeOptions;
 import alluxio.master.file.options.GetStatusOptions;
 import alluxio.master.file.options.ListStatusOptions;
@@ -749,7 +750,7 @@ public FileInfo getFileInfo(AlluxioURI path, GetStatusOptions options)
         throw e;
       }
       // Possible ufs sync.
-      if (syncMetadata(journalContext, inodePath, lockingScheme, 1)) {
+      if (syncMetadata(journalContext, inodePath, lockingScheme, DescendantType.ONE)) {
         // If synced, do not load metadata.
         options.setLoadMetadataType(LoadMetadataType.Never);
       }
@@ -828,24 +829,25 @@ public List<FileInfo> listStatus(AlluxioURI path, ListStatusOptions listStatusOp
         throw e;
       }
       // Possible ufs sync.
-      if (syncMetadata(journalContext, inodePath, lockingScheme, 1)) {
+      if (syncMetadata(journalContext, inodePath, lockingScheme, DescendantType.ONE)) {
         // If synced, do not load metadata.
         listStatusOptions.setLoadMetadataType(LoadMetadataType.Never);
       }
 
       // load metadata for 1 level of descendants
-      int loadDescendantLevels =
-          (listStatusOptions.getLoadMetadataType() != LoadMetadataType.Never) ? 1 : 0;
+      DescendantType loadDescendantType =
+          (listStatusOptions.getLoadMetadataType() != LoadMetadataType.Never) ? DescendantType.ONE :
+              DescendantType.NONE;
       LoadMetadataOptions loadMetadataOptions =
           LoadMetadataOptions.defaults().setCreateAncestors(true)
-              .setLoadDescendantLevels(loadDescendantLevels);
+              .setLoadDescendantType(loadDescendantType);
       Inode<?> inode;
       if (inodePath.fullPathExists()) {
         inode = inodePath.getInode();
         if (inode.isDirectory()
             && listStatusOptions.getLoadMetadataType() != LoadMetadataType.Always
             && ((InodeDirectory) inode).isDirectChildrenLoaded()) {
-          loadMetadataOptions.setLoadDescendantLevels(0);
+          loadMetadataOptions.setLoadDescendantType(DescendantType.NONE);
         }
       } else {
         checkLoadMetadataOptions(listStatusOptions.getLoadMetadataType(), inodePath.getUri());
@@ -946,7 +948,7 @@ public List<AlluxioURI> checkConsistency(AlluxioURI path, CheckConsistencyOption
         throw e;
       }
       // Possible ufs sync.
-      syncMetadata(journalContext, parent, lockingScheme, -1);
+      syncMetadata(journalContext, parent, lockingScheme, DescendantType.ALL);
 
       try (InodeLockList children = mInodeTree.lockDescendants(parent, InodeTree.LockMode.READ)) {
         if (!checkConsistencyInternal(parent.getInode(), parent.getUri())) {
@@ -1170,7 +1172,7 @@ public long createFile(AlluxioURI path, CreateFileOptions options)
         throw e;
       }
       // Possible ufs sync.
-      syncMetadata(journalContext, inodePath, lockingScheme, 1);
+      syncMetadata(journalContext, inodePath, lockingScheme, DescendantType.ONE);
 
       mMountTable.checkUnderWritableMountPoint(path);
       createFileAndJournal(inodePath, options, journalContext);
@@ -1350,7 +1352,8 @@ public void delete(AlluxioURI path, DeleteOptions options) throws IOException,
         mMountTable.checkUnderWritableMountPoint(path);
       }
       // Possible ufs sync.
-      syncMetadata(journalContext, inodePath, lockingScheme, options.isRecursive() ? -1 : 1);
+      syncMetadata(journalContext, inodePath, lockingScheme,
+          options.isRecursive() ? DescendantType.ALL : DescendantType.ONE);
       if (!inodePath.fullPathExists()) {
         throw new FileDoesNotExistException(ExceptionMessage.PATH_DOES_NOT_EXIST.getMessage(path));
       }
@@ -1821,7 +1824,7 @@ public long createDirectory(AlluxioURI path, CreateDirectoryOptions options)
         throw e;
       }
       // Possible ufs sync.
-      syncMetadata(journalContext, inodePath, lockingScheme, 1);
+      syncMetadata(journalContext, inodePath, lockingScheme, DescendantType.ONE);
 
       mMountTable.checkUnderWritableMountPoint(path);
       createDirectoryAndJournal(inodePath, options, journalContext);
@@ -1919,8 +1922,8 @@ public void rename(AlluxioURI srcPath, AlluxioURI dstPath, RenameOptions options
         throw e;
       }
       // Possible ufs sync.
-      syncMetadata(journalContext, srcInodePath, srcLockingScheme, 1);
-      syncMetadata(journalContext, dstInodePath, dstLockingScheme, 1);
+      syncMetadata(journalContext, srcInodePath, srcLockingScheme, DescendantType.ONE);
+      syncMetadata(journalContext, dstInodePath, dstLockingScheme, DescendantType.ONE);
 
       mMountTable.checkUnderWritableMountPoint(srcPath);
       mMountTable.checkUnderWritableMountPoint(dstPath);
@@ -2442,26 +2445,25 @@ private void loadMetadataAndJournal(LockedInodePath inodePath, LoadMetadataOptio
         loadDirectoryMetadataAndJournal(inodePath, options, journalContext);
         InodeDirectory inode = (InodeDirectory) inodePath.getInode();
 
-        if (options.getLoadDescendantLevels() != 0) {
+        if (options.getLoadDescendantType() != DescendantType.NONE) {
           UfsStatus[] children = ufs.listStatus(ufsUri.toString());
           for (UfsStatus childStatus : children) {
-            boolean skipExisting = inode.getChild(childStatus.getName()) != null;
-            // Do not skip existing directories if there are more descendant levels to load
-            if (childStatus.isDirectory() && (options.getLoadDescendantLevels() < 0
-                || options.getLoadDescendantLevels() > 1)) {
-              skipExisting = false;
+            if (PathUtils.isTemporaryFileName(childStatus.getName())) {
+              continue;
             }
-            if (PathUtils.isTemporaryFileName(childStatus.getName()) || skipExisting) {
+            if (inode.getChild(childStatus.getName()) != null && (childStatus.isFile()
+                || options.getLoadDescendantType() != DescendantType.ALL)) {
+              // stop traversing if this is an existing file, or an existing directory without
+              // loading all descendants.
               continue;
             }
             TempInodePathForChild tempInodePath =
                 new TempInodePathForChild(inodePath, childStatus.getName());
-            int loadDescendantLevels = options.getLoadDescendantLevels();
-            if (loadDescendantLevels > 0) {
-              loadDescendantLevels--;
-            }
+            DescendantType loadDescendantType =
+                (options.getLoadDescendantType() == DescendantType.ONE) ? DescendantType.NONE :
+                    DescendantType.ALL;
             LoadMetadataOptions loadMetadataOptions =
-                LoadMetadataOptions.defaults().setLoadDescendantLevels(loadDescendantLevels)
+                LoadMetadataOptions.defaults().setLoadDescendantType(loadDescendantType)
                     .setCreateAncestors(false).setUfsStatus(childStatus);
             loadMetadataAndJournal(tempInodePath, loadMetadataOptions, journalContext);
           }
@@ -2602,7 +2604,8 @@ private void loadMetadataIfNotExistAndJournal(LockedInodePath inodePath,
     if (inodeExists) {
       try {
         Inode<?> inode = inodePath.getInode();
-        loadDirectChildren = inode.isDirectory() && (options.getLoadDescendantLevels() != 0);
+        loadDirectChildren =
+            inode.isDirectory() && (options.getLoadDescendantType() != DescendantType.NONE);
       } catch (FileDoesNotExistException e) {
         // This should never happen.
         throw new RuntimeException(e);
@@ -2639,7 +2642,7 @@ public void mount(AlluxioURI alluxioPath, AlluxioURI ufsPath, MountOptions optio
       }
       mMountTable.checkUnderWritableMountPoint(alluxioPath);
       // Possible ufs sync.
-      syncMetadata(journalContext, inodePath, lockingScheme, 1);
+      syncMetadata(journalContext, inodePath, lockingScheme, DescendantType.ONE);
 
       mountAndJournal(inodePath, ufsPath, options, journalContext);
       auditContext.setSucceeded(true);
@@ -2905,7 +2908,8 @@ public void setAttribute(AlluxioURI path, SetAttributeOptions options)
         throw e;
       }
       // Possible ufs sync.
-      syncMetadata(journalContext, inodePath, lockingScheme, options.isRecursive() ? -1 : 1);
+      syncMetadata(journalContext, inodePath, lockingScheme,
+          options.isRecursive() ? DescendantType.ALL : DescendantType.ONE);
       if (!inodePath.fullPathExists()) {
         throw new FileDoesNotExistException(ExceptionMessage.PATH_DOES_NOT_EXIST.getMessage(path));
       }
@@ -3058,11 +3062,11 @@ private void scheduleAsyncPersistenceInternal(LockedInodePath inodePath) throws
    * @param journalContext the journal context
    * @param inodePath the Alluxio inode path to sync with UFS
    * @param lockingScheme the locking scheme used to lock the inode path
-   * @param syncDescendantLevels the number of levels to sync
+   * @param syncDescendantType how to sync descendants
    * @return true if the sync was performed successfully, false otherwise (including errors)
    */
   private boolean syncMetadata(JournalContext journalContext, LockedInodePath inodePath,
-      LockingScheme lockingScheme, int syncDescendantLevels) {
+      LockingScheme lockingScheme, DescendantType syncDescendantType) {
     if (!lockingScheme.shouldSync()) {
       return false;
     }
@@ -3120,7 +3124,7 @@ private boolean syncMetadata(JournalContext journalContext, LockedInodePath inod
         }
 
         if (syncPlan.toSyncChildren()) {
-          loadMetadata = syncDirMetadata(journalContext, inodePath, syncDescendantLevels);
+          loadMetadata = syncDirMetadata(journalContext, inodePath, syncDescendantType);
         }
       }
     } catch (Exception e) {
@@ -3140,7 +3144,7 @@ private boolean syncMetadata(JournalContext journalContext, LockedInodePath inod
     if (loadMetadata) {
       try {
         loadMetadataAndJournal(inodePath, LoadMetadataOptions.defaults().setCreateAncestors(true)
-            .setLoadDescendantLevels(syncDescendantLevels), journalContext);
+            .setLoadDescendantType(syncDescendantType), journalContext);
       } catch (Exception e) {
         LOG.error("Failed to load metadata for path: {} error: {}", inodePath.getUri(),
             e.toString());
@@ -3153,9 +3157,10 @@ private boolean syncMetadata(JournalContext journalContext, LockedInodePath inod
   }
 
   private boolean syncDirMetadata(JournalContext journalContext, LockedInodePath inodePath,
-      int syncDescendantLevels) throws FileDoesNotExistException, InvalidPathException, IOException,
+      DescendantType syncDescendantType)
+      throws FileDoesNotExistException, InvalidPathException, IOException,
       DirectoryNotEmptyException {
-    if (syncDescendantLevels == 0) {
+    if (syncDescendantType == DescendantType.NONE) {
       return false;
     }
     MountTable.Resolution resolution = mMountTable.resolve(inodePath.getUri());
@@ -3222,8 +3227,10 @@ private boolean syncDirMetadata(JournalContext journalContext, LockedInodePath i
           tempInodePath.setDescendant(inodeEntry.getValue(),
               inodePath.getUri().join(inodeEntry.getKey()));
 
-          loadMetadata |= syncDirMetadata(journalContext, tempInodePath,
-              syncDescendantLevels < 0 ? -1 : syncDescendantLevels - 1);
+          if (syncDescendantType == DescendantType.ALL) {
+            // Recursively sync children
+            loadMetadata |= syncDirMetadata(journalContext, tempInodePath, DescendantType.ALL);
+          }
         }
       }
     }

core/server/master/src/main/java/alluxio/master/file/FileSystemMasterClientServiceHandler.java

@@ -23,6 +23,7 @@
 import alluxio.master.file.options.CreateDirectoryOptions;
 import alluxio.master.file.options.CreateFileOptions;
 import alluxio.master.file.options.DeleteOptions;
+import alluxio.master.file.options.DescendantType;
 import alluxio.master.file.options.FreeOptions;
 import alluxio.master.file.options.GetStatusOptions;
 import alluxio.master.file.options.ListStatusOptions;
@@ -275,7 +276,8 @@ public LoadMetadataTResponse loadMetadata(final String alluxioPath, final boolea
       @Override
       public LoadMetadataTResponse call() throws AlluxioException, IOException {
         return new LoadMetadataTResponse(mFileSystemMaster.loadMetadata(new AlluxioURI(alluxioPath),
-            LoadMetadataOptions.defaults().setCreateAncestors(true).setLoadDescendantLevels(1)));
+            LoadMetadataOptions.defaults().setCreateAncestors(true).setLoadDescendantType(
+                DescendantType.ONE)));
       }
 
       @Override

core/server/master/src/main/java/alluxio/master/file/options/DescendantType.java

@@ -0,0 +1,24 @@
+/*
+ * The Alluxio Open Foundation licenses this work under the Apache License, version 2.0
+ * (the "License"). You may not use this work except in compliance with the License, which is
+ * available at www.apache.org/licenses/LICENSE-2.0
+ *
+ * This software is distributed on an "AS IS" basis, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND,
+ * either express or implied, as more fully set forth in the License.
+ *
+ * See the NOTICE file distributed with this work for information regarding copyright ownership.
+ */
+
+package alluxio.master.file.options;
+
+/**
+ * Type of descendant level processing for filesystem inodes.
+ */
+public enum DescendantType {
+  /** No descendant processing. */
+  NONE,
+  /** Process only 1 level of children. */
+  ONE,
+  /** Process all descendants. */
+  ALL,
+}

core/server/master/src/main/java/alluxio/master/file/options/LoadMetadataOptions.java

@@ -27,13 +27,7 @@
 public final class LoadMetadataOptions {
   private CommonOptions mCommonOptions;
   private boolean mCreateAncestors;
-  /**
-   * The number of descendant levels to load.
-   * -1: load all levels
-   *  0: do not load
-   * 1+: specific number of levels to load
-   */
-  private int mLoadDescendantLevels;
+  private DescendantType mLoadDescendantType;
   private UfsStatus mUfsStatus;
 
   /**
@@ -48,7 +42,7 @@ private LoadMetadataOptions() {
     mCommonOptions = CommonOptions.defaults();
     mCreateAncestors = false;
     mUfsStatus = null;
-    mLoadDescendantLevels = 0;
+    mLoadDescendantType = DescendantType.NONE;
   }
 
   /**
@@ -71,10 +65,10 @@ public CommonOptions getCommonOptions() {
   }
 
   /**
-   * @return the number of levels of descendants to load (-1 means no loading)
+   * @return the type of descendants to load
    */
-  public int getLoadDescendantLevels() {
-    return mLoadDescendantLevels;
+  public DescendantType getLoadDescendantType() {
+    return mLoadDescendantType;
   }
 
   /**
@@ -115,11 +109,11 @@ public LoadMetadataOptions setCreateAncestors(boolean createAncestors) {
   }
 
   /**
-   * @param loadDescendantLevels the number of levels of descendants to load (-1 means no loading)
+   * @param loadDescendantType the type of descendants to load
    * @return the updated object
    */
-  public LoadMetadataOptions setLoadDescendantLevels(int loadDescendantLevels) {
-    mLoadDescendantLevels = loadDescendantLevels;
+  public LoadMetadataOptions setLoadDescendantType(DescendantType loadDescendantType) {
+    mLoadDescendantType = loadDescendantType;
     return this;
   }
 
@@ -145,21 +139,21 @@ public boolean equals(Object o) {
     LoadMetadataOptions that = (LoadMetadataOptions) o;
     return Objects.equal(mCreateAncestors, that.mCreateAncestors)
         && Objects.equal(mCommonOptions, that.mCommonOptions)
-        && Objects.equal(mLoadDescendantLevels, that.mLoadDescendantLevels)
+        && Objects.equal(mLoadDescendantType, that.mLoadDescendantType)
         && Objects.equal(mUfsStatus, that.mUfsStatus);
   }
 
   @Override
   public int hashCode() {
-    return Objects.hashCode(mCreateAncestors, mLoadDescendantLevels, mUfsStatus, mCommonOptions);
+    return Objects.hashCode(mCreateAncestors, mLoadDescendantType, mUfsStatus, mCommonOptions);
   }
 
   @Override
   public String toString() {
     return Objects.toStringHelper(this)
         .add("commonOptions", mCommonOptions)
         .add("createAncestors", mCreateAncestors)
-        .add("loadDescendantLevels", mLoadDescendantLevels)
+        .add("loadDescendantLevels", mLoadDescendantType)
         .add("ufsStatus", mUfsStatus).toString();
   }
 }

core/server/master/src/test/java/alluxio/master/file/FileSystemMasterTest.java

@@ -47,6 +47,7 @@
 import alluxio.master.file.options.CreateDirectoryOptions;
 import alluxio.master.file.options.CreateFileOptions;
 import alluxio.master.file.options.DeleteOptions;
+import alluxio.master.file.options.DescendantType;
 import alluxio.master.file.options.FreeOptions;
 import alluxio.master.file.options.GetStatusOptions;
 import alluxio.master.file.options.ListStatusOptions;
@@ -1848,7 +1849,8 @@ public void testLoadMetadata() throws Exception {
 
     // This should not throw file exists exception those a/f1 is loaded.
     mFileSystemMaster.loadMetadata(new AlluxioURI("alluxio:/a"),
-        LoadMetadataOptions.defaults().setCreateAncestors(true).setLoadDescendantLevels(1));
+        LoadMetadataOptions.defaults().setCreateAncestors(true).setLoadDescendantType(
+            DescendantType.ONE));
 
     // TODO(peis): Avoid this hack by adding an option in getFileInfo to skip loading metadata.
     try {
@@ -1861,7 +1863,8 @@ public void testLoadMetadata() throws Exception {
     }
 
     mFileSystemMaster.loadMetadata(new AlluxioURI("alluxio:/a"),
-        LoadMetadataOptions.defaults().setCreateAncestors(true).setLoadDescendantLevels(1));
+        LoadMetadataOptions.defaults().setCreateAncestors(true)
+            .setLoadDescendantType(DescendantType.ONE));
   }
 
   /**

core/server/master/src/test/java/alluxio/master/file/options/LoadMetadataOptionsTest.java

@@ -27,19 +27,24 @@ public void defaults() {
     LoadMetadataOptions options = LoadMetadataOptions.defaults();
     Assert.assertEquals(false, options.isCreateAncestors());
     Assert.assertEquals(null, options.getUfsStatus());
-    Assert.assertEquals(0, options.getLoadDescendantLevels());
+    Assert.assertEquals(DescendantType.NONE, options.getLoadDescendantType());
   }
 
   @Test
   public void fields() {
     Random random = new Random();
     boolean isCreateAncestors = random.nextBoolean();
-    int loadDescendantLevels = random.nextInt();
     LoadMetadataOptions options = LoadMetadataOptions.defaults();
     options.setCreateAncestors(isCreateAncestors);
-    options.setLoadDescendantLevels(loadDescendantLevels);
     Assert.assertEquals(isCreateAncestors, options.isCreateAncestors());
-    Assert.assertEquals(loadDescendantLevels, options.getLoadDescendantLevels());
+
+    DescendantType loadDescendantType = DescendantType.ALL;
+    options.setLoadDescendantType(loadDescendantType);
+    Assert.assertEquals(loadDescendantType, options.getLoadDescendantType());
+
+    loadDescendantType = DescendantType.ONE;
+    options.setLoadDescendantType(loadDescendantType);
+    Assert.assertEquals(loadDescendantType, options.getLoadDescendantType());
   }
 
   @Test