dParikesit commented on code in PR #8345:
URL: https://github.com/apache/hadoop/pull/8345#discussion_r2960672359


##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterRpcClient.java:
##########
@@ -1599,51 +1599,55 @@ protected static <T extends RemoteLocationContext, R> 
Map<T, R> postProcessResul
     // transfer originCall & callerContext to worker threads of executor.
     final Call originCall = Server.getCurCall().get();
     final CallerContext originContext = CallerContext.getCurrent();
-    for (final T location : locations) {
-      String nsId = location.getNameserviceId();
-      boolean isObserverRead = isObserverReadEligible(nsId, m);
-      final List<? extends FederationNamenodeContext> namenodes =
-          getOrderedNamenodes(nsId, isObserverRead);
-      final Class<?> proto = method.getProtocol();
-      final Object[] paramList = method.getParams(location);
-      if (standby) {
-        // Call the objectGetter to all NNs (including standby)
-        for (final FederationNamenodeContext nn : namenodes) {
-          String nnId = nn.getNamenodeId();
-          final List<FederationNamenodeContext> nnList =
-              Collections.singletonList(nn);
-          T nnLocation = location;
-          if (location instanceof RemoteLocation) {
-            nnLocation = (T)new RemoteLocation(nsId, nnId, location.getDest());
+    try{
+      for (final T location : locations) {
+        String nsId = location.getNameserviceId();
+        boolean isObserverRead = isObserverReadEligible(nsId, m);
+        final List<? extends FederationNamenodeContext> namenodes =
+            getOrderedNamenodes(nsId, isObserverRead);
+        final Class<?> proto = method.getProtocol();
+        final Object[] paramList = method.getParams(location);
+        if (standby) {
+          // Call the objectGetter to all NNs (including standby)
+          for (final FederationNamenodeContext nn : namenodes) {
+            String nnId = nn.getNamenodeId();
+            final List<FederationNamenodeContext> nnList =
+                Collections.singletonList(nn);
+            T nnLocation = location;
+            if (location instanceof RemoteLocation) {
+              nnLocation = (T)new RemoteLocation(nsId, nnId, 
location.getDest());
+            }
+            orderedLocations.add(nnLocation);
+            callables.add(
+                () -> {
+                  transferThreadLocalContext(originCall, originContext);
+                  return invokeMethod(
+                      ugi, nnList, isObserverRead, proto, m, paramList);
+                });
           }
-          orderedLocations.add(nnLocation);
+        } else {
+          // Call the objectGetter in order of nameservices in the NS list
+          orderedLocations.add(location);
           callables.add(
               () -> {
                 transferThreadLocalContext(originCall, originContext);
                 return invokeMethod(
-                    ugi, nnList, isObserverRead, proto, m, paramList);
+                    ugi, namenodes, isObserverRead, proto, m, paramList);
               });
         }
-      } else {
-        // Call the objectGetter in order of nameservices in the NS list
-        orderedLocations.add(location);
-        callables.add(
-            () -> {
-              transferThreadLocalContext(originCall, originContext);
-              return invokeMethod(
-                  ugi, namenodes, isObserverRead, proto, m, paramList);
-            });
       }
-    }
 
-    if (rpcMonitor != null) {
-      rpcMonitor.proxyOp();
-    }
-    if (this.router.getRouterClientMetrics() != null) {
-      this.router.getRouterClientMetrics().incInvokedConcurrent(m);
-    }
+      if (rpcMonitor != null) {
+        rpcMonitor.proxyOp();
+      }
+      if (this.router.getRouterClientMetrics() != null) {
+        this.router.getRouterClientMetrics().incInvokedConcurrent(m);
+      }
 
-    return getRemoteResults(method, timeOutMs, controller, orderedLocations, 
callables);
+      return getRemoteResults(method, timeOutMs, controller, orderedLocations, 
callables);

Review Comment:
   Thanks for the feedback! I have moved getRemoteResults outside the try-block 
and change finally to catch. Please let me know if you have any other comments.



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

Reply via email to