Repository: hadoop Updated Branches: refs/heads/HADOOP-13345 1b27f15d1 -> 31e737be0
HADOOP-14323. ITestS3GuardListConsistency failure w/ Local, authoritative metadata store. Contributed by Aaron Fabbri Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/31e737be Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/31e737be Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/31e737be Branch: refs/heads/HADOOP-13345 Commit: 31e737be09bbbec25a9f16456810fde24ad2e0e7 Parents: 1b27f15 Author: Mingliang Liu <lium...@apache.org> Authored: Wed Apr 26 13:35:21 2017 -0700 Committer: Mingliang Liu <lium...@apache.org> Committed: Wed Apr 26 13:35:21 2017 -0700 ---------------------------------------------------------------------- .../fs/s3a/s3guard/DirListingMetadata.java | 10 +++ .../fs/s3a/s3guard/LocalMetadataStore.java | 3 +- .../fs/s3a/ITestS3GuardListConsistency.java | 79 ++++++++++++++------ 3 files changed, 69 insertions(+), 23 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/31e737be/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/DirListingMetadata.java ---------------------------------------------------------------------- diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/DirListingMetadata.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/DirListingMetadata.java index ba6d1a6..f13b447 100644 --- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/DirListingMetadata.java +++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/DirListingMetadata.java @@ -81,6 +81,16 @@ public class DirListingMetadata { } /** + * Copy constructor. + * @param d the existing {@link DirListingMetadata} object. + */ + public DirListingMetadata(DirListingMetadata d) { + path = d.path; + isAuthoritative = d.isAuthoritative; + listMap = new ConcurrentHashMap<>(d.listMap); + } + + /** * @return {@code Path} of the directory that contains this listing. */ public Path getPath() { http://git-wip-us.apache.org/repos/asf/hadoop/blob/31e737be/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 165ed5e..52e5b2a 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 @@ -169,7 +169,8 @@ public class LocalMetadataStore implements MetadataStore { LOG.debug("listChildren({}) -> {}", path, listing == null ? "null" : listing.prettyPrint()); } - return listing; + // Make a copy so callers can mutate without affecting our state + return listing == null ? null : new DirListingMetadata(listing); } @Override http://git-wip-us.apache.org/repos/asf/hadoop/blob/31e737be/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 47d88073..5e83906 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 @@ -27,10 +27,12 @@ import org.apache.hadoop.fs.RemoteIterator; import org.apache.hadoop.fs.contract.AbstractFSContract; import org.apache.hadoop.fs.contract.s3a.S3AContract; import org.apache.hadoop.fs.s3a.s3guard.DirListingMetadata; -import org.apache.hadoop.fs.s3a.s3guard.S3Guard; import org.junit.Assume; import org.junit.Test; +import java.io.FileNotFoundException; +import java.io.IOException; +import java.net.URI; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -265,6 +267,49 @@ public class ITestS3GuardListConsistency extends AbstractS3ATestBase { } } + private static S3AFileSystem asS3AFS(FileSystem fs) { + assertTrue("Not a S3AFileSystem: " + fs, fs instanceof S3AFileSystem); + return (S3AFileSystem)fs; + } + + /** Create a separate S3AFileSystem instance for testing. */ + private S3AFileSystem createTestFS(URI fsURI, boolean disableS3Guard, + boolean authoritativeMeta) + throws IOException { + Configuration conf; + + // Create a FileSystem that is S3-backed only + conf = createConfiguration(); + S3ATestUtils.disableFilesystemCaching(conf); + if (disableS3Guard) { + conf.set(Constants.S3_METADATA_STORE_IMPL, + Constants.S3GUARD_METASTORE_NULL); + } else { + S3ATestUtils.maybeEnableS3Guard(conf); + conf.setBoolean(Constants.METADATASTORE_AUTHORITATIVE, authoritativeMeta); + } + FileSystem fs = FileSystem.get(fsURI, conf); + return asS3AFS(fs); + } + + private static void assertPathDoesntExist(FileSystem fs, Path p) + throws IOException { + try { + FileStatus s = fs.getFileStatus(p); + } catch (FileNotFoundException e) { + return; + } + fail("Path should not exist: " + p); + } + + /** + * In listStatus(), when S3Guard is enabled, the full listing for a + * directory is "written back" to the MetadataStore before the listing is + * returned. Currently this "write back" behavior occurs when + * fs.s3a.metadatastore.authoritative is true. This test validates this + * behavior. + * @throws Exception on failure + */ @Test public void testListStatusWriteBack() throws Exception { Assume.assumeTrue(getFileSystem().hasMetadataStore()); @@ -272,33 +317,23 @@ public class ITestS3GuardListConsistency extends AbstractS3ATestBase { Configuration conf; Path directory = path("ListStatusWriteBack"); - // Create a FileSystem that is S3-backed only - conf = createConfiguration(); - conf.setBoolean("fs.s3a.impl.disable.cache", true); - conf.set(Constants.S3_METADATA_STORE_IMPL, - Constants.S3GUARD_METASTORE_NULL); - FileSystem noS3Guard = FileSystem.get(directory.toUri(), conf); + // "raw" S3AFileSystem without S3Guard + S3AFileSystem noS3Guard = createTestFS(directory.toUri(), true, false); - // Create a FileSystem with S3Guard and write-back disabled - conf = createConfiguration(); - S3ATestUtils.maybeEnableS3Guard(conf); - conf.setBoolean("fs.s3a.impl.disable.cache", true); - conf.setBoolean(Constants.METADATASTORE_AUTHORITATIVE, false); - FileSystem noWriteBack = FileSystem.get(directory.toUri(), conf); + // Another with S3Guard and write-back disabled + S3AFileSystem noWriteBack = createTestFS(directory.toUri(), false, false); - // Create a FileSystem with S3Guard and write-back enabled - conf = createConfiguration(); - S3ATestUtils.maybeEnableS3Guard(conf); - conf.setBoolean("fs.s3a.impl.disable.cache", true); - conf.setBoolean(Constants.METADATASTORE_AUTHORITATIVE, true); - FileSystem yesWriteBack = FileSystem.get(directory.toUri(), conf); + // Another S3Guard and write-back enabled + S3AFileSystem yesWriteBack = createTestFS(directory.toUri(), false, true); // delete the existing directory (in case of last test failure) noS3Guard.delete(directory, true); // Create a directory on S3 only noS3Guard.mkdirs(new Path(directory, "OnS3")); // Create a directory on both S3 and metadata store - noWriteBack.mkdirs(new Path(directory, "OnS3AndMS")); + Path p = new Path(directory, "OnS3AndMS"); + assertPathDoesntExist(noWriteBack, p); + noWriteBack.mkdirs(p); FileStatus[] fsResults; DirListingMetadata mdResults; @@ -311,7 +346,7 @@ public class ITestS3GuardListConsistency extends AbstractS3ATestBase { // Metadata store without write-back should still only contain /OnS3AndMS, // because newly discovered /OnS3 is not written back to metadata store - mdResults = S3Guard.getMetadataStore(noWriteBack).listChildren(directory); + mdResults = noWriteBack.getMetadataStore().listChildren(directory); assertEquals("Metadata store without write back should still only know " + "about /OnS3AndMS, but it has: " + mdResults, 1, mdResults.numEntries()); @@ -324,7 +359,7 @@ public class ITestS3GuardListConsistency extends AbstractS3ATestBase { // Metadata store with write-back should contain both because the newly // discovered /OnS3 should have been written back to metadata store - mdResults = S3Guard.getMetadataStore(yesWriteBack).listChildren(directory); + mdResults = yesWriteBack.getMetadataStore().listChildren(directory); assertEquals("Unexpected number of results from metadata store. " + "Should have /OnS3 and /OnS3AndMS: " + mdResults, 2, mdResults.numEntries()); --------------------------------------------------------------------- To unsubscribe, e-mail: common-commits-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-commits-h...@hadoop.apache.org
