[
https://issues.apache.org/jira/browse/HDFS-17723?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18073692#comment-18073692
]
ASF GitHub Bot commented on HDFS-17723:
---------------------------------------
Hexiaoqiao commented on code in PR #8396:
URL: https://github.com/apache/hadoop/pull/8396#discussion_r3085273147
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/protocolPB/RouterAdminProtocolTranslatorPB.java:
##########
@@ -207,6 +213,22 @@ public RemoveMountTableEntryResponse removeMountTableEntry(
}
}
+ @Override
+ public RemoveMountTableEntriesResponse removeMountTableEntries(
+ RemoveMountTableEntriesRequest request) throws IOException {
+ RemoveMountTableEntriesRequestPBImpl requestPB =
+ (RemoveMountTableEntriesRequestPBImpl)request;
+ RemoveMountTableEntriesRequestProto proto = requestPB.getProto();
+ try {
+ RemoveMountTableEntriesResponseProto responseProto =
+ rpcProxy.removeMountTableEntries(null, proto);
+ return new RemoveMountTableEntriesResponsePBImpl(responseProto);
+ } catch (ServiceException e) {
+
Review Comment:
codestyle: remove the blank line.
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/tools/federation/RouterAdmin.java:
##########
@@ -862,24 +859,44 @@ private MountTable getMountEntry(String mount,
MountTableManager mountTable)
}
/**
- * Remove mount point.
+ * Remove one or multiple mount points.
*
- * @param path Path to remove.
- * @return If the mount point was removed successfully.
+ * @param params parameters, should contain paths to remove
+ * @param paramIdx starting param index
* @throws IOException If it cannot be removed.
*/
- public boolean removeMount(String path) throws IOException {
- path = normalizeFileSystemPath(path);
+ public void removeMounts(String[] params, int paramIdx) throws IOException {
+ List<String> pathsToRemove = new ArrayList<>();
+ while (paramIdx < params.length) {
+ pathsToRemove.add(normalizeFileSystemPath(params[paramIdx]));
+ paramIdx++;
+ }
MountTableManager mountTable = client.getMountTableManager();
- RemoveMountTableEntryRequest request =
- RemoveMountTableEntryRequest.newInstance(path);
- RemoveMountTableEntryResponse response =
- mountTable.removeMountTableEntry(request);
- boolean removed = response.getStatus();
- if (!removed) {
- System.out.println("Cannot remove mount point " + path);
+ if (pathsToRemove.isEmpty()) {
+ return;
+ }
+ if (pathsToRemove.size() == 1) {
+ String path = pathsToRemove.get(0);
+ RemoveMountTableEntryRequest request =
RemoveMountTableEntryRequest.newInstance(path);
+ RemoveMountTableEntryResponse response =
mountTable.removeMountTableEntry(request);
+ boolean removed = response.getStatus();
+ if (!removed) {
+ System.out.println("Cannot remove mount point " + path);
Review Comment:
System.err.println
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/tools/federation/RouterAdmin.java:
##########
@@ -862,24 +859,44 @@ private MountTable getMountEntry(String mount,
MountTableManager mountTable)
}
/**
- * Remove mount point.
+ * Remove one or multiple mount points.
*
- * @param path Path to remove.
- * @return If the mount point was removed successfully.
+ * @param params parameters, should contain paths to remove
+ * @param paramIdx starting param index
* @throws IOException If it cannot be removed.
*/
- public boolean removeMount(String path) throws IOException {
- path = normalizeFileSystemPath(path);
+ public void removeMounts(String[] params, int paramIdx) throws IOException {
+ List<String> pathsToRemove = new ArrayList<>();
+ while (paramIdx < params.length) {
+ pathsToRemove.add(normalizeFileSystemPath(params[paramIdx]));
+ paramIdx++;
+ }
MountTableManager mountTable = client.getMountTableManager();
- RemoveMountTableEntryRequest request =
- RemoveMountTableEntryRequest.newInstance(path);
- RemoveMountTableEntryResponse response =
- mountTable.removeMountTableEntry(request);
- boolean removed = response.getStatus();
- if (!removed) {
- System.out.println("Cannot remove mount point " + path);
+ if (pathsToRemove.isEmpty()) {
+ return;
+ }
+ if (pathsToRemove.size() == 1) {
+ String path = pathsToRemove.get(0);
+ RemoveMountTableEntryRequest request =
RemoveMountTableEntryRequest.newInstance(path);
+ RemoveMountTableEntryResponse response =
mountTable.removeMountTableEntry(request);
+ boolean removed = response.getStatus();
+ if (!removed) {
+ System.out.println("Cannot remove mount point " + path);
+ } else {
+ System.out.println("Successfully removed mount point " + path);
+ }
+ return;
+ }
+ RemoveMountTableEntriesRequest request =
+ RemoveMountTableEntriesRequest.newInstance(pathsToRemove);
+ RemoveMountTableEntriesResponse response =
mountTable.removeMountTableEntries(request);
+ for (String path : pathsToRemove) {
+ if (response.getFailedRecordsKeys().contains(path)) {
+ System.out.println("Cannot remove mount point " + path);
Review Comment:
System.err.println
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/store/impl/MountTableStoreImpl.java:
##########
@@ -209,6 +214,56 @@ public RemoveMountTableEntryResponse removeMountTableEntry(
return response;
}
+ @Override
+ public RemoveMountTableEntriesResponse removeMountTableEntries(
+ RemoveMountTableEntriesRequest request) throws IOException {
+ List<String> failedPaths = new ArrayList<>();
+ List<MountTable> entriesToRemove = new ArrayList<>();
+ List<MountTable> allEntries =
getDriver().get(getRecordClass()).getRecords();
+ for (String path : request.getSrcPaths()) {
+ final MountTable partial = MountTable.newInstance();
+ partial.setSourcePath(path);
+ final Query<MountTable> query = new Query<>(partial);
+ List<MountTable> filtered = filterMultiple(query, allEntries);
+ MountTable deleteEntry = null;
+ if (filtered.size() == 1) {
+ deleteEntry = filtered.get(0);
+ }
+
+ if (deleteEntry != null) {
+ RouterPermissionChecker pc = RouterAdminServer.getPermissionChecker();
+ if (pc != null) {
+ try {
+ pc.checkPermission(deleteEntry, FsAction.WRITE);
+ entriesToRemove.add(deleteEntry);
+ } catch (IOException ioe) {
+ failedPaths.add(path);
+ }
+ }
+ } else {
+ failedPaths.add(path);
+ }
+ }
+
+ boolean anyRemoved = false;
+ Map<MountTable, Boolean> statuses =
getDriver().removeMultiple(entriesToRemove);
+ for (Map.Entry<MountTable, Boolean> mapEntry : statuses.entrySet()) {
+ if (!mapEntry.getValue()) {
+ failedPaths.add(mapEntry.getKey().getSourcePath());
+ } else {
+ anyRemoved = true;
+ }
+ }
Review Comment:
Totally agree to add the distinguishing reason and throw it to end uesers.
> RBF: Optimize multiple mount point removal
> ------------------------------------------
>
> Key: HDFS-17723
> URL: https://issues.apache.org/jira/browse/HDFS-17723
> Project: Hadoop HDFS
> Issue Type: Improvement
> Reporter: Felix N
> Assignee: Felix N
> Priority: Minor
> Labels: pull-request-available
>
> Since each remove operation forces an update on all routers, removing a lot
> of mount points in one go when there are a decent number of routers can take
> a long time. It's better to just remove all of them in a batch and force
> update only once. Useful for nuking old unused mount points.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
