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.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
