Merge pull request #6943 from gpang/merge1.7

[MERGE] Merge branch-1.7

core/common/src/main/java/alluxio/underfs/Fingerprint.java

@@ -27,12 +27,14 @@
  */
 @NotThreadSafe
 public final class Fingerprint {
+  // TODO(gpang): partition fingerprint for metadata and content, for more efficient updates.
   /** These tags are required in all fingerprints. */
   private static final Tag[] REQUIRED_TAGS = {Tag.TYPE, Tag.UFS, Tag.OWNER, Tag.GROUP, Tag.MODE};
   /** These tags are optional, and are serialized after the required tags. */
   private static final Tag[] OPTIONAL_TAGS = {Tag.CONTENT_HASH};
 
   private static final Pattern SANITIZE_REGEX = Pattern.compile("[ :]");
+  private static final String UNDERSCORE = "_";
 
   private final Map<Tag, String> mValues;
 
@@ -140,35 +142,35 @@ public String serialize() {
    * @param tag the tag to update
    * @param value the new value of the tag
    */
-  public void updateTag(Tag tag, String value) {
+  public void putTag(Tag tag, String value) {
     if (value != null) {
       mValues.put(tag, sanitizeString(value));
     }
   }
 
+  /**
+   * @param tag the tag to get
+   * @return the value of the tag
+   */
+  public String getTag(Tag tag) {
+    String value = mValues.get(tag);
+    if (value == null) {
+      return UNDERSCORE;
+    }
+    return value;
+  }
+
   private Fingerprint(Map<Tag, String> values) {
     mValues = new HashMap<>();
     for (Map.Entry<Tag, String> entry : values.entrySet()) {
       putTag(entry.getKey(), entry.getValue());
     }
   }
 
-  private void putTag(Tag tag, String value) {
-    mValues.put(tag, sanitizeString(value));
-  }
-
-  private String getTag(Tag tag) {
-    String value = mValues.get(tag);
-    if (value == null) {
-      return "_";
-    }
-    return value;
-  }
-
   private String sanitizeString(String input) {
     if (input == null || input.isEmpty()) {
-      return "_";
+      return UNDERSCORE;
     }
-    return SANITIZE_REGEX.matcher(input).replaceAll("_");
+    return SANITIZE_REGEX.matcher(input).replaceAll(UNDERSCORE);
   }
 }

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

@@ -3136,10 +3136,11 @@ private boolean syncMetadata(JournalContext journalContext, LockedInodePath inod
     // The high-level process for the syncing is:
     // 1. Find all Alluxio paths which are not consistent with the corresponding UFS path.
     //    This means the UFS path does not exist, or is different from the Alluxio metadata.
-    // 2. Delete those Alluxio paths which are not consistent with Alluxio. After this step, all
+    // 2. If possible, update an Alluxio directory with the corresponding UFS directory.
+    // 3. Delete any Alluxio path not consistent with UFS, or not in UFS. After this step, all
     //    the paths in Alluxio are consistent with UFS, and there may be additional UFS paths to
     //    load.
-    // 3. Load metadata from UFS.
+    // 4. Load metadata from UFS.
 
     // Set to true if ufs metadata must be loaded.
     boolean loadMetadata = false;
@@ -3172,10 +3173,20 @@ private boolean syncMetadata(JournalContext journalContext, LockedInodePath inod
         AlluxioURI ufsUri = resolution.getUri();
         try (CloseableResource<UnderFileSystem> ufsResource = resolution.acquireUfsResource()) {
           UnderFileSystem ufs = ufsResource.get();
+          String ufsFingerprint = ufs.getFingerprint(ufsUri.toString());
+          boolean isMountPoint = mMountTable.isMountPoint(inodePath.getUri());
 
           UfsSyncUtils.SyncPlan syncPlan =
-              UfsSyncUtils.computeSyncPlan(inode, ufs.getFingerprint(ufsUri.toString()));
-
+              UfsSyncUtils.computeSyncPlan(inode, ufsFingerprint, isMountPoint);
+
+          if (syncPlan.toUpdateDirectory()) {
+            // Fingerprints only consider permissions for directory inodes.
+            UfsStatus ufsStatus = ufs.getStatus(ufsUri.toString());
+            inode.setOwner(ufsStatus.getOwner());
+            inode.setGroup(ufsStatus.getGroup());
+            inode.setMode(ufsStatus.getMode());
+            inode.setUfsFingerprint(ufsFingerprint);
+          }
           if (syncPlan.toDelete()) {
             try {
               deleteInternal(inodePath, false, System.currentTimeMillis(), syncDeleteOptions,
@@ -3192,7 +3203,7 @@ private boolean syncMetadata(JournalContext journalContext, LockedInodePath inod
           }
 
           if (syncPlan.toSyncChildren()) {
-            loadMetadata = syncDirMetadata(journalContext, inodePath, syncDescendantType);
+            loadMetadata = syncChildrenMetadata(journalContext, inodePath, syncDescendantType);
           }
         }
       }
@@ -3227,7 +3238,7 @@ private boolean syncMetadata(JournalContext journalContext, LockedInodePath inod
     return true;
   }
 
-  private boolean syncDirMetadata(JournalContext journalContext, LockedInodePath inodePath,
+  private boolean syncChildrenMetadata(JournalContext journalContext, LockedInodePath inodePath,
       DescendantType syncDescendantType)
       throws FileDoesNotExistException, InvalidPathException, IOException,
       DirectoryNotEmptyException {
@@ -3301,7 +3312,8 @@ private boolean syncDirMetadata(JournalContext journalContext, LockedInodePath i
 
             if (syncDescendantType == DescendantType.ALL) {
               // Recursively sync children
-              loadMetadata |= syncDirMetadata(journalContext, tempInodePath, DescendantType.ALL);
+              loadMetadata |=
+                  syncChildrenMetadata(journalContext, tempInodePath, DescendantType.ALL);
             }
           }
         }
@@ -3430,9 +3442,9 @@ private List<Inode<?>> setAttributeInternal(LockedInodePath inodePath, boolean r
             if (!existingFingerprint.equals(Constants.INVALID_UFS_FINGERPRINT)) {
               // Update existing fingerprint, since contents did not change
               Fingerprint fp = Fingerprint.parse(existingFingerprint);
-              fp.updateTag(Fingerprint.Tag.OWNER, owner);
-              fp.updateTag(Fingerprint.Tag.GROUP, group);
-              fp.updateTag(Fingerprint.Tag.MODE, mode);
+              fp.putTag(Fingerprint.Tag.OWNER, owner);
+              fp.putTag(Fingerprint.Tag.GROUP, group);
+              fp.putTag(Fingerprint.Tag.MODE, mode);
               options.setUfsFingerprint(fp.serialize());
             } else {
               // Need to retrieve the fingerprint from ufs.

core/server/master/src/main/java/alluxio/master/file/meta/UfsSyncUtils.java

@@ -12,6 +12,7 @@
 package alluxio.master.file.meta;
 
 import alluxio.Constants;
+import alluxio.underfs.Fingerprint;
 import alluxio.underfs.UfsStatus;
 
 import javax.annotation.concurrent.NotThreadSafe;
@@ -30,29 +31,41 @@ private UfsSyncUtils() {} // prevent instantiation
    *
    * @param inode the inode to sync
    * @param ufsFingerprint the ufs fingerprint to check for the sync
+   * @param isMountPoint true if this inode is a mount point, false otherwise
    * @return a {@link SyncPlan} describing how to sync the inode with the ufs
    */
-  public static SyncPlan computeSyncPlan(Inode inode, String ufsFingerprint) {
+  public static SyncPlan computeSyncPlan(Inode inode, String ufsFingerprint, boolean isMountPoint) {
     boolean isSynced = inodeUfsIsSynced(inode, ufsFingerprint);
+    boolean ufsExists = !Constants.INVALID_UFS_FINGERPRINT.equals(ufsFingerprint);
+    boolean ufsIsDir = ufsFingerprint != null && Fingerprint.Type.DIRECTORY.name()
+        .equals(Fingerprint.parse(ufsFingerprint).getTag(Fingerprint.Tag.TYPE));
 
     UfsSyncUtils.SyncPlan syncPlan = new UfsSyncUtils.SyncPlan();
     if (!isSynced) {
-      // Alluxio inode is not synced with UFS.
-      if (inode.getParentId() != InodeTree.NO_PARENT) {
-        // Do not delete the root.
+      // Alluxio inode is not synced with UFS, so update the inode metadata
+      // Updating an inode is achieved by deleting the inode, and then loading metadata.
+
+      if (inode.isDirectory() && (isMountPoint || ufsIsDir)) {
+        // Instead of deleting and then loading metadata to update, try to update directly
+        // - mount points should not be deleted
+        // - directory permissions can be updated without removing the inode
+        syncPlan.setUpdateDirectory();
+        syncPlan.setSyncChildren();
+      } else {
+        // update inode, by deleting and then optionally loading metadata
         syncPlan.setDelete();
+        if (ufsExists) {
+          // UFS exists, so load metadata later.
+          syncPlan.setLoadMetadata();
+        }
       }
-      if (!Constants.INVALID_UFS_FINGERPRINT.equals(ufsFingerprint)) {
-        // UFS exists, so load metadata later.
-        syncPlan.setLoadMetadata();
+    } else {
+      // Inode is already synced.
+      if (inode.isDirectory() && inode.isPersisted()) {
+        // Both Alluxio and UFS are directories, so sync the children of the directory.
+        syncPlan.setSyncChildren();
       }
     }
-
-    if (inode.isDirectory() && inode.isPersisted() && isSynced) {
-      // Both Alluxio and UFS are directories, so sync the children of the directory.
-      syncPlan.setSyncChildren();
-    }
-
     return syncPlan;
   }
 
@@ -76,33 +89,36 @@ public static boolean inodeUfsIsSynced(Inode inode, String ufsFingerprint) {
           && inodeFile.getUfsFingerprint().equals(ufsFingerprint)
           && !inodeFile.getUfsFingerprint().equals(Constants.INVALID_UFS_FINGERPRINT);
     } else {
-      // ufs fingerprint must exist.
-      // TODO(gpang): Currently, directory fingerprints are not considered, because directories are
-      // created/modified more frequently than files.
-      isSyncedPersisted =
-          inode.isPersisted() && !Constants.INVALID_UFS_FINGERPRINT.equals(ufsFingerprint);
+      isSyncedPersisted = inode.isPersisted() && inode.getUfsFingerprint().equals(ufsFingerprint);
     }
     return isSyncedPersisted || isSyncedUnpersisted;
   }
 
   /**
    * A class describing how to sync an inode with the ufs.
    * A sync plan has several steps:
-   * 1. delete: the inode should be deleted
-   * 2. syncChildren: the inode is a directory, and the children should be synced
-   * 3. loadMetadata: the inode metadata should be loaded from UFS
+   * 1. updateDirectory: the directory inode should update permissions from UFS directory
+   * 2. delete: the inode should be deleted
+   * 3. syncChildren: the inode is a directory, and the children should be synced
+   * 4. loadMetadata: the inode metadata should be loaded from UFS
    */
   public static final class SyncPlan {
+    private boolean mUpdateDirectory;
     private boolean mDelete;
     private boolean mLoadMetadata;
     private boolean mSyncChildren;
 
     SyncPlan() {
+      mUpdateDirectory = false;
       mDelete = false;
       mLoadMetadata = false;
       mSyncChildren = false;
     }
 
+    void setUpdateDirectory() {
+      mUpdateDirectory = true;
+    }
+
     void setDelete() {
       mDelete = true;
     }
@@ -115,6 +131,13 @@ void setSyncChildren() {
       mSyncChildren = true;
     }
 
+    /**
+     * @return true if the direcotry inode permissions should be updated from UFS directory
+     */
+    public boolean toUpdateDirectory() {
+      return mUpdateDirectory;
+    }
+
     /**
      * @return true if the inode should be deleted for the sync plan
      */

docs/en/Powered-By-Alluxio.md

@@ -30,7 +30,6 @@ or let us know by email (project.alluxio@gmail.com) or
 * [Intel](http://www.intel.com/)
 * [JD](http://www.jd.com/)
 * [Juniper Networks](http://www.juniper.net/)
-* []
 * [Lenovo](https://www3.lenovo.com/us/en/)
 * [Microsoft](https://www.microsoft.com/en-us/)
 * [Momo](https://www.immomo.com/)
@@ -49,8 +48,6 @@ or let us know by email (project.alluxio@gmail.com) or
 * [Walmart](https://www.walmart.com/)
 * [ZTE](http://www.zte.com.cn)
 * [Kyligence](http://en.kyligence.io/)
-Here is a list that tracks external software projects that supplement Alluxio and add to its ecosystem.
-
 * [Alibaba OSS](http://www.aliyun.com/product/oss/?lang=en)
 * [Amazon S3](https://aws.amazon.com/s3/)
 * [Alluxio C++ Client](https://github.com/stormspirit/libtachyon)

tests/src/test/java/alluxio/master/file/UfsSyncIntegrationTest.java

@@ -32,6 +32,7 @@
 import alluxio.security.authorization.Mode;
 import alluxio.underfs.UnderFileSystem;
 import alluxio.util.CommonUtils;
+import alluxio.util.io.FileUtils;
 import alluxio.wire.CommonOptions;
 import alluxio.wire.LoadMetadataType;
 
@@ -46,6 +47,7 @@
 import java.io.File;
 import java.io.FileWriter;
 import java.io.IOException;
+import java.nio.file.attribute.PosixFilePermissions;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
@@ -348,6 +350,37 @@ public void alluxioModeFingerprintUpdate() throws Exception {
     Assert.assertNotEquals(startFingerprint, endFingerprint);
   }
 
+  @Test
+  public void UfsDirUpdatePermissions() throws Exception {
+    new File(ufsPath("/dir1")).mkdirs();
+    new File(ufsPath("/dir1/dir2")).mkdirs();
+    String fileA = "/dir1/dir2/fileA";
+    writeUfsFile(ufsPath(fileA), 1);
+
+    // Set the mode for the directory
+    FileUtils.changeLocalFilePermission(ufsPath("/dir1"), "rwxrwxrwx");
+
+    GetStatusOptions options =
+        GetStatusOptions.defaults().setLoadMetadataType(LoadMetadataType.Never)
+            .setCommonOptions(SYNC_ALWAYS);
+    URIStatus status = mFileSystem.getStatus(new AlluxioURI(alluxioPath("/dir1")), options);
+    Assert.assertNotNull(status);
+
+    Assert.assertEquals(
+        FileUtils.translatePosixPermissionToMode(PosixFilePermissions.fromString("rwxrwxrwx")),
+        status.getMode());
+
+    // Change the mode for the directory
+    FileUtils.changeLocalFilePermission(ufsPath("/dir1"), "rwxr-xr-x");
+
+    status = mFileSystem.getStatus(new AlluxioURI(alluxioPath("/dir1")), options);
+    Assert.assertNotNull(status);
+
+    Assert.assertEquals(
+        FileUtils.translatePosixPermissionToMode(PosixFilePermissions.fromString("rwxr-xr-x")),
+        status.getMode());
+  }
+
   @Test
   public void createNestedFileSync() throws Exception {
     Configuration.set(PropertyKey.USER_FILE_METADATA_SYNC_INTERVAL, "0");