slfan1989 commented on code in PR #5636:
URL: https://github.com/apache/hadoop/pull/5636#discussion_r1205701835
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/test/java/org/apache/hadoop/yarn/server/router/webapp/TestRouterWebServiceUtil.java:
##########
@@ -678,4 +683,83 @@ public void testMergeDiffApplicationStatisticsInfo() {
Assert.assertEquals(YarnApplicationState.FINISHED, item3Result.getState());
Assert.assertEquals(item4.getCount(), item3Result.getCount());
}
+
+ @Test
+ public void testCreateJerseyClient() {
+ // Case1, default timeout, The default timeout is 30s.
+ YarnConfiguration configuration = new YarnConfiguration();
+ Client client01 = RouterWebServiceUtil.createJerseyClient(configuration);
+ Map<String, Object> properties = client01.getProperties();
+ int readTimeOut = (int) properties.get(ClientConfig.PROPERTY_READ_TIMEOUT);
+ int connectTimeOut = (int)
properties.get(ClientConfig.PROPERTY_CONNECT_TIMEOUT);
+ Assert.assertEquals(30000, readTimeOut);
+ Assert.assertEquals(30000, connectTimeOut);
+ client01.destroy();
+
+ // Case2, set a negative timeout, We'll get the default timeout(30s)
+ YarnConfiguration configuration2 = new YarnConfiguration();
+ configuration2.setLong(YarnConfiguration.ROUTER_WEBAPP_CONNECT_TIMEOUT,
-1L);
+ configuration2.setLong(YarnConfiguration.ROUTER_WEBAPP_READ_TIMEOUT, -1L);
+ Client client02 = RouterWebServiceUtil.createJerseyClient(configuration2);
+ Map<String, Object> properties02 = client02.getProperties();
+ int readTimeOut02 = (int)
properties02.get(ClientConfig.PROPERTY_READ_TIMEOUT);
+ int connectTimeOut02 = (int)
properties02.get(ClientConfig.PROPERTY_CONNECT_TIMEOUT);
+ Assert.assertEquals(30000, readTimeOut02);
+ Assert.assertEquals(30000, connectTimeOut02);
+ client02.destroy();
+
+ // Case3, Set the maximum value that exceeds the integer
+ // We'll get the default timeout(30s)
+ YarnConfiguration configuration3 = new YarnConfiguration();
+ long connectTimeOutLong = (long) Integer.MAX_VALUE + 1;
+ long readTimeOutLong = (long) Integer.MAX_VALUE + 1;
Review Comment:
Thank you for helping to review the code! I agree with your point that if
users set a very large value for the timeout, it could indeed lead to
unexpected timeouts. Generally, this timeout value should not exceed 1 minute.
If the user sets a value much larger than this, I will print a warning log to
alert them.
Could this approach work?
--
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]
