goiri commented on a change in pull request #2981:
URL: https://github.com/apache/hadoop/pull/2981#discussion_r627061466
##########
File path:
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/tools/federation/RouterAdmin.java
##########
@@ -1035,7 +1049,56 @@ private boolean updateQuota(String mount, long nsQuota,
long ssQuota)
.updateMountTableEntry(updateRequest);
return updateResponse.getStatus();
}
-
+
+ /**
+ * initViewFsToMountTable.
+ *
+ * @param parameters The specified cluster to initialize.
+ * @param i Index in the parameters
+ * @return If the quota was updated.
+ * @throws IOException Error adding the mount point.
+ */
+ public boolean initViewFsToMountTable(String[] parameters, int i) throws
IOException {
Review comment:
Pass the cluster directly and do the check outside.
##########
File path:
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/tools/federation/RouterAdmin.java
##########
@@ -1035,7 +1049,56 @@ private boolean updateQuota(String mount, long nsQuota,
long ssQuota)
.updateMountTableEntry(updateRequest);
return updateResponse.getStatus();
}
-
+
+ /**
+ * initViewFsToMountTable.
+ *
+ * @param parameters The specified cluster to initialize.
+ * @param i Index in the parameters
+ * @return If the quota was updated.
+ * @throws IOException Error adding the mount point.
+ */
+ public boolean initViewFsToMountTable(String[] parameters, int i) throws
IOException {
+ String clusterName = parameters[i++];
+ if (clusterName == null) {
+ System.out.println("Please enter the cluster name.");
+ return false;
+ }
+ final String mountTablePrefix =
Review comment:
Put a comment with an example of the path.
##########
File path:
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/tools/federation/RouterAdmin.java
##########
@@ -1035,7 +1049,56 @@ private boolean updateQuota(String mount, long nsQuota,
long ssQuota)
.updateMountTableEntry(updateRequest);
return updateResponse.getStatus();
}
-
+
+ /**
+ * initViewFsToMountTable.
+ *
+ * @param parameters The specified cluster to initialize.
+ * @param i Index in the parameters
+ * @return If the quota was updated.
+ * @throws IOException Error adding the mount point.
+ */
+ public boolean initViewFsToMountTable(String[] parameters, int i) throws
IOException {
+ String clusterName = parameters[i++];
+ if (clusterName == null) {
+ System.out.println("Please enter the cluster name.");
+ return false;
+ }
+ final String mountTablePrefix =
+ Constants.CONFIG_VIEWFS_PREFIX + "." + clusterName + "." +
+ Constants.CONFIG_VIEWFS_LINK + "./";
+ Map<String, String> viewFsMap = getConf().getValByRegex(mountTablePrefix);
+ if (viewFsMap.size() == 0) {
Review comment:
.empty()
##########
File path:
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/tools/federation/RouterAdmin.java
##########
@@ -1035,7 +1049,56 @@ private boolean updateQuota(String mount, long nsQuota,
long ssQuota)
.updateMountTableEntry(updateRequest);
return updateResponse.getStatus();
}
-
+
+ /**
+ * initViewFsToMountTable.
+ *
+ * @param parameters The specified cluster to initialize.
+ * @param i Index in the parameters
+ * @return If the quota was updated.
+ * @throws IOException Error adding the mount point.
+ */
+ public boolean initViewFsToMountTable(String[] parameters, int i) throws
IOException {
+ String clusterName = parameters[i++];
+ if (clusterName == null) {
+ System.out.println("Please enter the cluster name.");
+ return false;
+ }
+ final String mountTablePrefix =
+ Constants.CONFIG_VIEWFS_PREFIX + "." + clusterName + "." +
+ Constants.CONFIG_VIEWFS_LINK + "./";
+ Map<String, String> viewFsMap = getConf().getValByRegex(mountTablePrefix);
+ if (viewFsMap.size() == 0) {
+ System.out.println("Please check the cluster name and veiwfs " +
Review comment:
Typo
##########
File path:
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/tools/federation/RouterAdmin.java
##########
@@ -1035,7 +1049,56 @@ private boolean updateQuota(String mount, long nsQuota,
long ssQuota)
.updateMountTableEntry(updateRequest);
return updateResponse.getStatus();
}
-
+
+ /**
+ * initViewFsToMountTable.
+ *
+ * @param parameters The specified cluster to initialize.
+ * @param i Index in the parameters
+ * @return If the quota was updated.
+ * @throws IOException Error adding the mount point.
+ */
+ public boolean initViewFsToMountTable(String[] parameters, int i) throws
IOException {
+ String clusterName = parameters[i++];
+ if (clusterName == null) {
+ System.out.println("Please enter the cluster name.");
+ return false;
+ }
+ final String mountTablePrefix =
+ Constants.CONFIG_VIEWFS_PREFIX + "." + clusterName + "." +
+ Constants.CONFIG_VIEWFS_LINK + "./";
+ Map<String, String> viewFsMap = getConf().getValByRegex(mountTablePrefix);
+ if (viewFsMap.size() == 0) {
+ System.out.println("Please check the cluster name and veiwfs " +
+ "configuration.");
+ }
+ for (String key : viewFsMap.keySet()) {
+ Path path = new Path(viewFsMap.get(key));
+ String owner = null;
Review comment:
Extract this into a method that generates the ACLEntity right away.
##########
File path:
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/tools/federation/RouterAdmin.java
##########
@@ -1035,7 +1049,56 @@ private boolean updateQuota(String mount, long nsQuota,
long ssQuota)
.updateMountTableEntry(updateRequest);
return updateResponse.getStatus();
}
-
+
+ /**
+ * initViewFsToMountTable.
+ *
+ * @param parameters The specified cluster to initialize.
+ * @param i Index in the parameters
+ * @return If the quota was updated.
+ * @throws IOException Error adding the mount point.
+ */
+ public boolean initViewFsToMountTable(String[] parameters, int i) throws
IOException {
+ String clusterName = parameters[i++];
+ if (clusterName == null) {
+ System.out.println("Please enter the cluster name.");
+ return false;
+ }
+ final String mountTablePrefix =
+ Constants.CONFIG_VIEWFS_PREFIX + "." + clusterName + "." +
+ Constants.CONFIG_VIEWFS_LINK + "./";
+ Map<String, String> viewFsMap = getConf().getValByRegex(mountTablePrefix);
+ if (viewFsMap.size() == 0) {
+ System.out.println("Please check the cluster name and veiwfs " +
+ "configuration.");
+ }
+ for (String key : viewFsMap.keySet()) {
+ Path path = new Path(viewFsMap.get(key));
+ String owner = null;
+ String group = null;
+ FsPermission mode = null;
+ try {
+ FileSystem fs = path.getFileSystem(getConf());
+ if (fs.exists(path)) {
+ FileStatus fileStatus = fs.getFileStatus(path);
+ owner = fileStatus.getOwner();
+ group = fileStatus.getGroup();
+ mode = fileStatus.getPermission();
+ }
+ } catch (Exception e) {
+ LOG.warn("Exception encountered", e);
+ }
+ DestinationOrder order = DestinationOrder.HASH;
+ String mount =
+ key.split(clusterName + "." + Constants.CONFIG_VIEWFS_LINK + ".")[1];
+ String dest = path.toUri().getPath();
+ String[] nss = new String[]{path.toUri().getAuthority()};
+ addMount(mount, nss, dest, false, false, order,
Review comment:
Log the mount added.
##########
File path:
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/tools/federation/RouterAdmin.java
##########
@@ -1035,7 +1049,56 @@ private boolean updateQuota(String mount, long nsQuota,
long ssQuota)
.updateMountTableEntry(updateRequest);
return updateResponse.getStatus();
}
-
+
+ /**
+ * initViewFsToMountTable.
+ *
+ * @param parameters The specified cluster to initialize.
+ * @param i Index in the parameters
+ * @return If the quota was updated.
+ * @throws IOException Error adding the mount point.
+ */
+ public boolean initViewFsToMountTable(String[] parameters, int i) throws
IOException {
+ String clusterName = parameters[i++];
+ if (clusterName == null) {
+ System.out.println("Please enter the cluster name.");
+ return false;
+ }
+ final String mountTablePrefix =
+ Constants.CONFIG_VIEWFS_PREFIX + "." + clusterName + "." +
+ Constants.CONFIG_VIEWFS_LINK + "./";
+ Map<String, String> viewFsMap = getConf().getValByRegex(mountTablePrefix);
+ if (viewFsMap.size() == 0) {
+ System.out.println("Please check the cluster name and veiwfs " +
+ "configuration.");
+ }
+ for (String key : viewFsMap.keySet()) {
+ Path path = new Path(viewFsMap.get(key));
+ String owner = null;
+ String group = null;
+ FsPermission mode = null;
+ try {
+ FileSystem fs = path.getFileSystem(getConf());
+ if (fs.exists(path)) {
+ FileStatus fileStatus = fs.getFileStatus(path);
+ owner = fileStatus.getOwner();
+ group = fileStatus.getGroup();
+ mode = fileStatus.getPermission();
+ }
+ } catch (Exception e) {
+ LOG.warn("Exception encountered", e);
Review comment:
Inconsistent to switch between System.out and LOG, choose one.
##########
File path:
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/tools/federation/RouterAdmin.java
##########
@@ -132,7 +136,7 @@ private String getUsage(String cmd) {
{"-add", "-update", "-rm", "-ls", "-getDestination", "-setQuota",
"-setStorageTypeQuota", "-clrQuota", "-clrStorageTypeQuota",
"-safemode", "-nameservice", "-getDisabledNameservices",
- "-refresh", "-refreshRouterArgs",
+ "-refresh", "-initViewFsToMountTable", "-refreshRouterArgs",
Review comment:
Let's add a unit test.
##########
File path:
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/tools/federation/RouterAdmin.java
##########
@@ -1035,7 +1049,56 @@ private boolean updateQuota(String mount, long nsQuota,
long ssQuota)
.updateMountTableEntry(updateRequest);
return updateResponse.getStatus();
}
-
+
+ /**
+ * initViewFsToMountTable.
+ *
+ * @param parameters The specified cluster to initialize.
+ * @param i Index in the parameters
+ * @return If the quota was updated.
+ * @throws IOException Error adding the mount point.
+ */
+ public boolean initViewFsToMountTable(String[] parameters, int i) throws
IOException {
+ String clusterName = parameters[i++];
+ if (clusterName == null) {
+ System.out.println("Please enter the cluster name.");
+ return false;
+ }
+ final String mountTablePrefix =
+ Constants.CONFIG_VIEWFS_PREFIX + "." + clusterName + "." +
+ Constants.CONFIG_VIEWFS_LINK + "./";
+ Map<String, String> viewFsMap = getConf().getValByRegex(mountTablePrefix);
+ if (viewFsMap.size() == 0) {
+ System.out.println("Please check the cluster name and veiwfs " +
+ "configuration.");
+ }
+ for (String key : viewFsMap.keySet()) {
Review comment:
entrySet() given that you are using both key and value
##########
File path:
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/tools/federation/RouterAdmin.java
##########
@@ -1035,7 +1049,56 @@ private boolean updateQuota(String mount, long nsQuota,
long ssQuota)
.updateMountTableEntry(updateRequest);
return updateResponse.getStatus();
}
-
+
+ /**
+ * initViewFsToMountTable.
+ *
+ * @param parameters The specified cluster to initialize.
+ * @param i Index in the parameters
+ * @return If the quota was updated.
+ * @throws IOException Error adding the mount point.
+ */
+ public boolean initViewFsToMountTable(String[] parameters, int i) throws
IOException {
+ String clusterName = parameters[i++];
+ if (clusterName == null) {
+ System.out.println("Please enter the cluster name.");
+ return false;
+ }
+ final String mountTablePrefix =
+ Constants.CONFIG_VIEWFS_PREFIX + "." + clusterName + "." +
+ Constants.CONFIG_VIEWFS_LINK + "./";
+ Map<String, String> viewFsMap = getConf().getValByRegex(mountTablePrefix);
+ if (viewFsMap.size() == 0) {
+ System.out.println("Please check the cluster name and veiwfs " +
+ "configuration.");
+ }
+ for (String key : viewFsMap.keySet()) {
+ Path path = new Path(viewFsMap.get(key));
+ String owner = null;
+ String group = null;
+ FsPermission mode = null;
+ try {
+ FileSystem fs = path.getFileSystem(getConf());
+ if (fs.exists(path)) {
+ FileStatus fileStatus = fs.getFileStatus(path);
+ owner = fileStatus.getOwner();
+ group = fileStatus.getGroup();
+ mode = fileStatus.getPermission();
+ }
+ } catch (Exception e) {
+ LOG.warn("Exception encountered", e);
+ }
+ DestinationOrder order = DestinationOrder.HASH;
+ String mount =
+ key.split(clusterName + "." + Constants.CONFIG_VIEWFS_LINK + ".")[1];
+ String dest = path.toUri().getPath();
Review comment:
Extract path.toUri()
##########
File path:
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/tools/federation/RouterAdmin.java
##########
@@ -384,7 +391,14 @@ public int run(String[] argv) throws Exception {
getDisabledNameservices();
} else if ("-refresh".equals(cmd)) {
refresh(address);
- } else if ("-refreshRouterArgs".equals(cmd)) {
+ } else if ("-initViewFsToMountTable".equals(cmd)) {
+ if (initViewFsToMountTable(argv, i)) {
Review comment:
Can we extract argv[i] (and whatever) here instead of passing a generic
array and an index?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]