[GitHub] [hadoop] ayushtkn commented on a diff in pull request #4990: HDFS-13507. RBF RouterAdmin disable update functionality in add cmd and supports add/update one mount table with different destina
ayushtkn commented on code in PR #4990:
URL: https://github.com/apache/hadoop/pull/4990#discussion_r1211229496
##
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/tools/federation/RouterAdmin.java:
##
@@ -699,32 +697,52 @@ public boolean addMount(AddMountAttributes
addMountAttributes)
// Get the existing entry
MountTableManager mountTable = client.getMountTableManager();
MountTable existingEntry = getMountEntry(mount, mountTable);
-MountTable existingOrNewEntry =
-
addMountAttributes.getNewOrUpdatedMountTableEntryWithAttributes(existingEntry);
-if (existingOrNewEntry == null) {
+if (existingEntry != null) {
+ System.err.println("MountTable entry:" + mount + " already exists.");
return false;
}
-if (existingEntry == null) {
- AddMountTableEntryRequest request = AddMountTableEntryRequest
- .newInstance(existingOrNewEntry);
- AddMountTableEntryResponse addResponse =
mountTable.addMountTableEntry(request);
- boolean added = addResponse.getStatus();
- if (!added) {
-System.err.println("Cannot add mount point " + mount);
- }
- return added;
-} else {
- UpdateMountTableEntryRequest updateRequest =
- UpdateMountTableEntryRequest.newInstance(existingOrNewEntry);
- UpdateMountTableEntryResponse updateResponse =
- mountTable.updateMountTableEntry(updateRequest);
- boolean updated = updateResponse.getStatus();
- if (!updated) {
-System.err.println("Cannot update mount point " + mount);
- }
- return updated;
+MountTable mountEntry =
addMountAttributes.getMountTableEntryWithAttributes();
+AddMountTableEntryRequest request =
AddMountTableEntryRequest.newInstance(mountEntry);
+AddMountTableEntryResponse addResponse =
mountTable.addMountTableEntry(request);
+boolean added = addResponse.getStatus();
+if (!added) {
+ System.err.println("Cannot add mount point " + mount);
}
+return added;
+ }
+
+ /**
+ * Return the map from namespace to destination.
+ * @param nss input namespaces.
+ * @param destinations input destinations.
+ * @return one map from namespace to destination.
+ * @throws IOException throw IOException if the destinations is invalida.
Review Comment:
typo ``invalida``
##
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/tools/federation/RouterAdmin.java:
##
@@ -699,32 +697,52 @@ public boolean addMount(AddMountAttributes
addMountAttributes)
// Get the existing entry
MountTableManager mountTable = client.getMountTableManager();
MountTable existingEntry = getMountEntry(mount, mountTable);
-MountTable existingOrNewEntry =
-
addMountAttributes.getNewOrUpdatedMountTableEntryWithAttributes(existingEntry);
-if (existingOrNewEntry == null) {
+if (existingEntry != null) {
+ System.err.println("MountTable entry:" + mount + " already exists.");
return false;
}
-if (existingEntry == null) {
- AddMountTableEntryRequest request = AddMountTableEntryRequest
- .newInstance(existingOrNewEntry);
- AddMountTableEntryResponse addResponse =
mountTable.addMountTableEntry(request);
- boolean added = addResponse.getStatus();
- if (!added) {
-System.err.println("Cannot add mount point " + mount);
- }
- return added;
-} else {
- UpdateMountTableEntryRequest updateRequest =
- UpdateMountTableEntryRequest.newInstance(existingOrNewEntry);
- UpdateMountTableEntryResponse updateResponse =
- mountTable.updateMountTableEntry(updateRequest);
- boolean updated = updateResponse.getStatus();
- if (!updated) {
-System.err.println("Cannot update mount point " + mount);
- }
- return updated;
+MountTable mountEntry =
addMountAttributes.getMountTableEntryWithAttributes();
+AddMountTableEntryRequest request =
AddMountTableEntryRequest.newInstance(mountEntry);
+AddMountTableEntryResponse addResponse =
mountTable.addMountTableEntry(request);
+boolean added = addResponse.getStatus();
+if (!added) {
+ System.err.println("Cannot add mount point " + mount);
}
+return added;
+ }
+
+ /**
+ * Return the map from namespace to destination.
+ * @param nss input namespaces.
+ * @param destinations input destinations.
+ * @return one map from namespace to destination.
+ * @throws IOException throw IOException if the destinations is invalida.
+ */
+ public static Map getDestMap(String[] nss, String[]
destinations)
+ throws IOException {
+if (isInvalidDestinations(nss, destinations)) {
+ String message = "Invalid number of namespaces and destinations. The
number of destinations: "
+ + destinations.length + " is not equal to the number of namespaces:
" + nss.length;
+ throw new IOException(message);
+}
+Map destMap = n
[GitHub] [hadoop] ayushtkn commented on a diff in pull request #4990: HDFS-13507. RBF RouterAdmin disable update functionality in add cmd and supports add/update one mount table with different destina
ayushtkn commented on code in PR #4990: URL: https://github.com/apache/hadoop/pull/4990#discussion_r1203379687 ## hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterAdminServer.java: ## @@ -425,6 +426,18 @@ private void verifyFileExistenceInDest(MountTable mountTable) throws IOException } } + private void verifyMountTableExistence(MountTable mountTable) Review Comment: Add a javadoc there is a method already in class ``namespaceExists`` so can we do something like, ``VerifyMountEntryExists`` or something like that ## hadoop-hdfs-project/hadoop-hdfs/src/site/markdown/HDFSCommands.md: ## @@ -463,24 +464,24 @@ Usage: [-refreshSuperUserGroupsConfiguration] [-refreshCallQueue] -| COMMAND\_OPTION | Description | -|: |: | -| `-add` *source* *nameservices* *destination* | Add a mount table entry or update if it exists. | -| `-update` *source* *nameservices* *destination* | Update a mount table entry attributes. | -| `-rm` *source* | Remove mount point of specified path. | -| `-ls` `[-d]` *path* | List mount points under specified path. Specify -d parameter to get detailed listing.| -| `-getDestination` *path* | Get the subcluster where a file is or should be created. | -| `-setQuota` *path* `-nsQuota` *nsQuota* `-ssQuota` *ssQuota* | Set quota for specified path. See [HDFS Quotas Guide](./HdfsQuotaAdminGuide.html) for the quota detail. | -| `-setStorageTypeQuota` *path* `-storageType` *storageType* *stQuota* | Set storage type quota for specified path. See [HDFS Quotas Guide](./HdfsQuotaAdminGuide.html) for the quota detail. | -| `-clrQuota` *path* | Clear quota of given mount point. See [HDFS Quotas Guide](./HdfsQuotaAdminGuide.html) for the quota detail. | -| `-clrStorageTypeQuota` *path* | Clear storage type quota of given mount point. See [HDFS Quotas Guide](./HdfsQuotaAdminGuide.html) for the quota detail. | -| `-safemode` `enter` `leave` `get` | Manually set the Router entering or leaving safe mode. The option *get* will be used for verifying if the Router is in safe mode state. | -| `-nameservice` `disable` `enable` *nameservice* | Disable/enable a name service from the federation. If disabled, requests will not go to that name service. | -| `-getDisabledNameservices` | Get the name services that are disabled in the federation. | -| `-refresh` | Update mount table cache of the connected router. | +| COMMAND\_OPTION | Description | +|: |:--| +| `-add` *source* *nameservices* *destination* | Add a mount table entry. | +| `-update` *source* *nameservices* *destination* | Update a mount table entry attributes. | +| `-rm` *source* | Remove mount point of specified path. | +| `-ls` `[-d]` *path* | List mount points under specified path. Specify -d parameter to get detailed listing. | +| `-getDestination` *path* | Get the subcluster where a file is or should be created. | +| `-setQuota` *path* `-nsQuota` *nsQuota* `-ssQuota` *ssQuota* | Set quota for specified path. See [HDFS Quotas Guide](./HdfsQuotaAdminGuide.html) for the quota detail. | +| `-setStorageTypeQuota` *path* `-storageType` *storageType* *stQuota* | Set storage type quota for specified path. See [HDFS Quotas Guide](./HdfsQuotaAdminGuide.html) for the quota detail. | +| `-clrQuota` *path* | Clear quota of given mount point. See [HDFS Quotas Guide](./HdfsQuotaAdminGuide.html) for the quota detail.
[GitHub] [hadoop] ayushtkn commented on a diff in pull request #4990: HDFS-13507. RBF RouterAdmin disable update functionality in add cmd and supports add/update one mount table with different destina
ayushtkn commented on code in PR #4990:
URL: https://github.com/apache/hadoop/pull/4990#discussion_r994232772
##
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/tools/federation/RouterAdmin.java:
##
@@ -147,13 +147,13 @@ private String getUsage(String cmd) {
return usage.toString();
}
if (cmd.equals("-add")) {
- return "\t[-add
"
+ return "\t[-add
"
+ "[-readonly] [-faulttolerant] "
+ "[-order HASH|LOCAL|RANDOM|HASH_ALL|SPACE] "
+ "-owner -group -mode ]";
} else if (cmd.equals("-update")) {
return "\t[-update "
- + " [ ] "
+ + " [ ] "
Review Comment:
Cool, makes sense, will go through that once more. From the description of
the command it appeared that it doesn’t
--
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.
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] ayushtkn commented on a diff in pull request #4990: HDFS-13507. RBF RouterAdmin disable update functionality in add cmd and supports add/update one mount table with different destina
ayushtkn commented on code in PR #4990:
URL: https://github.com/apache/hadoop/pull/4990#discussion_r994223890
##
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/tools/federation/RouterAdmin.java:
##
@@ -147,13 +147,13 @@ private String getUsage(String cmd) {
return usage.toString();
}
if (cmd.equals("-add")) {
- return "\t[-add
"
+ return "\t[-add
"
+ "[-readonly] [-faulttolerant] "
+ "[-order HASH|LOCAL|RANDOM|HASH_ALL|SPACE] "
+ "-owner -group -mode ]";
} else if (cmd.equals("-update")) {
return "\t[-update "
- + " [ ] "
+ + " [ ] "
Review Comment:
I feel for the sake of compatibility we should allow the old command as
well, just remove the update functionality from ADD.
That is if only one destination is provided it should work as before.
We just add a new functionality where destinations can be provided as csv
as well in case targets are different in each NS
--
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.
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
