slfan1989 commented on code in PR #4904:
URL: https://github.com/apache/hadoop/pull/4904#discussion_r976348794
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/test/java/org/apache/hadoop/yarn/server/router/clientrm/TestFederationClientInterceptor.java:
##########
@@ -582,9 +583,9 @@ public void testGetClusterMetricsRequest() throws Exception
{
ClientMethod remoteMethod = new ClientMethod("getClusterMetrics",
new Class[] {GetClusterMetricsRequest.class},
new Object[] {GetClusterMetricsRequest.newInstance()});
- Map<SubClusterId, GetClusterMetricsResponse> clusterMetrics = interceptor.
- invokeConcurrent(new ArrayList<>(), remoteMethod,
GetClusterMetricsResponse.class);
- Assert.assertTrue(clusterMetrics.isEmpty());
+ Collection<GetClusterMetricsResponse> clusterMetrics = interceptor.
+ invokeConcurrent(remoteMethod, GetClusterMetricsResponse.class);
+ Assert.assertTrue(!clusterMetrics.isEmpty());
Review Comment:
`invokeConcurrent` This function originally contains 3 parameters
1. ArrayList<SubClusterId> clusterIds
2. ClientMethod request
3. Class<R> clazz
The first parameter is the SubClusters currently active.
In order to get the first parameter, all calling functions need to call the
following function
`Map<SubClusterId, SubClusterInfo> subclusters =
federationFacade.getSubClusters(true);`
I think we can put the fetching of the 1st parameter inside
`invokeConcurrent`, which is more reasonable.
This unit test tests a situation where there is no Subcluster available, so
an empty result is returned.
```
Map<SubClusterId, GetClusterMetricsResponse> clusterMetrics = interceptor.
invokeConcurrent(new ArrayList<>(), remoteMethod,
GetClusterMetricsResponse.class);
```
I removed the 1st parameter, it is a little difficult to simulate the
situation where all Subclusters are unavailable.
I try to ensure that the original test logic can be completed, and I will
refactor part of the code.
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/test/java/org/apache/hadoop/yarn/server/router/clientrm/TestFederationClientInterceptor.java:
##########
@@ -582,9 +583,9 @@ public void testGetClusterMetricsRequest() throws Exception
{
ClientMethod remoteMethod = new ClientMethod("getClusterMetrics",
new Class[] {GetClusterMetricsRequest.class},
new Object[] {GetClusterMetricsRequest.newInstance()});
- Map<SubClusterId, GetClusterMetricsResponse> clusterMetrics = interceptor.
- invokeConcurrent(new ArrayList<>(), remoteMethod,
GetClusterMetricsResponse.class);
- Assert.assertTrue(clusterMetrics.isEmpty());
+ Collection<GetClusterMetricsResponse> clusterMetrics = interceptor.
+ invokeConcurrent(remoteMethod, GetClusterMetricsResponse.class);
+ Assert.assertTrue(!clusterMetrics.isEmpty());
Review Comment:
I will fix it.
--
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]
