Skip to content

HDFS-16216. RBF: Wrong path when get mount point status #3415

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

HDFS-16216. RBF: Wrong path when get mount point status #3415

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions ...f/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterClientProtocol.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -846,7 +846,7 @@ public DirectoryListing getListing(String src, byte[] startAfter,
}
Path childPath = new Path(src, child);
HdfsFileStatus dirStatus =
getMountPointStatus(childPath.toString(), 0, date);
getMountPointStatus(childPath.toString(), 0, date, true);

// if there is no subcluster path, always add mount point
if (lastName == null) {
Expand Down Expand Up @@ -919,10 +919,10 @@ public HdfsFileStatus getFileInfo(String src) throws IOException {
if (dates != null && dates.containsKey(src)) {
date = dates.get(src);
}
ret = getMountPointStatus(src, children.size(), date);
ret = getMountPointStatus(src, children.size(), date, false);
Copy link
Member

@goiri goiri Oct 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like false is kind of the exception for this method.
Can we keep the old signature with true as the default?

} else if (children != null) {
// The src is a mount point, but there are no files or directories
ret = getMountPointStatus(src, 0, 0);
ret = getMountPointStatus(src, 0, 0, false);
}
}

Expand Down Expand Up @@ -1983,11 +1983,12 @@ private static FsPermission getParentPermission(final FsPermission mask) {
* @param name Name of the mount point.
* @param childrenNum Number of children.
* @param date Map with the dates.
* @param appendName If append name.
* @return New HDFS file status representing a mount point.
*/
@VisibleForTesting
HdfsFileStatus getMountPointStatus(
String name, int childrenNum, long date) {
String name, int childrenNum, long date, boolean appendName) {
long modTime = date;
long accessTime = date;
FsPermission permission = FsPermission.getDirDefault();
Expand Down Expand Up @@ -2038,7 +2039,7 @@ HdfsFileStatus getMountPointStatus(
}
long inodeId = 0;
Path path = new Path(name);
String nameStr = path.getName();
String nameStr = appendName ? path.getName() : "";
return new HdfsFileStatus.Builder()
.isdir(true)
.mtime(modTime)
Expand Down
40 changes: 37 additions & 3 deletions ...f/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterMountTable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -356,22 +356,29 @@ public void testGetMountPointStatus() throws IOException {
String child = "testA";
Path childPath = new Path(src, child);
HdfsFileStatus dirStatus =
clientProtocol.getMountPointStatus(childPath.toString(), 0, 0);
clientProtocol.getMountPointStatus(childPath.toString(), 0, 0, true);
assertEquals(child, dirStatus.getLocalName());

String src1 = "/testA";
String child1 = "testB";
Path childPath1 = new Path(src1, child1);
HdfsFileStatus dirStatus1 =
clientProtocol.getMountPointStatus(childPath1.toString(), 0, 0);
clientProtocol.getMountPointStatus(childPath1.toString(), 0, 0, true);
assertEquals(child1, dirStatus1.getLocalName());

String src2 = "/testA/testB";
String child2 = "testC";
Path childPath2 = new Path(src2, child2);
HdfsFileStatus dirStatus2 =
clientProtocol.getMountPointStatus(childPath2.toString(), 0, 0);
clientProtocol.getMountPointStatus(childPath2.toString(), 0, 0, true);
assertEquals(child2, dirStatus2.getLocalName());

String src3 = "/testA/testB/testC";
String child3 = "testD";
Path childPath3 = new Path(src3, child3);
HdfsFileStatus dirStatus3 =
clientProtocol.getMountPointStatus(childPath3.toString(), 0, 0, false);
assertTrue(dirStatus3.getLocalName().length() == 0);
Copy link
Member

@goiri goiri Oct 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assertEquals

Copy link
Member

@goiri goiri Oct 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there's also isEmpty() for the string.
Could we also check for some other attribute other than getLocalName()?

}
/**
* GetListing of testPath through router.
Expand Down Expand Up @@ -763,4 +770,31 @@ public void testListStatusMountPoint() throws Exception {
nnFs0.delete(new Path("/testLsMountEntryDest"), true);
}
}


@Test
public void testGetMountPointFileStatus() throws Exception {

// Add a read only entry
MountTable test = MountTable.newInstance(
"/test", Collections.singletonMap("ns0", "/empty"));
assertTrue(addMountTable(test));
MountTable testFull= MountTable.newInstance(
"/test/test1/test2/test3", Collections.singletonMap("ns0", "/empty"));
assertTrue(addMountTable(testFull));


FileStatus status =
routerFs.getFileStatus(new Path("/test"));
assertEquals("/test", status.getPath().toUri().getPath());

status = routerFs.getFileStatus(new Path("/test/test1"));
assertEquals("/test/test1", status.getPath().toUri().getPath());

status = routerFs.getFileStatus(new Path("/test/test1/test2"));
assertEquals("/test/test1/test2", status.getPath().toUri().getPath());

status = routerFs.getFileStatus(new Path("/test/test1/test2/test3"));
assertEquals("/test/test1/test2/test3", status.getPath().toUri().getPath());
}
}