-
Notifications
You must be signed in to change notification settings - Fork 8.8k
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-16728. RBF throw IndexOutOfBoundsException with disableNameServices #4734
Conversation
🎊 +1 overall
This message was automatically generated. |
@@ -1765,6 +1765,9 @@ protected List<RemoteLocation> getLocationsForPath(String path, | |||
locs.add(loc); | |||
} | |||
} | |||
if (locs.isEmpty()) { | |||
throw new NoLocationException(path, this.subclusterResolver.getClass().getSimpleName()); |
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.
Cleaner to pass this.subclusterResolver.getClass() and do the getSimpleName() in the exception.
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.
fixed
try { | ||
FsPermission permission = new FsPermission("777"); | ||
LambdaTestUtils.intercept(NoLocationException.class, | ||
() -> router.getRouter().getRpcServer() |
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.
Extract router.getRouter().getRpcServer() and we can make this fit in one line.
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.
fixed
*/ | ||
public class NoLocationException extends IOException { | ||
|
||
private static final long serialVersionUID = 1L; |
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 this part of the code make sense? Why do we assign a value of 1?
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.
Yes, serialVersionUID is a universal version identifier for a Serializable class. It's useful for class serialization and deserialization. And 1L has no special meaning. It is just a number that will match 1L as the "other" version id.
The serialVersionUID is a universal version identifier for a Serializable class. Deserialization uses this number to ensure that a loaded class corresponds exactly to a serialized object. If no match is found, then an InvalidClassException is thrown.
@goiri @slfan1989 Master, I have updated this patch, please help me review it again. Thanks |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
@goiri @slfan1989 Master, ping, please help me review it again? Thanks |
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.
Great catch. LGTM. +1 from my side. Thanks @ZanderXu .
@goiri @Hexiaoqiao I have rebased this patch based on the latest trunk. Please help me review and merge it into trunk. Then I will rebase HDFS-16734 on the latest trunk. |
🎊 +1 overall
This message was automatically generated. |
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.
LGTM
…ces (apache#4734). Contributed by ZanderXu. Reviewed-by: He Xiaoqiao <hexiaoqiao@apache.org> Reviewed-by: Inigo Goiri <inigoiri@apache.org> Signed-off-by: Ayush Saxena <ayushsaxena@apache.org>
Description of PR
RBF will throw an IndexOutOfBoundsException when the namespace is disabled.
Suppose we have a mount point /a/b -> ns0 -> /a/b and disabled the ns0.
RBF will throw IndexOutOfBoundsException during handling requests with path starting with /a/b.