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

ASF GitHub Bot commented on HDFS-16978:
---------------------------------------

goiri commented on code in PR #5554:
URL: https://github.com/apache/hadoop/pull/5554#discussion_r1167244069


##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/tools/federation/RouterAdmin.java:
##########
@@ -511,90 +687,45 @@ private boolean refreshRouterCache() throws IOException {
    * @throws IOException If it cannot add the mount point.
    */
   public boolean addMount(String[] parameters, int i) throws IOException {
-    // Mandatory parameters
-    String mount = parameters[i++];
-    String[] nss = parameters[i++].split(",");
-    String dest = parameters[i++];
-
-    // Optional parameters
-    boolean readOnly = false;
-    boolean faultTolerant = false;
-    String owner = null;
-    String group = null;
-    FsPermission mode = null;
-    DestinationOrder order = DestinationOrder.HASH;
-    while (i < parameters.length) {
-      if (parameters[i].equals("-readonly")) {
-        readOnly = true;
-      } else if (parameters[i].equals("-faulttolerant")) {
-        faultTolerant = true;
-      } else if (parameters[i].equals("-order")) {
-        i++;
-        try {
-          order = DestinationOrder.valueOf(parameters[i]);
-        } catch(Exception e) {
-          System.err.println("Cannot parse order: " + parameters[i]);
-        }
-      } else if (parameters[i].equals("-owner")) {
-        i++;
-        owner = parameters[i];
-      } else if (parameters[i].equals("-group")) {
-        i++;
-        group = parameters[i];
-      } else if (parameters[i].equals("-mode")) {
-        i++;
-        short modeValue = Short.parseShort(parameters[i], 8);
-        mode = new FsPermission(modeValue);
-      } else {
-        printUsage("-add");
-        return false;
-      }
-
-      i++;
+    AddMountAttributes addMountAttributes = getAddMountAttributes(parameters, 
i, false);
+    if (addMountAttributes == null) {
+      return false;
     }
-
-    return addMount(mount, nss, dest, readOnly, faultTolerant, order,
-        new ACLEntity(owner, group, mode));
+    return addMount(addMountAttributes);
   }
 
   /**
    * Add a mount table entry or update if it exists.
    *
-   * @param mount Mount point.
-   * @param nss Namespaces where this is mounted to.
-   * @param dest Destination path.
-   * @param readonly If the mount point is read only.
-   * @param order Order of the destination locations.
-   * @param aclInfo the ACL info for mount point.
+   * @param addMountAttributes attributes associated with add mount point 
request.
    * @return If the mount point was added.
    * @throws IOException Error adding the mount point.
    */
-  public boolean addMount(String mount, String[] nss, String dest,
-      boolean readonly, boolean faultTolerant, DestinationOrder order,
-      ACLEntity aclInfo)
+  public boolean addMount(AddMountAttributes addMountAttributes)
       throws IOException {
-    mount = normalizeFileSystemPath(mount);
+    String mount = normalizeFileSystemPath(addMountAttributes.getMount());
     // Get the existing entry
     MountTableManager mountTable = client.getMountTableManager();
     MountTable existingEntry = getMountEntry(mount, mountTable);
 
     if (existingEntry == null) {
       // Create and add the entry if it doesn't exist
       Map<String, String> destMap = new LinkedHashMap<>();
-      for (String ns : nss) {
-        destMap.put(ns, dest);
+      for (String ns : addMountAttributes.getNss()) {
+        destMap.put(ns, addMountAttributes.getDest());
       }
       MountTable newEntry = MountTable.newInstance(mount, destMap);
-      if (readonly) {
+      if (addMountAttributes.isReadonly()) {
         newEntry.setReadOnly(true);

Review Comment:
   Couldn't you do this as part of AddMountAttributes?
   Like getMountTableEntry()?





> RBF: New Router admin command to support bulk add of mount points
> -----------------------------------------------------------------
>
>                 Key: HDFS-16978
>                 URL: https://issues.apache.org/jira/browse/HDFS-16978
>             Project: Hadoop HDFS
>          Issue Type: Improvement
>            Reporter: Viraj Jasani
>            Assignee: Viraj Jasani
>            Priority: Minor
>              Labels: pull-request-available
>
> All state store implementations support adding multiple state store records 
> using single putAll() implementation. We should provide new router admin API 
> to support bulk addition of mount table entries that can utilize this build 
> add implementation at state store level.
> For more than one mount point to be added, the goal of bulk addition should be
>  # To reduce frequent router calls
>  # To avoid frequent state store cache refreshers with each single mount 
> point addition



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

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