HDFS-13845. RBF: The default MountTableResolver should fail resolving multi-destination paths. Contributed by yanghuafeng.
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/0dd9a27f Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/0dd9a27f Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/0dd9a27f Branch: refs/heads/HDFS-13891 Commit: 0dd9a27fdced779c60c16e777aabb050b2da5af2 Parents: b94d1d4 Author: Brahma Reddy Battula <bra...@apache.org> Authored: Tue Oct 30 11:21:08 2018 +0530 Committer: Brahma Reddy Battula <bra...@apache.org> Committed: Tue Nov 13 13:18:57 2018 +0530 ---------------------------------------------------------------------- .../federation/resolver/MountTableResolver.java | 15 +++++-- .../resolver/TestMountTableResolver.java | 45 ++++++++++++++++---- .../router/TestDisableNameservices.java | 36 ++++++++++------ 3 files changed, 70 insertions(+), 26 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/0dd9a27f/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/resolver/MountTableResolver.java ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/resolver/MountTableResolver.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/resolver/MountTableResolver.java index 121469f..9e69840 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/resolver/MountTableResolver.java +++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/resolver/MountTableResolver.java @@ -539,21 +539,28 @@ public class MountTableResolver * @param entry Mount table entry. * @return PathLocation containing the namespace, local path. */ - private static PathLocation buildLocation( - final String path, final MountTable entry) { - + private PathLocation buildLocation( + final String path, final MountTable entry) throws IOException { String srcPath = entry.getSourcePath(); if (!path.startsWith(srcPath)) { LOG.error("Cannot build location, {} not a child of {}", path, srcPath); return null; } + + List<RemoteLocation> dests = entry.getDestinations(); + if (getClass() == MountTableResolver.class && dests.size() > 1) { + throw new IOException("Cannnot build location, " + + getClass().getSimpleName() + + " should not resolve multiple destinations for " + path); + } + String remainingPath = path.substring(srcPath.length()); if (remainingPath.startsWith(Path.SEPARATOR)) { remainingPath = remainingPath.substring(1); } List<RemoteLocation> locations = new LinkedList<>(); - for (RemoteLocation oneDst : entry.getDestinations()) { + for (RemoteLocation oneDst : dests) { String nsId = oneDst.getNameserviceId(); String dest = oneDst.getDest(); String newPath = dest; http://git-wip-us.apache.org/repos/asf/hadoop/blob/0dd9a27f/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/resolver/TestMountTableResolver.java ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/resolver/TestMountTableResolver.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/resolver/TestMountTableResolver.java index 5e3b861..14ccb61 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/resolver/TestMountTableResolver.java +++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/resolver/TestMountTableResolver.java @@ -79,6 +79,8 @@ public class TestMountTableResolver { * __usr * ____bin -> 2:/bin * __readonly -> 2:/tmp + * __multi -> 5:/dest1 + * 6:/dest2 * * @throws IOException If it cannot set the mount table. */ @@ -126,6 +128,12 @@ public class TestMountTableResolver { MountTable readOnlyEntry = MountTable.newInstance("/readonly", map); readOnlyEntry.setReadOnly(true); mountTable.addEntry(readOnlyEntry); + + // /multi + map = getMountTableEntry("5", "/dest1"); + map.put("6", "/dest2"); + MountTable multiEntry = MountTable.newInstance("/multi", map); + mountTable.addEntry(multiEntry); } @Before @@ -201,6 +209,17 @@ public class TestMountTableResolver { } } + @Test + public void testMuiltipleDestinations() throws IOException { + try { + mountTable.getDestinationForPath("/multi"); + fail("The getDestinationForPath call should fail."); + } catch (IOException ioe) { + GenericTestUtils.assertExceptionContains( + "MountTableResolver should not resolve multiple destinations", ioe); + } + } + private void compareLists(List<String> list1, String[] list2) { assertEquals(list1.size(), list2.length); for (String item : list2) { @@ -236,8 +255,9 @@ public class TestMountTableResolver { // Check getting all mount points (virtual and real) beneath a path List<String> mounts = mountTable.getMountPoints("/"); - assertEquals(4, mounts.size()); - compareLists(mounts, new String[] {"tmp", "user", "usr", "readonly"}); + assertEquals(5, mounts.size()); + compareLists(mounts, new String[] {"tmp", "user", "usr", + "readonly", "multi"}); mounts = mountTable.getMountPoints("/user"); assertEquals(2, mounts.size()); @@ -263,6 +283,9 @@ public class TestMountTableResolver { mounts = mountTable.getMountPoints("/unknownpath"); assertNull(mounts); + + mounts = mountTable.getMountPoints("/multi"); + assertEquals(0, mounts.size()); } private void compareRecords(List<MountTable> list1, String[] list2) { @@ -282,10 +305,10 @@ public class TestMountTableResolver { // Check listing the mount table records at or beneath a path List<MountTable> records = mountTable.getMounts("/"); - assertEquals(9, records.size()); + assertEquals(10, records.size()); compareRecords(records, new String[] {"/", "/tmp", "/user", "/usr/bin", "user/a", "/user/a/demo/a", "/user/a/demo/b", "/user/b/file1.txt", - "readonly"}); + "readonly", "multi"}); records = mountTable.getMounts("/user"); assertEquals(5, records.size()); @@ -305,6 +328,10 @@ public class TestMountTableResolver { assertEquals(1, records.size()); compareRecords(records, new String[] {"/readonly"}); assertTrue(records.get(0).isReadOnly()); + + records = mountTable.getMounts("/multi"); + assertEquals(1, records.size()); + compareRecords(records, new String[] {"/multi"}); } @Test @@ -313,7 +340,7 @@ public class TestMountTableResolver { // 3 mount points are present /tmp, /user, /usr compareLists(mountTable.getMountPoints("/"), - new String[] {"user", "usr", "tmp", "readonly"}); + new String[] {"user", "usr", "tmp", "readonly", "multi"}); // /tmp currently points to namespace 2 assertEquals("2", mountTable.getDestinationForPath("/tmp/testfile.txt") @@ -324,7 +351,7 @@ public class TestMountTableResolver { // Now 2 mount points are present /user, /usr compareLists(mountTable.getMountPoints("/"), - new String[] {"user", "usr", "readonly"}); + new String[] {"user", "usr", "readonly", "multi"}); // /tmp no longer exists, uses default namespace for mapping / assertEquals("1", mountTable.getDestinationForPath("/tmp/testfile.txt") @@ -337,7 +364,7 @@ public class TestMountTableResolver { // 3 mount points are present /tmp, /user, /usr compareLists(mountTable.getMountPoints("/"), - new String[] {"user", "usr", "tmp", "readonly"}); + new String[] {"user", "usr", "tmp", "readonly", "multi"}); // /usr is virtual, uses namespace 1->/ assertEquals("1", mountTable.getDestinationForPath("/usr/testfile.txt") @@ -348,7 +375,7 @@ public class TestMountTableResolver { // Verify the remove failed compareLists(mountTable.getMountPoints("/"), - new String[] {"user", "usr", "tmp", "readonly"}); + new String[] {"user", "usr", "tmp", "readonly", "multi"}); } @Test @@ -380,7 +407,7 @@ public class TestMountTableResolver { // Initial table loaded testDestination(); - assertEquals(9, mountTable.getMounts("/").size()); + assertEquals(10, mountTable.getMounts("/").size()); // Replace table with /1 and /2 List<MountTable> records = new ArrayList<>(); http://git-wip-us.apache.org/repos/asf/hadoop/blob/0dd9a27f/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestDisableNameservices.java ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestDisableNameservices.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestDisableNameservices.java index 15b104d..610927d 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestDisableNameservices.java +++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestDisableNameservices.java @@ -21,6 +21,7 @@ import static org.apache.hadoop.hdfs.server.federation.FederationTestUtils.simul import static org.apache.hadoop.util.Time.monotonicNow; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import java.io.IOException; import java.util.Iterator; @@ -43,13 +44,13 @@ import org.apache.hadoop.hdfs.server.federation.metrics.FederationMetrics; import org.apache.hadoop.hdfs.server.federation.resolver.MembershipNamenodeResolver; import org.apache.hadoop.hdfs.server.federation.resolver.MountTableManager; import org.apache.hadoop.hdfs.server.federation.resolver.MountTableResolver; -import org.apache.hadoop.hdfs.server.federation.resolver.order.DestinationOrder; import org.apache.hadoop.hdfs.server.federation.store.DisabledNameserviceStore; import org.apache.hadoop.hdfs.server.federation.store.StateStoreService; import org.apache.hadoop.hdfs.server.federation.store.protocol.AddMountTableEntryRequest; import org.apache.hadoop.hdfs.server.federation.store.protocol.DisableNameserviceRequest; import org.apache.hadoop.hdfs.server.federation.store.records.MountTable; import org.apache.hadoop.hdfs.server.namenode.NameNode; +import org.apache.hadoop.test.GenericTestUtils; import org.codehaus.jettison.json.JSONObject; import org.junit.After; import org.junit.AfterClass; @@ -106,14 +107,18 @@ public class TestDisableNameservices { // Setup a mount table to map to the two namespaces MountTableManager mountTable = routerAdminClient.getMountTableManager(); Map<String, String> destinations = new TreeMap<>(); - destinations.put("ns0", "/"); - destinations.put("ns1", "/"); - MountTable newEntry = MountTable.newInstance("/", destinations); - newEntry.setDestOrder(DestinationOrder.RANDOM); + destinations.put("ns0", "/dirns0"); + MountTable newEntry = MountTable.newInstance("/dirns0", destinations); AddMountTableEntryRequest request = AddMountTableEntryRequest.newInstance(newEntry); mountTable.addMountTableEntry(request); + destinations = new TreeMap<>(); + destinations.put("ns1", "/dirns1"); + newEntry = MountTable.newInstance("/dirns1", destinations); + request = AddMountTableEntryRequest.newInstance(newEntry); + mountTable.addMountTableEntry(request); + // Refresh the cache in the Router Router router = routerContext.getRouter(); MountTableResolver mountTableResolver = @@ -122,9 +127,9 @@ public class TestDisableNameservices { // Add a folder to each namespace NamenodeContext nn0 = cluster.getNamenode("ns0", null); - nn0.getFileSystem().mkdirs(new Path("/dirns0")); + nn0.getFileSystem().mkdirs(new Path("/dirns0/0")); NamenodeContext nn1 = cluster.getNamenode("ns1", null); - nn1.getFileSystem().mkdirs(new Path("/dirns1")); + nn1.getFileSystem().mkdirs(new Path("/dirns1/1")); } @AfterClass @@ -153,14 +158,12 @@ public class TestDisableNameservices { @Test public void testWithoutDisabling() throws IOException { - // ns0 is slow and renewLease should take a long time long t0 = monotonicNow(); routerProtocol.renewLease("client0"); long t = monotonicNow() - t0; assertTrue("It took too little: " + t + "ms", t > TimeUnit.SECONDS.toMillis(1)); - // Return the results from all subclusters even if slow FileSystem routerFs = routerContext.getFileSystem(); FileStatus[] filesStatus = routerFs.listStatus(new Path("/")); @@ -171,7 +174,6 @@ public class TestDisableNameservices { @Test public void testDisabling() throws Exception { - disableNameservice("ns0"); // renewLease should be fast as we are skipping ns0 @@ -180,12 +182,20 @@ public class TestDisableNameservices { long t = monotonicNow() - t0; assertTrue("It took too long: " + t + "ms", t < TimeUnit.SECONDS.toMillis(1)); - // We should not report anything from ns0 FileSystem routerFs = routerContext.getFileSystem(); - FileStatus[] filesStatus = routerFs.listStatus(new Path("/")); + FileStatus[] filesStatus = null; + try { + routerFs.listStatus(new Path("/")); + fail("The listStatus call should fail."); + } catch (IOException ioe) { + GenericTestUtils.assertExceptionContains( + "No remote locations available", ioe); + } + + filesStatus = routerFs.listStatus(new Path("/dirns1")); assertEquals(1, filesStatus.length); - assertEquals("dirns1", filesStatus[0].getPath().getName()); + assertEquals("1", filesStatus[0].getPath().getName()); } @Test --------------------------------------------------------------------- To unsubscribe, e-mail: common-commits-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-commits-h...@hadoop.apache.org
