KeeProMise commented on code in PR #6298:
URL: https://github.com/apache/hadoop/pull/6298#discussion_r1405584545
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterRpcClient.java:
##########
@@ -1119,10 +1119,10 @@ public <R extends RemoteLocationContext, T>
RemoteResult invokeSequential(
// Invoke in priority order
for (final RemoteLocationContext loc : locations) {
String ns = loc.getNameserviceId();
- acquirePermit(ns, ugi, remoteMethod, controller);
Review Comment:
When the router uses FairnessPolicyController, each time a request is
processed,
the permit of the ns corresponding to the request will be obtained first
(method acquirePermit),
and then the information of namenodes corresponding to the ns will be
obtained(method getOrderedNamenodes).
If acquirePermit succeeds but getOrderedNamenodes throws an exception, the
permit will not be released.
Therefore we need to getOrderedNamenodes first and then acquirePermit.
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/fairness/TestRouterHandlersFairness.java:
##########
@@ -100,6 +108,57 @@ public void testFairnessControlOn() throws Exception {
startLoadTest(true);
}
+ /**
+ * Ensure that the semaphore is not acquired,
+ * when invokeSequential or invokeConcurrent throws any exception.
+ */
+ @Test
+ public void testReleasedWhenExceptionOccurs() throws Exception{
+ setupCluster(true, false);
+ RouterContext routerContext = cluster.getRandomRouter();
+ RouterRpcClient rpcClient =
+ routerContext.getRouter().getRpcServer().getRPCClient();
+ // Mock an ActiveNamenodeResolver and inject it into RouterRpcClient,
+ // so RouterRpcClient's getOrderedNamenodes method will report an
exception.
+ ActiveNamenodeResolver mockNamenodeResolver =
mock(ActiveNamenodeResolver.class);
+ Field field = rpcClient.getClass().getDeclaredField("namenodeResolver");
+ field.setAccessible(true);
+ field.set(rpcClient, mockNamenodeResolver);
+
+ // Use getFileInfo test invokeSequential.
+ DFSClient client = routerContext.getClient();
+ int availablePermits =
+
rpcClient.getRouterRpcFairnessPolicyController().getAvailablePermits("ns0");
+ try {
+ LOG.info("Use getFileInfo test invokeSequential.");
+ client.getFileInfo("/test.txt");
+ }catch (IOException ioe) {
+ assertExceptionContains("Cannot locate a registered namenode", ioe);
+ }
+ // Ensure that the semaphore is not acquired.
+ assertEquals(availablePermits,
+
rpcClient.getRouterRpcFairnessPolicyController().getAvailablePermits("ns0"));
+
+ // Use renewLease test invokeConcurrent.
+ Collection<RemoteLocation> locations = new ArrayList<>();
+ locations.add(new RemoteLocation("ns0", "/", "/"));
+ RemoteMethod renewLease = new RemoteMethod(
+ "renewLease",
+ new Class[]{java.lang.String.class, java.util.List.class},
+ new Object[]{null, null});
+ availablePermits =
+
rpcClient.getRouterRpcFairnessPolicyController().getAvailablePermits("ns0");
+ try {
+ LOG.info("Use renewLease test invokeConcurrent.");
+ rpcClient.invokeConcurrent(locations, renewLease);
+ }catch (IOException ioe) {
Review Comment:
done
--
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]
