[ 
https://issues.apache.org/jira/browse/HDFS-13226?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16390537#comment-16390537
 ] 

maobaolong edited comment on HDFS-13226 at 3/8/18 1:06 AM:
-----------------------------------------------------------

[~elgoiri] Yeah, i think throw the IllegalArgumentException is the wise choose, 
indeed, user want to see the first mistake, and the message should be display 
to the user directly.

 

So, my suggestion is that we should change the return type of validate to void.
{code:java}
 @Override
  public void validate() {
    super.validate();
    if (!this.getSourcePath().startsWith("/")
        || this.getSourcePath().startsWith("//")) {
      throw new IllegalArgumentException("Invalid entry, all mount points must 
start with a single / ");
    }
    for (RemoteLocation loc : getDestinations()) {
      String nsId = loc.getNameserviceId();
      if (nsId.length() == 0) {
        throw new IllegalArgumentException("Invalid entry, invalid destination 
nameservice ");
      } else if (!loc.getDest().startsWith("/") || 
this.getSourcePath().startsWith("//")) {
        throw new IllegalArgumentException("Invalid entry, all destination must 
start with a single / ");
      }
    }
  }
{code}
And we have to change the BaseRecord validate method return type and other 
subclass too, or for compromise, we can keep the return value to boolean now? 
As this a big deal, we must think twice.

Maybe we can reference the HAState.checkOperation


was (Author: maobaolong):
[~elgoiri] Yeah, i think throw the IllegalArgumentException is the wise choose, 
indeed, user want to see the first mistake, and the message should be display 
to the user directly.

 

So, my suggestion is that we should change the return type of validate to void.
{code:java}
 @Override
  public void validate() {
    super.validate();
    if (!this.getSourcePath().startsWith("/")
        || this.getSourcePath().startsWith("//")) {
      throw new IllegalArgumentException("Invalid entry, all mount points must 
start with a single / ");
    }
    for (RemoteLocation loc : getDestinations()) {
      String nsId = loc.getNameserviceId();
      if (nsId.length() == 0) {
        throw new IllegalArgumentException("Invalid entry, invalid destination 
nameservice ");
      } else if (!loc.getDest().startsWith("/") || 
this.getSourcePath().startsWith("//")) {
        throw new IllegalArgumentException("Invalid entry, all destination must 
start with a single / ");
      }
    }
  }
{code}
And we have to change the BaseRecord validate method return type and other 
subclass too, or for compromise, we can keep the return value to boolean now? 
As this a big deal, we must think twice.

 

> 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
>
>
> 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

Reply via email to