-
Notifications
You must be signed in to change notification settings - Fork 8.9k
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.
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
HDFS-16724. RBF should support get the information about ancestor mount points #4719
Conversation
🎊 +1 overall
This message was automatically generated. |
import java.io.IOException; | ||
|
||
/** | ||
* Exception when no location found. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a better name that reflects this is from the mount point perspective?
Extend the javadoc a little anyway.
@@ -956,7 +959,7 @@ public HdfsFileStatus getFileInfo(String src) throws IOException { | |||
if (children != null && !children.isEmpty()) { | |||
Map<String, Long> dates = getMountPointDates(src); | |||
long date = 0; | |||
if (dates != null && dates.containsKey(src)) { | |||
if (dates.containsKey(src)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would leave the null check; it is good practice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Modified it since getMountPointDates
always return a not null response. I will roll back this modification.
...-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterClientProtocol.java
Show resolved
Hide resolved
@@ -1731,7 +1731,7 @@ protected List<RemoteLocation> getLocationsForPath(String path, | |||
final PathLocation location = | |||
this.subclusterResolver.getDestinationForPath(path); | |||
if (location == null) { | |||
throw new IOException("Cannot find locations for " + path + " in " + | |||
throw new NoLocationException("Cannot find locations for " + path + " in " + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we have a dedicated exception, we can pass the path
and class
parameters and build the message in NoLocationException
public void clearMountTable() throws IOException { | ||
RouterClient client = routerContext.getAdminClient(); | ||
MountTableManager mountTableManager = client.getMountTableManager(); | ||
GetMountTableEntriesRequest req1 = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it fit in a single line?
MountTableManager mountTableManager = client.getMountTableManager(); | ||
AddMountTableEntryRequest addRequest = | ||
AddMountTableEntryRequest.newInstance(entry); | ||
AddMountTableEntryResponse addResponse = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Single line
try { | ||
routerProtocol.getFileInfo("/testdir2"); | ||
fail(); | ||
} catch (IOException ioe) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LambdaTestUtils#intercept
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And check for the exception type.
...-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterClientProtocol.java
Show resolved
Hide resolved
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
@goiri Master, ping. Thanks |
@goiri Master, can help me merge this patch into trunk? Then I will rebase HDFS-16728 and HDFS-16734 on the latest trunk. |
Description of PR
Suppose RBF cluster have 2 nameservices and to mount point as below:
Suppose we disable default nameservice of the RBF cluster. If we try to get the file information of the path /user, RBF will throw one IOException to client due to can not find locations for path /user.
But as this case, RBF should should return one valid response to client, because /user has two sub mount point ns1 and ns2.