HDFS-14369. RBF: Fix trailing / for webhdfs. Contributed by Akira Ajisaka.

This commit is contained in:
Ayush Saxena 2019-04-08 21:43:59 +05:30 committed by Brahma Reddy Battula
parent dd8c2b92df
commit 0f9b8d7a75
4 changed files with 160 additions and 25 deletions

View File

@ -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);
}
/**

View File

@ -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;
}
/**

View File

@ -184,6 +184,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());
@ -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) {
@ -334,6 +409,26 @@ public class TestMountTableResolver {
compareRecords(records, new String[] {"/multi"});
}
@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 {

View File

@ -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);
}
}
}