HADOOP-15798. LocalMetadataStore put() does not retain isDeleted in parent listing. Contributed by Gabor Bota.
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/e148c3ff Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/e148c3ff Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/e148c3ff Branch: refs/heads/trunk Commit: e148c3ff094e16d788f0bf9833ced02d27640805 Parents: 5d96b74 Author: Sean Mackrory <mackror...@apache.org> Authored: Mon Nov 26 09:25:36 2018 -0700 Committer: Sean Mackrory <mackror...@apache.org> Committed: Wed Nov 28 10:45:09 2018 -0700 ---------------------------------------------------------------------- .../fs/s3a/s3guard/LocalMetadataEntry.java | 3 ++ .../fs/s3a/s3guard/LocalMetadataStore.java | 29 +++++++++++++------- .../fs/s3a/ITestS3GuardListConsistency.java | 18 +++++++----- .../fs/s3a/s3guard/MetadataStoreTestBase.java | 15 ++++++++++ 4 files changed, 48 insertions(+), 17 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/e148c3ff/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/LocalMetadataEntry.java ---------------------------------------------------------------------- diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/LocalMetadataEntry.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/LocalMetadataEntry.java index 6040d67..5405074 100644 --- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/LocalMetadataEntry.java +++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/LocalMetadataEntry.java @@ -31,6 +31,9 @@ public final class LocalMetadataEntry { @Nullable private DirListingMetadata dirListingMetadata; + LocalMetadataEntry() { + } + LocalMetadataEntry(PathMetadata pmd){ pathMetadata = pmd; dirListingMetadata = null; http://git-wip-us.apache.org/repos/asf/hadoop/blob/e148c3ff/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/LocalMetadataStore.java ---------------------------------------------------------------------- diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/LocalMetadataStore.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/LocalMetadataStore.java index 1a7f028..b8f9635 100644 --- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/LocalMetadataStore.java +++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/LocalMetadataStore.java @@ -269,19 +269,28 @@ public class LocalMetadataStore implements MetadataStore { Path parentPath = path.getParent(); if (parentPath != null) { LocalMetadataEntry parentMeta = localCache.getIfPresent(parentPath); - DirListingMetadata parentDirMeta = - new DirListingMetadata(parentPath, DirListingMetadata.EMPTY_DIR, - false); - parentDirMeta.put(status); - - getDirListingMeta(parentPath); + // Create empty parent LocalMetadataEntry if it doesn't exist if (parentMeta == null){ - localCache.put(parentPath, new LocalMetadataEntry(parentDirMeta)); - } else if (!parentMeta.hasDirMeta()) { + parentMeta = new LocalMetadataEntry(); + localCache.put(parentPath, parentMeta); + } + + // If there is no directory metadata on the parent entry, create + // an empty one + if (!parentMeta.hasDirMeta()) { + DirListingMetadata parentDirMeta = + new DirListingMetadata(parentPath, DirListingMetadata.EMPTY_DIR, + false); parentMeta.setDirListingMetadata(parentDirMeta); - } else { - parentMeta.getDirListingMeta().put(status); + } + + // Add the child status to the listing + parentMeta.getDirListingMeta().put(status); + + // Mark the listing entry as deleted if the meta is set to deleted + if(meta.isDeleted()) { + parentMeta.getDirListingMeta().markDeleted(path); } } } http://git-wip-us.apache.org/repos/asf/hadoop/blob/e148c3ff/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3GuardListConsistency.java ---------------------------------------------------------------------- diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3GuardListConsistency.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3GuardListConsistency.java index a1df1a5..d3b3c21 100644 --- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3GuardListConsistency.java +++ b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3GuardListConsistency.java @@ -178,7 +178,8 @@ public class ITestS3GuardListConsistency extends AbstractS3ATestBase { public void testConsistentListAfterDelete() throws Exception { S3AFileSystem fs = getFileSystem(); // test will fail if NullMetadataStore (the default) is configured: skip it. - Assume.assumeTrue(fs.hasMetadataStore()); + Assume.assumeTrue("FS needs to have a metadatastore.", + fs.hasMetadataStore()); // Any S3 keys that contain DELAY_KEY_SUBSTRING will be delayed // in listObjects() results via InconsistentS3Client @@ -190,11 +191,11 @@ public class ITestS3GuardListConsistency extends AbstractS3ATestBase { inconsistentPath}; for (Path path : testDirs) { - assertTrue(fs.mkdirs(path)); + assertTrue("Can't create directory: " + path, fs.mkdirs(path)); } clearInconsistency(fs); for (Path path : testDirs) { - assertTrue(fs.delete(path, false)); + assertTrue("Can't delete path: " + path, fs.delete(path, false)); } FileStatus[] paths = fs.listStatus(path("a/b/")); @@ -202,10 +203,13 @@ public class ITestS3GuardListConsistency extends AbstractS3ATestBase { for (FileStatus fileState : paths) { list.add(fileState.getPath()); } - assertFalse(list.contains(path("a/b/dir1"))); - assertFalse(list.contains(path("a/b/dir2"))); - // This should fail without S3Guard, and succeed with it. - assertFalse(list.contains(inconsistentPath)); + + assertFalse("This path should be deleted.", + list.contains(path("a/b/dir1"))); + assertFalse("This path should be deleted.", + list.contains(path("a/b/dir2"))); + assertFalse("This should fail without S3Guard, and succeed with it.", + list.contains(inconsistentPath)); } /** http://git-wip-us.apache.org/repos/asf/hadoop/blob/e148c3ff/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/MetadataStoreTestBase.java ---------------------------------------------------------------------- diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/MetadataStoreTestBase.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/MetadataStoreTestBase.java index 27537c0..799c5a0 100644 --- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/MetadataStoreTestBase.java +++ b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/MetadataStoreTestBase.java @@ -808,6 +808,21 @@ public abstract class MetadataStoreTestBase extends HadoopTestBase { } } + @Test + public void testPutRetainsIsDeletedInParentListing() throws Exception { + final Path path = strToPath("/a/b"); + final FileStatus fileStatus = basicFileStatus(path, 0, false); + PathMetadata pm = new PathMetadata(fileStatus); + pm.setIsDeleted(true); + ms.put(pm); + if(!allowMissing()) { + final PathMetadata pathMetadata = + ms.listChildren(path.getParent()).get(path); + assertTrue("isDeleted should be true on the parent listing", + pathMetadata.isDeleted()); + } + } + /* * Helper functions. */ --------------------------------------------------------------------- To unsubscribe, e-mail: common-commits-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-commits-h...@hadoop.apache.org
