You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-commits@hadoop.apache.org by ay...@apache.org on 2019/04/08 16:15:39 UTC
[hadoop] branch HDFS-13891 updated: HDFS-14369. RBF: Fix trailing /
for webhdfs. Contributed by Akira Ajisaka.
This is an automated email from the ASF dual-hosted git repository.
ayushsaxena pushed a commit to branch HDFS-13891
in repository https://gitbox.apache.org/repos/asf/hadoop.git
The following commit(s) were added to refs/heads/HDFS-13891 by this push:
new e508ab9 HDFS-14369. RBF: Fix trailing / for webhdfs. Contributed by Akira Ajisaka.
e508ab9 is described below
commit e508ab9061355fa123262ba11f4f7dfb1f59ddd7
Author: Ayush Saxena <ay...@apache.org>
AuthorDate: Mon Apr 8 21:43:59 2019 +0530
HDFS-14369. RBF: Fix trailing / for webhdfs. Contributed by Akira Ajisaka.
---
.../federation/resolver/MountTableResolver.java | 14 ++-
.../hadoop/hdfs/tools/federation/RouterAdmin.java | 16 ++-
.../resolver/TestMountTableResolver.java | 123 ++++++++++++++++++---
.../federation/router/TestRouterMountTable.java | 32 ++++++
4 files changed, 160 insertions(+), 25 deletions(-)
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 da58551..03b051d 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
@@ -57,6 +57,7 @@ import org.apache.hadoop.hdfs.server.federation.store.StateStoreUnavailableExcep
import org.apache.hadoop.hdfs.server.federation.store.protocol.GetMountTableEntriesRequest;
import org.apache.hadoop.hdfs.server.federation.store.protocol.GetMountTableEntriesResponse;
import org.apache.hadoop.hdfs.server.federation.store.records.MountTable;
+import org.apache.hadoop.hdfs.tools.federation.RouterAdmin;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -399,12 +400,13 @@ public class MountTableResolver
/**
* Build the path location to insert into the cache atomically. It must hold
* the read lock.
- * @param path Path to check/insert.
+ * @param str Path to check/insert.
* @return New remote location.
* @throws IOException If it cannot find the location.
*/
- public PathLocation lookupLocation(final String path) throws IOException {
+ public PathLocation lookupLocation(final String str) throws IOException {
PathLocation ret = null;
+ final String path = RouterAdmin.normalizeFileSystemPath(str);
MountTable entry = findDeepest(path);
if (entry != null) {
ret = buildLocation(path, entry);
@@ -432,12 +434,13 @@ public class MountTableResolver
*/
public MountTable getMountPoint(final String path) throws IOException {
verifyMountTable();
- return findDeepest(path);
+ return findDeepest(RouterAdmin.normalizeFileSystemPath(path));
}
@Override
- public List<String> getMountPoints(final String path) throws IOException {
+ public List<String> getMountPoints(final String str) throws IOException {
verifyMountTable();
+ final String path = RouterAdmin.normalizeFileSystemPath(str);
Set<String> children = new TreeSet<>();
readLock.lock();
@@ -493,8 +496,7 @@ public class MountTableResolver
*/
public List<MountTable> getMounts(final String path) throws IOException {
verifyMountTable();
-
- return getTreeValues(path, false);
+ return getTreeValues(RouterAdmin.normalizeFileSystemPath(path), false);
}
/**
diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/tools/federation/RouterAdmin.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/tools/federation/RouterAdmin.java
index 9d03a44..8f6d917 100644
--- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/tools/federation/RouterAdmin.java
+++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/tools/federation/RouterAdmin.java
@@ -26,12 +26,12 @@ import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
+import java.util.regex.Pattern;
import org.apache.hadoop.classification.InterfaceAudience.Private;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.conf.Configured;
import org.apache.hadoop.fs.CommonConfigurationKeys;
-import org.apache.hadoop.fs.Path;
import org.apache.hadoop.fs.permission.FsPermission;
import org.apache.hadoop.hdfs.DFSConfigKeys;
import org.apache.hadoop.hdfs.HdfsConfiguration;
@@ -93,6 +93,9 @@ public class RouterAdmin extends Configured implements Tool {
private RouterClient client;
+ /** Pre-compiled regular expressions to detect duplicated slashes. */
+ private static final Pattern SLASHES = Pattern.compile("/+");
+
public static void main(String[] argv) throws Exception {
Configuration conf = new HdfsConfiguration();
RouterAdmin admin = new RouterAdmin(conf);
@@ -1062,12 +1065,15 @@ public class RouterAdmin extends Configured implements Tool {
/**
* Normalize a path for that filesystem.
*
- * @param path Path to normalize.
+ * @param str Path to normalize. The path doesn't have scheme or authority.
* @return Normalized path.
*/
- private static String normalizeFileSystemPath(final String path) {
- Path normalizedPath = new Path(path);
- return normalizedPath.toString();
+ public static String normalizeFileSystemPath(final String str) {
+ String path = SLASHES.matcher(str).replaceAll("/");
+ if (path.length() > 1 && path.endsWith("/")) {
+ path = path.substring(0, path.length()-1);
+ }
+ return path;
}
/**
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 14ccb61..ada2815 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
@@ -185,6 +185,23 @@ public class TestMountTableResolver {
}
@Test
+ public void testDestinationOfConsecutiveSlash() throws IOException {
+ // Check files
+ assertEquals("1->/tesfile1.txt",
+ mountTable.getDestinationForPath("//tesfile1.txt///").toString());
+
+ assertEquals("3->/user/testfile2.txt",
+ mountTable.getDestinationForPath("/user///testfile2.txt").toString());
+
+ assertEquals("2->/user/test/testfile3.txt",
+ mountTable.getDestinationForPath("///user/a/testfile3.txt").toString());
+
+ assertEquals("3->/user/b/testfile4.txt",
+ mountTable.getDestinationForPath("/user/b/testfile4.txt//").toString());
+ }
+
+
+ @Test
public void testDefaultNameServiceEnable() throws IOException {
assertTrue(mountTable.isDefaultNSEnable());
mountTable.setDefaultNameService("3");
@@ -232,62 +249,120 @@ public class TestMountTableResolver {
// Check get the mount table entry for a path
MountTable mtEntry;
mtEntry = mountTable.getMountPoint("/");
- assertTrue(mtEntry.getSourcePath().equals("/"));
+ assertEquals("/", mtEntry.getSourcePath());
mtEntry = mountTable.getMountPoint("/user");
- assertTrue(mtEntry.getSourcePath().equals("/user"));
+ assertEquals("/user", mtEntry.getSourcePath());
mtEntry = mountTable.getMountPoint("/user/a");
- assertTrue(mtEntry.getSourcePath().equals("/user/a"));
+ assertEquals("/user/a", mtEntry.getSourcePath());
mtEntry = mountTable.getMountPoint("/user/a/");
- assertTrue(mtEntry.getSourcePath().equals("/user/a"));
+ assertEquals("/user/a", mtEntry.getSourcePath());
mtEntry = mountTable.getMountPoint("/user/a/11");
- assertTrue(mtEntry.getSourcePath().equals("/user/a"));
+ assertEquals("/user/a", mtEntry.getSourcePath());
mtEntry = mountTable.getMountPoint("/user/a1");
- assertTrue(mtEntry.getSourcePath().equals("/user"));
+ assertEquals("/user", mtEntry.getSourcePath());
+ }
+
+ @Test
+ public void testGetMountPointOfConsecutiveSlashes() throws IOException {
+ // Check get the mount table entry for a path
+ MountTable mtEntry;
+ mtEntry = mountTable.getMountPoint("///");
+ assertEquals("/", mtEntry.getSourcePath());
+
+ mtEntry = mountTable.getMountPoint("///user//");
+ assertEquals("/user", mtEntry.getSourcePath());
+
+ mtEntry = mountTable.getMountPoint("/user///a");
+ assertEquals("/user/a", mtEntry.getSourcePath());
+
+ mtEntry = mountTable.getMountPoint("/user/a////");
+ assertEquals("/user/a", mtEntry.getSourcePath());
+
+ mtEntry = mountTable.getMountPoint("///user/a/11//");
+ assertEquals("/user/a", mtEntry.getSourcePath());
+
+ mtEntry = mountTable.getMountPoint("/user///a1///");
+ assertEquals("/user", mtEntry.getSourcePath());
+ }
+
+ @Test
+ public void testTrailingSlashInInputPath() throws IOException {
+ // Check mount points beneath the path with trailing slash.
+ getMountPoints(true);
}
@Test
public void testGetMountPoints() throws IOException {
+ // Check mount points beneath the path without trailing slash.
+ getMountPoints(false);
+ }
+ private void getMountPoints(boolean trailingSlash) throws IOException {
// Check getting all mount points (virtual and real) beneath a path
List<String> mounts = mountTable.getMountPoints("/");
assertEquals(5, mounts.size());
compareLists(mounts, new String[] {"tmp", "user", "usr",
"readonly", "multi"});
- mounts = mountTable.getMountPoints("/user");
+ String path = trailingSlash ? "/user/" : "/user";
+ mounts = mountTable.getMountPoints(path);
assertEquals(2, mounts.size());
compareLists(mounts, new String[] {"a", "b"});
- mounts = mountTable.getMountPoints("/user/a");
+ path = trailingSlash ? "/user/a/" : "/user/a";
+ mounts = mountTable.getMountPoints(path);
assertEquals(1, mounts.size());
compareLists(mounts, new String[] {"demo"});
- mounts = mountTable.getMountPoints("/user/a/demo");
+ path = trailingSlash ? "/user/a/demo/" : "/user/a/demo";
+ mounts = mountTable.getMountPoints(path);
assertEquals(1, mounts.size());
compareLists(mounts, new String[] {"test"});
- mounts = mountTable.getMountPoints("/user/a/demo/test");
+ path = trailingSlash ? "/user/a/demo/test/" : "/user/a/demo/test";
+ mounts = mountTable.getMountPoints(path);
assertEquals(2, mounts.size());
compareLists(mounts, new String[] {"a", "b"});
- mounts = mountTable.getMountPoints("/tmp");
+ path = trailingSlash ? "/tmp/" : "/tmp";
+ mounts = mountTable.getMountPoints(path);
assertEquals(0, mounts.size());
- mounts = mountTable.getMountPoints("/t");
+ path = trailingSlash ? "/t/" : "/t";
+ mounts = mountTable.getMountPoints(path);
assertNull(mounts);
- mounts = mountTable.getMountPoints("/unknownpath");
+ path = trailingSlash ? "/unknownpath/" : "/unknownpath";
+ mounts = mountTable.getMountPoints(path);
assertNull(mounts);
- mounts = mountTable.getMountPoints("/multi");
+ path = trailingSlash ? "/multi/" : "/multi";
+ mounts = mountTable.getMountPoints(path);
assertEquals(0, mounts.size());
}
+ @Test
+ public void testSuccessiveSlashesInInputPath() throws IOException {
+ // Check getting all mount points (virtual and real) beneath a path
+ List<String> mounts = mountTable.getMountPoints("///");
+ assertEquals(5, mounts.size());
+ compareLists(mounts, new String[] {"tmp", "user", "usr",
+ "readonly", "multi"});
+ String path = "///user//";
+ mounts = mountTable.getMountPoints(path);
+ assertEquals(2, mounts.size());
+ compareLists(mounts, new String[] {"a", "b"});
+ path = "/user///a";
+ mounts = mountTable.getMountPoints(path);
+ assertEquals(1, mounts.size());
+ compareLists(mounts, new String[] {"demo"});
+ }
+
private void compareRecords(List<MountTable> list1, String[] list2) {
assertEquals(list1.size(), list2.length);
for (String item : list2) {
@@ -335,6 +410,26 @@ public class TestMountTableResolver {
}
@Test
+ public void testGetMountsOfConsecutiveSlashes() throws IOException {
+ // Check listing the mount table records at or beneath a path
+ List<MountTable> records = mountTable.getMounts("///");
+ 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", "multi"});
+
+ records = mountTable.getMounts("/user///");
+ assertEquals(5, records.size());
+ compareRecords(records, new String[] {"/user", "/user/a/demo/a",
+ "/user/a/demo/b", "user/a", "/user/b/file1.txt"});
+
+ records = mountTable.getMounts("///user///a");
+ assertEquals(3, records.size());
+ compareRecords(records,
+ new String[] {"/user/a/demo/a", "/user/a/demo/b", "/user/a"});
+ }
+
+ @Test
public void testRemoveSubTree()
throws UnsupportedOperationException, IOException {
diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterMountTable.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterMountTable.java
index 4f6f702..24dfc3f 100644
--- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterMountTable.java
+++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterMountTable.java
@@ -442,4 +442,36 @@ public class TestRouterMountTable {
"Directory/File does not exist /mount/file",
() -> routerFs.setOwner(new Path("/mount/file"), "user", "group"));
}
+
+ /**
+ * Regression test for HDFS-14369.
+ * Verify that getListing works with the path with trailing slash.
+ */
+ @Test
+ public void testGetListingWithTrailingSlash() throws IOException {
+ try {
+ // Add mount table entry
+ MountTable addEntry = MountTable.newInstance("/testlist",
+ Collections.singletonMap("ns0", "/testlist"));
+ assertTrue(addMountTable(addEntry));
+ addEntry = MountTable.newInstance("/testlist/tmp0",
+ Collections.singletonMap("ns0", "/testlist/tmp0"));
+ assertTrue(addMountTable(addEntry));
+ addEntry = MountTable.newInstance("/testlist/tmp1",
+ Collections.singletonMap("ns1", "/testlist/tmp1"));
+ assertTrue(addMountTable(addEntry));
+
+ nnFs0.mkdirs(new Path("/testlist/tmp0"));
+ nnFs1.mkdirs(new Path("/testlist/tmp1"));
+ // Fetch listing
+ DirectoryListing list = routerProtocol.getListing(
+ "/testlist/", HdfsFileStatus.EMPTY_NAME, false);
+ HdfsFileStatus[] statuses = list.getPartialListing();
+ // should return tmp0 and tmp1
+ assertEquals(2, statuses.length);
+ } finally {
+ nnFs0.delete(new Path("/testlist/tmp0"), true);
+ nnFs1.delete(new Path("/testlist/tmp1"), true);
+ }
+ }
}
\ No newline at end of file
---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org