diff --git a/core/server/master/src/main/java/alluxio/master/file/DefaultFileSystemMaster.java b/core/server/master/src/main/java/alluxio/master/file/DefaultFileSystemMaster.java index 121feb72f575..39ec2f8836d7 100644 --- a/core/server/master/src/main/java/alluxio/master/file/DefaultFileSystemMaster.java +++ b/core/server/master/src/main/java/alluxio/master/file/DefaultFileSystemMaster.java @@ -3127,69 +3127,18 @@ private boolean syncMetadata(JournalContext journalContext, LockedInodePath inod // Set to true if ufs metadata must be loaded. boolean loadMetadata = false; - // Set to true if the given inode was deleted. boolean deletedInode = false; - DeleteOptions syncDeleteOptions = - DeleteOptions.defaults().setRecursive(true).setAlluxioOnly(true).setUnchecked(true); - try { if (!inodePath.fullPathExists()) { // The requested path does not exist in Alluxio, so just load metadata. loadMetadata = true; } else { - // The requested path already exists in Alluxio. - Inode inode = inodePath.getInode(); - - if (inode instanceof InodeFile && !((InodeFile) inode).isCompleted()) { - // Do not sync an incomplete file, since the UFS file is expected to not exist. - return false; - } - if (inode.getPersistenceState() == PersistenceState.TO_BE_PERSISTED) { - // Do not sync a file in the process of being persisted, since the UFS file is being - // written. - return false; - } - - MountTable.Resolution resolution = mMountTable.resolve(inodePath.getUri()); - AlluxioURI ufsUri = resolution.getUri(); - UnderFileSystem ufs = resolution.getUfs(); - String ufsFingerprint = ufs.getFingerprint(ufsUri.toString()); - boolean isMountPoint = mMountTable.isMountPoint(inodePath.getUri()); - - UfsSyncUtils.SyncPlan syncPlan = - UfsSyncUtils.computeSyncPlan(inode, ufsFingerprint, isMountPoint); - - if (syncPlan.toUpdateDirectory()) { - // Fingerprints only consider permissions for directory inodes. - UfsStatus ufsStatus = ufs.getStatus(ufsUri.toString()); - SetAttributeOptions options = - SetAttributeOptions.defaults().setOwner(ufsStatus.getOwner()) - .setGroup(ufsStatus.getGroup()).setMode(ufsStatus.getMode()) - .setUfsFingerprint(ufsFingerprint); - long opTimeMs = System.currentTimeMillis(); - // use replayed, since updating UFS is not desired. - setAttributeInternal(inodePath, true, opTimeMs, options); - journalSetAttribute(inodePath, opTimeMs, options, journalContext); - } - if (syncPlan.toDelete()) { - try { - deleteInternal(inodePath, false, System.currentTimeMillis(), syncDeleteOptions, - journalContext, blockDeletionContext); - deletedInode = true; - } catch (DirectoryNotEmptyException | IOException e) { - // Should not happen, since it is an unchecked delete. - LOG.error("Unexpected error for unchecked delete.", e); - } - } - if (syncPlan.toLoadMetadata()) { - loadMetadata = true; - } - if (syncPlan.toSyncChildren()) { - loadMetadata = syncChildrenMetadata(journalContext, inodePath, syncDescendantType, - blockDeletionContext); - } + SyncResult result = + syncInodeMetadata(journalContext, inodePath, syncDescendantType, blockDeletionContext); + deletedInode = result.getDeletedInode(); + loadMetadata = result.getLoadMetadata(); } } catch (Exception e) { LOG.error("Failed to remove out-of-sync metadata for path: {}", inodePath.getUri(), e); @@ -3220,86 +3169,148 @@ private boolean syncMetadata(JournalContext journalContext, LockedInodePath inod return true; } - private boolean syncChildrenMetadata(JournalContext journalContext, LockedInodePath inodePath, - DescendantType syncDescendantType, BlockDeletionContext blockDeletionContext) - throws FileDoesNotExistException, InvalidPathException, IOException, - DirectoryNotEmptyException { - if (syncDescendantType == DescendantType.NONE) { - return false; + /** + * This class represents the result for a sync. The following are returned: + * - deleted: if true, the inode was already deleted as part of the syncing process + * - loadMetadata: if true, load metadata must be called (the last step of the full sync process) + */ + private static class SyncResult { + private boolean mLoadMetadata; + private boolean mDeletedInode; + + static SyncResult defaults() { + return new SyncResult(false, false); + } + + SyncResult(boolean loadMetadata, boolean deletedInode) { + mLoadMetadata = loadMetadata; + mDeletedInode = deletedInode; + } + + boolean getLoadMetadata() { + return mLoadMetadata; } - MountTable.Resolution resolution = mMountTable.resolve(inodePath.getUri()); - AlluxioURI ufsUri = resolution.getUri(); - UnderFileSystem ufs = resolution.getUfs(); + boolean getDeletedInode() { + return mDeletedInode; + } + } + + /** + * Syncs an inode with the UFS. + * + * @param journalContext the journal context + * @param inodePath the Alluxio inode path to sync with UFS + * @param syncDescendantType how to sync descendants + * @param blockDeletionContext the block deletion context + * @return the result of the sync, including if the inode was deleted, and if further load + * metadata is required + */ + private SyncResult syncInodeMetadata(JournalContext journalContext, LockedInodePath inodePath, + DescendantType syncDescendantType, BlockDeletionContext blockDeletionContext) + throws FileDoesNotExistException, InvalidPathException, IOException, AccessControlException { + // Set to true if ufs metadata must be loaded. boolean loadMetadata = false; + // Set to true if the given inode was deleted. + boolean deletedInode = false; + // The options for deleting. DeleteOptions syncDeleteOptions = DeleteOptions.defaults().setRecursive(true).setAlluxioOnly(true).setUnchecked(true); + + // The requested path already exists in Alluxio. Inode inode = inodePath.getInode(); - UfsStatus[] listStatus = ufs.listStatus(ufsUri.toString()); + if (inode instanceof InodeFile && !((InodeFile) inode).isCompleted()) { + // Do not sync an incomplete file, since the UFS file is expected to not exist. + return SyncResult.defaults(); + } + if (inode.getPersistenceState() == PersistenceState.TO_BE_PERSISTED) { + // Do not sync a file in the process of being persisted, since the UFS file is being + // written. + return SyncResult.defaults(); + } - if (listStatus != null) { - // maps children name to up-to-date ufs fingerprint - Map ufsChildFingerprints = new HashMap<>(); - // maps children name to inode - Map> inodeChildren = new HashMap<>(); + MountTable.Resolution resolution = mMountTable.resolve(inodePath.getUri()); + AlluxioURI ufsUri = resolution.getUri(); + UnderFileSystem ufs = resolution.getUfs(); + String ufsFingerprint = ufs.getFingerprint(ufsUri.toString()); + boolean containsMountPoint = mMountTable.containsMountPoint(inodePath.getUri()); + + UfsSyncUtils.SyncPlan syncPlan = + UfsSyncUtils.computeSyncPlan(inode, ufsFingerprint, containsMountPoint); - for (UfsStatus ufsChildStatus : listStatus) { - ufsChildFingerprints.put(ufsChildStatus.getName(), - Fingerprint.create(ufs.getUnderFSType(), ufsChildStatus).serialize()); + if (syncPlan.toUpdateDirectory()) { + // Fingerprints only consider permissions for directory inodes. + UfsStatus ufsStatus = null; + try { + ufsStatus = ufs.getStatus(ufsUri.toString()); + } catch (IOException e) { + // Ignore, since this directory inode could be out of sync (contains a mount point) } - InodeDirectory inodeDir = (InodeDirectory) inode; - for (Inode child : inodeDir.getChildren()) { - inodeChildren.put(child.getName(), child); + if (ufsStatus != null) { + SetAttributeOptions options = + SetAttributeOptions.defaults().setOwner(ufsStatus.getOwner()) + .setGroup(ufsStatus.getGroup()).setMode(ufsStatus.getMode()) + .setUfsFingerprint(ufsFingerprint); + long opTimeMs = System.currentTimeMillis(); + // use replayed, since updating UFS is not desired. + setAttributeInternal(inodePath, true, opTimeMs, options); + journalSetAttribute(inodePath, opTimeMs, options, journalContext); } - - // Iterate over ufs children and process children which do not exist in Alluxio. - for (Map.Entry ufsEntry : ufsChildFingerprints.entrySet()) { - if (!inodeChildren.containsKey(ufsEntry.getKey()) && !PathUtils - .isTemporaryFileName(ufsEntry.getKey())) { - // Ufs child exists, but Alluxio child does not. Must load metadata. - loadMetadata = true; - break; + } + if (syncPlan.toDelete()) { + try { + deleteInternal(inodePath, false, System.currentTimeMillis(), syncDeleteOptions, + journalContext, blockDeletionContext); + deletedInode = true; + } catch (DirectoryNotEmptyException | IOException e) { + // Should not happen, since it is an unchecked delete. + LOG.error("Unexpected error for unchecked delete.", e); + } + } + if (syncPlan.toLoadMetadata()) { + loadMetadata = true; + } + if (syncPlan.toSyncChildren() && inode instanceof InodeDirectory + && syncDescendantType != DescendantType.NONE) { + UfsStatus[] listStatus = ufs.listStatus(ufsUri.toString()); + if (listStatus != null) { + InodeDirectory inodeDir = (InodeDirectory) inode; + // maps children name to inode + Map> inodeChildren = new HashMap<>(); + for (Inode child : inodeDir.getChildren()) { + inodeChildren.put(child.getName(), child); } - } - // Iterate over Alluxio children and process persisted children. - for (Map.Entry> inodeEntry : inodeChildren.entrySet()) { - if (!inodeEntry.getValue().isPersisted()) { - // Ignore non-persisted inodes. - continue; + for (UfsStatus ufsChildStatus : listStatus) { + if (!inodeChildren.containsKey(ufsChildStatus.getName()) && !PathUtils + .isTemporaryFileName(ufsChildStatus.getName())) { + // Ufs child exists, but Alluxio child does not. Must load metadata. + loadMetadata = true; + break; + } } - String ufsFingerprint = ufsChildFingerprints.get(inodeEntry.getKey()); - boolean deleteChild = - !UfsSyncUtils.inodeUfsIsSynced(inodeEntry.getValue(), ufsFingerprint); - - if (deleteChild) { - TempInodePathForDescendant tempInodePath = - new TempInodePathForDescendant(inodePath); - tempInodePath.setDescendant(inodeEntry.getValue(), - inodePath.getUri().join(inodeEntry.getKey())); - - deleteInternal(tempInodePath, false, System.currentTimeMillis(), syncDeleteOptions, - journalContext, blockDeletionContext); - // Must load metadata afterwards. - loadMetadata = true; - } else if (inodeEntry.getValue().isDirectory()) { - // Recursively sync for this directory. - TempInodePathForDescendant tempInodePath = - new TempInodePathForDescendant(inodePath); - tempInodePath.setDescendant(inodeEntry.getValue(), - inodePath.getUri().join(inodeEntry.getKey())); - - if (syncDescendantType == DescendantType.ALL) { - // Recursively sync children - loadMetadata |= syncChildrenMetadata(journalContext, tempInodePath, DescendantType.ALL, - blockDeletionContext); + // Iterate over Alluxio children and process persisted children. + for (Map.Entry> inodeEntry : inodeChildren.entrySet()) { + if (!inodeEntry.getValue().isPersisted()) { + // Ignore non-persisted inodes. + continue; + } + TempInodePathForDescendant tempInodePath = new TempInodePathForDescendant(inodePath); + tempInodePath + .setDescendant(inodeEntry.getValue(), inodePath.getUri().join(inodeEntry.getKey())); + + // Recursively sync children + if (syncDescendantType != DescendantType.ALL) { + syncDescendantType = DescendantType.NONE; } + loadMetadata |= syncInodeMetadata(journalContext, tempInodePath, + syncDescendantType, blockDeletionContext).getLoadMetadata(); } } } - return loadMetadata; + return new SyncResult(loadMetadata, deletedInode); } @Override diff --git a/core/server/master/src/main/java/alluxio/master/file/meta/MountTable.java b/core/server/master/src/main/java/alluxio/master/file/meta/MountTable.java index bf36e39389ce..917c6d0f251f 100644 --- a/core/server/master/src/main/java/alluxio/master/file/meta/MountTable.java +++ b/core/server/master/src/main/java/alluxio/master/file/meta/MountTable.java @@ -260,6 +260,24 @@ public Map getMountTable() { } } + /** + * @param uri the Alluxio uri to check + * @return true if the specified uri is mount point, or has a descendant which is a mount point + */ + public boolean containsMountPoint(AlluxioURI uri) throws InvalidPathException { + String path = uri.getPath(); + + try (LockResource r = new LockResource(mReadLock)) { + for (Map.Entry entry : mMountTable.entrySet()) { + String mountPath = entry.getKey(); + if (PathUtils.hasPrefix(mountPath, path)) { + return true; + } + } + } + return false; + } + /** * @param uri an Alluxio path URI * @return whether the given uri is a mount point diff --git a/core/server/master/src/main/java/alluxio/master/file/meta/UfsSyncUtils.java b/core/server/master/src/main/java/alluxio/master/file/meta/UfsSyncUtils.java index 13df96cda877..9e77bf2960b3 100644 --- a/core/server/master/src/main/java/alluxio/master/file/meta/UfsSyncUtils.java +++ b/core/server/master/src/main/java/alluxio/master/file/meta/UfsSyncUtils.java @@ -31,10 +31,11 @@ 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 + * @param containsMountPoint true if this inode contains 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, boolean isMountPoint) { + public static SyncPlan computeSyncPlan(Inode inode, String ufsFingerprint, + boolean containsMountPoint) { boolean isSynced = inodeUfsIsSynced(inode, ufsFingerprint); boolean ufsExists = !Constants.INVALID_UFS_FINGERPRINT.equals(ufsFingerprint); boolean ufsIsDir = ufsFingerprint != null && Fingerprint.Type.DIRECTORY.name() @@ -45,9 +46,9 @@ public static SyncPlan computeSyncPlan(Inode inode, String ufsFingerprint, boole // 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)) { + if (inode.isDirectory() && (containsMountPoint || ufsIsDir)) { // Instead of deleting and then loading metadata to update, try to update directly - // - mount points should not be deleted + // - mount points (or paths with mount point descendants) should not be deleted // - directory permissions can be updated without removing the inode if (inode.getParentId() != InodeTree.NO_PARENT) { // Only update the inode if it is not the root directory. The root directory is a special diff --git a/tests/src/test/java/alluxio/master/file/UfsSyncIntegrationTest.java b/tests/src/test/java/alluxio/master/file/UfsSyncIntegrationTest.java index 1fe31979658c..064cfbc2660d 100644 --- a/tests/src/test/java/alluxio/master/file/UfsSyncIntegrationTest.java +++ b/tests/src/test/java/alluxio/master/file/UfsSyncIntegrationTest.java @@ -26,6 +26,7 @@ import alluxio.client.file.FileSystem; import alluxio.client.file.FileSystemTestUtils; import alluxio.client.file.URIStatus; +import alluxio.client.file.options.CreateDirectoryOptions; import alluxio.client.file.options.CreateFileOptions; import alluxio.client.file.options.DeleteOptions; import alluxio.client.file.options.ExistsOptions; @@ -306,6 +307,99 @@ public void lastModifiedLocalFileSync() throws Exception { Assert.assertNotEquals(startFingerprint, status.getUfsFingerprint()); } + @Test + public void mountPoint() throws Exception { + ListStatusOptions options = + ListStatusOptions.defaults().setLoadMetadataType(LoadMetadataType.Never) + .setCommonOptions(SYNC_NEVER); + List rootListing = + mFileSystem.listStatus(new AlluxioURI("/"), options).stream().map(URIStatus::getName) + .collect(Collectors.toList()); + Assert.assertEquals(1, rootListing.size()); + Assert.assertEquals("mnt", rootListing.get(0)); + + options = ListStatusOptions.defaults().setLoadMetadataType(LoadMetadataType.Never) + .setCommonOptions(SYNC_ALWAYS); + rootListing = + mFileSystem.listStatus(new AlluxioURI("/"), options).stream().map(URIStatus::getName) + .collect(Collectors.toList()); + Assert.assertEquals(1, rootListing.size()); + Assert.assertEquals("mnt", rootListing.get(0)); + } + + @Test + public void mountPointConflict() throws Exception { + GetStatusOptions getStatusOptions = + GetStatusOptions.defaults().setLoadMetadataType(LoadMetadataType.Never) + .setCommonOptions(SYNC_ALWAYS); + URIStatus status = mFileSystem.getStatus(new AlluxioURI("/"), getStatusOptions); + + // add a UFS dir which conflicts with a mount point. + String fromRootUfs = status.getUfsPath() + "/mnt"; + Assert.assertTrue(new File(fromRootUfs).mkdirs()); + + ListStatusOptions options = + ListStatusOptions.defaults().setLoadMetadataType(LoadMetadataType.Never) + .setCommonOptions(SYNC_ALWAYS); + List rootListing = mFileSystem.listStatus(new AlluxioURI("/"), options); + Assert.assertEquals(1, rootListing.size()); + Assert.assertEquals("mnt", rootListing.get(0).getName()); + Assert.assertNotEquals(fromRootUfs, rootListing.get(0).getUfsPath()); + } + + @Test + public void mountPointNested() throws Exception { + String ufsPath = Files.createTempDir().getAbsolutePath(); + mFileSystem.createDirectory(new AlluxioURI("/nested/mnt/"), + CreateDirectoryOptions.defaults().setRecursive(true).setWriteType(WriteType.CACHE_THROUGH)); + mFileSystem.mount(new AlluxioURI("/nested/mnt/ufs"), new AlluxioURI(ufsPath)); + + // recursively sync (setAttribute enables recursive sync) + mFileSystem.setAttribute(new AlluxioURI("/"), + SetAttributeOptions.defaults().setCommonOptions(SYNC_ALWAYS).setRecursive(true) + .setTtl(55555)); + + // Verify /nested/mnt/ dir has 1 mount point + ListStatusOptions options = + ListStatusOptions.defaults().setLoadMetadataType(LoadMetadataType.Never) + .setCommonOptions(SYNC_NEVER); + List listing = mFileSystem.listStatus(new AlluxioURI("/nested/mnt/"), options); + Assert.assertEquals(1, listing.size()); + Assert.assertEquals("ufs", listing.get(0).getName()); + + // Remove a directory in the parent UFS, which has a mount point descendant + URIStatus status = mFileSystem.getStatus(new AlluxioURI("/nested/mnt/"), + GetStatusOptions.defaults().setLoadMetadataType(LoadMetadataType.Never) + .setCommonOptions(SYNC_NEVER)); + Assert.assertTrue(new File(status.getUfsPath()).delete()); + + // recursively sync (setAttribute enables recursive sync) + mFileSystem.setAttribute(new AlluxioURI("/"), + SetAttributeOptions.defaults().setCommonOptions(SYNC_ALWAYS).setRecursive(true) + .setTtl(44444)); + + // Verify /nested/mnt/ dir has 1 mount point + listing = mFileSystem.listStatus(new AlluxioURI("/nested/mnt/"), options); + Assert.assertEquals(1, listing.size()); + Assert.assertEquals("ufs", listing.get(0).getName()); + + // Remove a directory in the parent UFS, which has a mount point descendant + status = mFileSystem.getStatus(new AlluxioURI("/nested/"), + GetStatusOptions.defaults().setLoadMetadataType(LoadMetadataType.Never) + .setCommonOptions(SYNC_NEVER)); + Assert.assertTrue(new File(status.getUfsPath()).delete()); + + // recursively sync (setAttribute enables recursive sync) + mFileSystem.setAttribute(new AlluxioURI("/"), + SetAttributeOptions.defaults().setCommonOptions(SYNC_ALWAYS).setRecursive(true) + .setTtl(44444)); + + // Verify /nested/mnt/ dir has 1 mount point + listing = mFileSystem.listStatus(new AlluxioURI("/nested/mnt/"), options); + Assert.assertEquals(1, listing.size()); + Assert.assertEquals("ufs", listing.get(0).getName()); + } + @Test public void ufsModeSync() throws Exception { GetStatusOptions options =