virajjasani commented on a change in pull request #3822:
URL: https://github.com/apache/hadoop/pull/3822#discussion_r773684705
##########
File path:
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestRPC.java
##########
@@ -1107,6 +1108,45 @@ public TestRpcService run() {
}
}
+ @Test
+ public void testNumInProcessHandlerMetrics() throws Exception {
+ UserGroupInformation ugi = UserGroupInformation.
+ createUserForTesting("user123", new String[0]);
+ // use 1 handler so the callq can be plugged
+ final Server server = setupTestServer(conf, 1);
+ try {
+ RpcMetrics rpcMetrics = server.getRpcMetrics();
+ assertEquals(rpcMetrics.getNumInProcessHandler(), 0);
+
+ ExternalCall<String> call1 = newExtCall(ugi,
+ new PrivilegedExceptionAction<String>() {
+ @Override
+ public String run() throws Exception {
+ assertTrue(rpcMetrics.getNumInProcessHandler() > 0);
Review comment:
How about exact comparison? We can use `assertEquals(1,
rpcMetrics.getNumInProcessHandler())`
##########
File path:
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestRPC.java
##########
@@ -1107,6 +1108,45 @@ public TestRpcService run() {
}
}
+ @Test
+ public void testNumInProcessHandlerMetrics() throws Exception {
+ UserGroupInformation ugi = UserGroupInformation.
+ createUserForTesting("user123", new String[0]);
+ // use 1 handler so the callq can be plugged
+ final Server server = setupTestServer(conf, 1);
+ try {
+ RpcMetrics rpcMetrics = server.getRpcMetrics();
+ assertEquals(rpcMetrics.getNumInProcessHandler(), 0);
+
+ ExternalCall<String> call1 = newExtCall(ugi,
+ new PrivilegedExceptionAction<String>() {
Review comment:
nit: use lambda?
##########
File path:
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Server.java
##########
@@ -3057,8 +3062,24 @@ private void internalQueueCall(Call call, boolean
blocking)
}
}
+ /**
+ * Define some states related to Handler.
+ */
+ private enum HandlerStatus {
+ /**Working for a Call.*/
+ IN_PROCESS,
+
+ /**Idle state, waiting for a new call.*/
+ IN_IDLE
+ }
Review comment:
Do we need this enum? We are updating the status to either `IN_PROCESS`
or `IN_IDLE` but not really exposing it anywhere.
##########
File path:
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Server.java
##########
@@ -498,6 +498,7 @@ protected ResponseBuffer initialValue() {
private Map<Integer, Listener> auxiliaryListenerMap;
private Responder responder = null;
private Handler[] handlers = null;
+ final private AtomicInteger numInProcessHandler = new AtomicInteger();
Review comment:
nit: `private final`
##########
File path:
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestRPC.java
##########
@@ -1107,6 +1108,45 @@ public TestRpcService run() {
}
}
+ @Test
+ public void testNumInProcessHandlerMetrics() throws Exception {
+ UserGroupInformation ugi = UserGroupInformation.
+ createUserForTesting("user123", new String[0]);
+ // use 1 handler so the callq can be plugged
+ final Server server = setupTestServer(conf, 1);
+ try {
+ RpcMetrics rpcMetrics = server.getRpcMetrics();
+ assertEquals(rpcMetrics.getNumInProcessHandler(), 0);
+
+ ExternalCall<String> call1 = newExtCall(ugi,
+ new PrivilegedExceptionAction<String>() {
+ @Override
+ public String run() throws Exception {
+ assertTrue(rpcMetrics.getNumInProcessHandler() > 0);
+ return UserGroupInformation.getCurrentUser().getUserName();
+ }
+ });
+ ExternalCall<Void> call2 = newExtCall(ugi,
+ new PrivilegedExceptionAction<Void>() {
Review comment:
same as above, lambda and assertEquals for exact comparison
--
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]
