[
https://issues.apache.org/jira/browse/HDFS-17723?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18072481#comment-18072481
]
ASF GitHub Bot commented on HDFS-17723:
---------------------------------------
KeeProMise commented on code in PR #8396:
URL: https://github.com/apache/hadoop/pull/8396#discussion_r3061745286
##########
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:
> Hi @KeeProMise, thank you for taking a look at this MR. It's indeed not
intentional. To keep the interface simple, I think fail fast on
`AccessControlException` and grouping driver failures and non-existent mounts
into the same `failedPaths` would be the best option. What do you think?
Hi @kokonguyen191, thanks for the quick response! Agree with the approach —
fail fast on AccessControlException keeps the behavior consistent with the
single-entry API, and grouping driver failures with non-existent mounts in
failedPaths is a reasonable simplification. Looking forward to the update!
> 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]