[
https://issues.apache.org/jira/browse/HDFS-13226?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16392308#comment-16392308
]
Yiqun Lin commented on HDFS-13226:
----------------------------------
[~maobaolong], thanks for updating the patch. Besides [~elgoiri]'s review
comments, there are some others comments from me.
Seems this comment didn't addressed.
{quote}1.In MountTable#validate, would you just use throwing exception to
replace the log printing like MembershipState#validate? I think you have
changed some validate logic.
{quote}
I mean we could just throw IllegalArgumentException to replace {{LOG.error}} in
{{MountTable#validate}} like {{MembershipState#validate}} did.
Other comments for the UT in {{TestMountTable}}:
1.Can you rename test method name to {{testValidation}}? It will look more
readable.
2. For the error test cases in test, we could be simplified the codes from
{code:java}
+ try {
+ record = MountTable.newInstance("testValidate", destinations);
+ } catch (Exception e) {
+ assertTrue("Invalid entry, all mount points must start with a single / "
+ .equals(e.getMessage()));
+ }
+ assertEquals(record, null);
{code}
to
{code:java}
try {
MountTable.newInstance("testValidate", destinations);
fail("Mount table entry should be created failed.")
} catch (Exception e) {
GenericTestUtils.assertExceptionContains(
"Invalid entry, all mount points must start with a single / ", e);
}
{code}
The normal test case, the exception catch is no need, can be simplified from
{code:java}
try {
record = MountTable.newInstance("/testValidate", destinations);
} catch (Exception e) {
e.printStackTrace();
}
assertNotNull(record);
{code}
to
{code:java}
record = MountTable.newInstance("/testValidate", destinations);
assertNotNull(record);
{code}
> RBF: We should throw the failure validate and refuse this mount entry
> ---------------------------------------------------------------------
>
> Key: HDFS-13226
> URL: https://issues.apache.org/jira/browse/HDFS-13226
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: hdfs
> Affects Versions: 3.2.0
> Reporter: maobaolong
> Assignee: maobaolong
> Priority: Major
> Labels: RBF
> Fix For: 3.2.0
>
> Attachments: HDFS-13226.001.patch, HDFS-13226.002.patch,
> HDFS-13226.003.patch, HDFS-13226.004.patch, HDFS-13226.005.patch
>
>
> one of the mount entry source path rule is that the source path must start
> with '\', somebody didn't follow the rule and execute the following command:
> {code:bash}
> $ hdfs dfsrouteradmin -add addnode/ ns1 /addnode/
> {code}
> But, the console show we are successful add this entry.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
