ayushtkn commented on code in PR #5200:
URL: https://github.com/apache/hadoop/pull/5200#discussion_r1062932845


##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterClientProtocol.java:
##########
@@ -465,6 +465,26 @@ public void setOwner(String src, String username, String 
groupname)
     }
   }
 
+  /**
+   * Try to get the remote location whose bpId is same with the input bpId 
from the input locations.
+   * @param locations the input RemoteLocations.
+   * @param bpId the input bpId.
+   * @return the remote location whose bpId is same with the input.
+   * @throws IOException
+   */
+  private RemoteLocation getLocationWithBPID(List<RemoteLocation> locations, 
String bpId)
+      throws IOException {
+    String nsId = rpcClient.getNameserviceForBlockPoolId(bpId);
+    for (RemoteLocation l : locations) {
+      if (l.getNameserviceId().equals(nsId)) {
+        return l;
+      }
+    }
+
+    LOG.debug("Can't found remote location for the {} from {}", bpId, 
locations);

Review Comment:
   nit:
   I think it will be  `find` instead of `found`
   checked here as well
   https://hinative.com/questions/16308167



##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterClientProtocol.java:
##########
@@ -545,12 +567,12 @@ public boolean complete(String src, String clientName, 
ExtendedBlock last,
             long.class},
         new RemoteParam(), clientName, last, fileId);
 
+    final List<RemoteLocation> locations = rpcServer.getLocationsForPath(src, 
true);
     if (last != null) {
-      return rpcClient.invokeSingle(last, method, Boolean.class);
+      RemoteLocation location = getLocationWithBPID(locations, 
last.getBlockPoolId());
+      return rpcClient.invokeSingle(location, method, Boolean.class);

Review Comment:
   We are doing this `` rpcServer.getLocationsForPath(src, true);`` and then 
getting the location for I think every old invokeSingle with Block as an 
argument.
   Can we not do this logic inside the removed ``invokeSingle`` method?



-- 
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

Reply via email to