goiri commented on code in PR #6298:
URL: https://github.com/apache/hadoop/pull/6298#discussion_r1405472006
##########
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");
Review Comment:
Do you always expect the exception?
In that case, we should use LambdaTestUtils#intercept
##########
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:
Spaces
##########
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) {
Review Comment:
Space
--
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]
