[GitHub] [hadoop] slfan1989 commented on a diff in pull request #4543: YARN-8900. [Router] Federation: routing getContainers REST invocations transparently to multiple RMs
slfan1989 commented on code in PR #4543:
URL: https://github.com/apache/hadoop/pull/4543#discussion_r1057240710
##
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/main/java/org/apache/hadoop/yarn/server/router/webapp/FederationInterceptorREST.java:
##
@@ -1366,4 +1393,44 @@ public void shutdown() {
threadpool.shutdown();
}
}
+
+ private Map
invokeConcurrent(Collection clusterIds,
+ ClientMethod request, Class clazz) {
+
+Map results = new HashMap<>();
+
+// Send the requests in parallel
+CompletionService compSvc = new
ExecutorCompletionService<>(this.threadpool);
+
+for (final SubClusterInfo info : clusterIds) {
+ compSvc.submit(() -> {
+DefaultRequestInterceptorREST interceptor =
getOrCreateInterceptorForSubCluster(
+info.getSubClusterId(), info.getRMWebServiceAddress());
+try {
+ Method method = DefaultRequestInterceptorREST.class.
+ getMethod(request.getMethodName(), request.getTypes());
+ Object retObj = method.invoke(interceptor, request.getParams());
+ R ret = clazz.cast(retObj);
+ return ret;
+} catch (Exception e) {
+ LOG.error("SubCluster {} failed to call {} method.",
info.getSubClusterId(),
+ request.getMethodName(), e);
+ return null;
+}
+ });
+}
+
+clusterIds.stream().forEach(clusterId -> {
+ try {
+Future future = compSvc.take();
+R response = future.get();
+if (response != null) {
+ results.put(clusterId, response);
Review Comment:
Thank you very much for your feedback, I agree with your description. For
the results of the FederationInterceptorREST interface, we will integrate the
results returned by all subClusters. This part does not care about the return
order of multi-threads. I've submitted a Follow Up pr to fix this, thanks again!
--
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: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] slfan1989 commented on a diff in pull request #4543: YARN-8900. [Router] Federation: routing getContainers REST invocations transparently to multiple RMs
slfan1989 commented on code in PR #4543:
URL: https://github.com/apache/hadoop/pull/4543#discussion_r922908056
##
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/test/java/org/apache/hadoop/yarn/server/router/webapp/MockDefaultRequestInterceptorREST.java:
##
@@ -231,4 +240,42 @@ public boolean isRunning() {
public void setRunning(boolean runningMode) {
this.isRunning = runningMode;
}
+
+ @Override
+ public ContainersInfo getContainers(HttpServletRequest req,
HttpServletResponse res,
+ String appId, String appAttemptId) {
+if (!isRunning) {
+ throw new RuntimeException("RM is stopped");
+}
+
+// We avoid to check if the Application exists in the system because we
need
+// to validate that each subCluster returns 1 container.
+ContainersInfo containers = new ContainersInfo();
+
+int subClusterId = Integer.valueOf(getSubClusterId().getId());
+
+ContainerId containerId = ContainerId.newContainerId(
+ApplicationAttemptId.fromString(appAttemptId), subClusterId);
+Resource allocatedResource =
+Resource.newInstance(subClusterId, subClusterId);
+
+NodeId assignedNode = NodeId.newInstance("Node", subClusterId);
+Priority priority = Priority.newInstance(subClusterId);
+long creationTime = subClusterId;
+long finishTime = subClusterId;
+String diagnosticInfo = "Diagnostic " + subClusterId;
+String logUrl = "Log " + subClusterId;
+int containerExitStatus = subClusterId;
+ContainerState containerState = ContainerState.COMPLETE;
+String nodeHttpAddress = "HttpAddress " + subClusterId;
+
+ContainerReport containerReport = ContainerReport.newInstance(containerId,
allocatedResource,
+assignedNode, priority, creationTime, finishTime, diagnosticInfo,
logUrl,
+containerExitStatus, containerState, nodeHttpAddress);
Review Comment:
as follows
```
ContainerReport containerReport = ContainerReport.newInstance(
containerId, allocatedResource, assignedNode, priority,
creationTime, finishTime, diagnosticInfo, logUrl,
containerExitStatus, containerState, nodeHttpAddress);
```
Is this possible?
--
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: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] slfan1989 commented on a diff in pull request #4543: YARN-8900. [Router] Federation: routing getContainers REST invocations transparently to multiple RMs
slfan1989 commented on code in PR #4543:
URL: https://github.com/apache/hadoop/pull/4543#discussion_r922908056
##
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/test/java/org/apache/hadoop/yarn/server/router/webapp/MockDefaultRequestInterceptorREST.java:
##
@@ -231,4 +240,42 @@ public boolean isRunning() {
public void setRunning(boolean runningMode) {
this.isRunning = runningMode;
}
+
+ @Override
+ public ContainersInfo getContainers(HttpServletRequest req,
HttpServletResponse res,
+ String appId, String appAttemptId) {
+if (!isRunning) {
+ throw new RuntimeException("RM is stopped");
+}
+
+// We avoid to check if the Application exists in the system because we
need
+// to validate that each subCluster returns 1 container.
+ContainersInfo containers = new ContainersInfo();
+
+int subClusterId = Integer.valueOf(getSubClusterId().getId());
+
+ContainerId containerId = ContainerId.newContainerId(
+ApplicationAttemptId.fromString(appAttemptId), subClusterId);
+Resource allocatedResource =
+Resource.newInstance(subClusterId, subClusterId);
+
+NodeId assignedNode = NodeId.newInstance("Node", subClusterId);
+Priority priority = Priority.newInstance(subClusterId);
+long creationTime = subClusterId;
+long finishTime = subClusterId;
+String diagnosticInfo = "Diagnostic " + subClusterId;
+String logUrl = "Log " + subClusterId;
+int containerExitStatus = subClusterId;
+ContainerState containerState = ContainerState.COMPLETE;
+String nodeHttpAddress = "HttpAddress " + subClusterId;
+
+ContainerReport containerReport = ContainerReport.newInstance(containerId,
allocatedResource,
+assignedNode, priority, creationTime, finishTime, diagnosticInfo,
logUrl,
+containerExitStatus, containerState, nodeHttpAddress);
Review Comment:
as follows
```
ContainerReport containerReport = ContainerReport.newInstance(
containerId, allocatedResource, assignedNode, priority,
creationTime, finishTime, diagnosticInfo, logUrl,
containerExitStatus, containerState, nodeHttpAddress);
```
Is this possible?
--
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: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] slfan1989 commented on a diff in pull request #4543: YARN-8900. [Router] Federation: routing getContainers REST invocations transparently to multiple RMs
slfan1989 commented on code in PR #4543:
URL: https://github.com/apache/hadoop/pull/4543#discussion_r922908101
##
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/main/java/org/apache/hadoop/yarn/server/router/webapp/FederationInterceptorREST.java:
##
@@ -1366,4 +1393,42 @@ public void shutdown() {
threadpool.shutdown();
}
}
+
+ private Map
invokeConcurrent(Collection clusterIds,
+ ClientMethod request, Class clazz) {
+
+Map results = new HashMap<>();
+
+// Send the requests in parallel
+CompletionService compSvc = new
ExecutorCompletionService<>(this.threadpool);
+
+for (final SubClusterInfo info : clusterIds) {
+ compSvc.submit(() -> {
+DefaultRequestInterceptorREST interceptor =
getOrCreateInterceptorForSubCluster(
+info.getSubClusterId(), info.getRMWebServiceAddress());
+try {
+ Method method = DefaultRequestInterceptorREST.class.
+ getMethod(request.getMethodName(), request.getTypes());
+ return (R) clazz.cast(method.invoke(interceptor,
request.getParams()));
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: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] slfan1989 commented on a diff in pull request #4543: YARN-8900. [Router] Federation: routing getContainers REST invocations transparently to multiple RMs
slfan1989 commented on code in PR #4543:
URL: https://github.com/apache/hadoop/pull/4543#discussion_r922908056
##
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/test/java/org/apache/hadoop/yarn/server/router/webapp/MockDefaultRequestInterceptorREST.java:
##
@@ -231,4 +240,42 @@ public boolean isRunning() {
public void setRunning(boolean runningMode) {
this.isRunning = runningMode;
}
+
+ @Override
+ public ContainersInfo getContainers(HttpServletRequest req,
HttpServletResponse res,
+ String appId, String appAttemptId) {
+if (!isRunning) {
+ throw new RuntimeException("RM is stopped");
+}
+
+// We avoid to check if the Application exists in the system because we
need
+// to validate that each subCluster returns 1 container.
+ContainersInfo containers = new ContainersInfo();
+
+int subClusterId = Integer.valueOf(getSubClusterId().getId());
+
+ContainerId containerId = ContainerId.newContainerId(
+ApplicationAttemptId.fromString(appAttemptId), subClusterId);
+Resource allocatedResource =
+Resource.newInstance(subClusterId, subClusterId);
+
+NodeId assignedNode = NodeId.newInstance("Node", subClusterId);
+Priority priority = Priority.newInstance(subClusterId);
+long creationTime = subClusterId;
+long finishTime = subClusterId;
+String diagnosticInfo = "Diagnostic " + subClusterId;
+String logUrl = "Log " + subClusterId;
+int containerExitStatus = subClusterId;
+ContainerState containerState = ContainerState.COMPLETE;
+String nodeHttpAddress = "HttpAddress " + subClusterId;
+
+ContainerReport containerReport = ContainerReport.newInstance(containerId,
allocatedResource,
+assignedNode, priority, creationTime, finishTime, diagnosticInfo,
logUrl,
+containerExitStatus, containerState, nodeHttpAddress);
Review Comment:
as follows
```
ContainerReport containerReport = ContainerReport.newInstance(
containerId, allocatedResource, assignedNode, priority,
creationTime, finishTime, diagnosticInfo, logUrl,
containerExitStatus, containerState, nodeHttpAddress);
```
Is this possible?
--
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: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] slfan1989 commented on a diff in pull request #4543: YARN-8900. [Router] Federation: routing getContainers REST invocations transparently to multiple RMs
slfan1989 commented on code in PR #4543:
URL: https://github.com/apache/hadoop/pull/4543#discussion_r922826523
##
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/test/java/org/apache/hadoop/yarn/server/router/webapp/MockDefaultRequestInterceptorREST.java:
##
@@ -231,4 +240,43 @@ public boolean isRunning() {
public void setRunning(boolean runningMode) {
this.isRunning = runningMode;
}
+
+ @Override
+ public ContainersInfo getContainers(HttpServletRequest req,
HttpServletResponse res,
+ String appId, String appAttemptId) {
+if (!isRunning) {
+ throw new RuntimeException("RM is stopped");
+}
+
+// We avoid to check if the Application exists in the system because we
need
+// to validate that each subCluster returns 1 container.
+ContainersInfo containers = new ContainersInfo();
+
+int subClusterId = Integer.valueOf(getSubClusterId().getId());
+
+ContainerId containerId = ContainerId.newContainerId(
+ApplicationAttemptId.fromString(appAttemptId), subClusterId);
+Resource allocatedResource =
+Resource.newInstance(subClusterId, subClusterId);
+
+NodeId assignedNode = NodeId.newInstance("Node", subClusterId);
+Priority priority = Priority.newInstance(subClusterId);
+long creationTime = subClusterId;
+long finishTime = subClusterId;
+String diagnosticInfo = "Diagnostic " + subClusterId;
+String logUrl = "Log " + subClusterId;
+int containerExitStatus = subClusterId;
+ContainerState containerState = ContainerState.COMPLETE;
+String nodeHttpAddress = "HttpAddress " + subClusterId;
+
+ContainerReport containerReport =
Review Comment:
Thanks for your suggestion, I will modify the code.
--
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: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] slfan1989 commented on a diff in pull request #4543: YARN-8900. [Router] Federation: routing getContainers REST invocations transparently to multiple RMs
slfan1989 commented on code in PR #4543:
URL: https://github.com/apache/hadoop/pull/4543#discussion_r922614970
##
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/test/java/org/apache/hadoop/yarn/server/router/webapp/TestFederationInterceptorREST.java:
##
@@ -560,4 +563,49 @@ SubClusterState.SC_RUNNING, new MonotonicClock().getTime(),
SubClusterRegisterRequest.newInstance(subClusterInfo));
}
+ @Test
+ public void testGetContainers()
+throws YarnException, IOException, InterruptedException {
+
+ApplicationId appId = ApplicationId.newInstance(Time.now(), 1);
+ApplicationSubmissionContextInfo context =
+new ApplicationSubmissionContextInfo();
+context.setApplicationId(appId.toString());
+
+// Submit the application we want the report later
+Response response = interceptor.submitApplication(context, null);
+
+Assert.assertNotNull(response);
+Assert.assertNotNull(stateStoreUtil.queryApplicationHomeSC(appId));
+
+ApplicationAttemptId appAttempt =
+ApplicationAttemptId.newInstance(appId, 1);
+
+ContainersInfo responseGet = interceptor.getContainers(null, null,
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: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] slfan1989 commented on a diff in pull request #4543: YARN-8900. [Router] Federation: routing getContainers REST invocations transparently to multiple RMs
slfan1989 commented on code in PR #4543:
URL: https://github.com/apache/hadoop/pull/4543#discussion_r922600402
##
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/test/java/org/apache/hadoop/yarn/server/router/webapp/TestFederationInterceptorREST.java:
##
@@ -560,4 +563,49 @@ SubClusterState.SC_RUNNING, new MonotonicClock().getTime(),
SubClusterRegisterRequest.newInstance(subClusterInfo));
}
+ @Test
+ public void testGetContainers()
+throws YarnException, IOException, InterruptedException {
+
+ApplicationId appId = ApplicationId.newInstance(Time.now(), 1);
+ApplicationSubmissionContextInfo context =
Review Comment:
I will fix it.
##
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/test/java/org/apache/hadoop/yarn/server/router/webapp/TestFederationInterceptorREST.java:
##
@@ -560,4 +563,49 @@ SubClusterState.SC_RUNNING, new MonotonicClock().getTime(),
SubClusterRegisterRequest.newInstance(subClusterInfo));
}
+ @Test
+ public void testGetContainers()
+throws YarnException, IOException, InterruptedException {
+
+ApplicationId appId = ApplicationId.newInstance(Time.now(), 1);
+ApplicationSubmissionContextInfo context =
+new ApplicationSubmissionContextInfo();
+context.setApplicationId(appId.toString());
+
+// Submit the application we want the report later
+Response response = interceptor.submitApplication(context, null);
+
+Assert.assertNotNull(response);
+Assert.assertNotNull(stateStoreUtil.queryApplicationHomeSC(appId));
+
+ApplicationAttemptId appAttempt =
+ApplicationAttemptId.newInstance(appId, 1);
+
+ContainersInfo responseGet = interceptor.getContainers(null, null,
+appId.toString(), appAttempt.toString());
+
+Assert.assertEquals(4, responseGet.getContainers().size());
+ }
+
+ @Test
+ public void testGetContainersNotExists() {
+ApplicationId appId = ApplicationId.newInstance(Time.now(), 1);
+ContainersInfo response =
Review Comment:
I will fix it.
##
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/test/java/org/apache/hadoop/yarn/server/router/webapp/TestFederationInterceptorREST.java:
##
@@ -560,4 +563,49 @@ SubClusterState.SC_RUNNING, new MonotonicClock().getTime(),
SubClusterRegisterRequest.newInstance(subClusterInfo));
}
+ @Test
+ public void testGetContainers()
+throws YarnException, IOException, InterruptedException {
+
+ApplicationId appId = ApplicationId.newInstance(Time.now(), 1);
+ApplicationSubmissionContextInfo context =
+new ApplicationSubmissionContextInfo();
+context.setApplicationId(appId.toString());
+
+// Submit the application we want the report later
+Response response = interceptor.submitApplication(context, null);
+
+Assert.assertNotNull(response);
+Assert.assertNotNull(stateStoreUtil.queryApplicationHomeSC(appId));
+
+ApplicationAttemptId appAttempt =
+ApplicationAttemptId.newInstance(appId, 1);
+
+ContainersInfo responseGet = interceptor.getContainers(null, null,
+appId.toString(), appAttempt.toString());
+
+Assert.assertEquals(4, responseGet.getContainers().size());
+ }
+
+ @Test
+ public void testGetContainersNotExists() {
+ApplicationId appId = ApplicationId.newInstance(Time.now(), 1);
+ContainersInfo response =
+interceptor.getContainers(null, null, appId.toString(), null);
+Assert.assertTrue(response.getContainers().isEmpty());
+ }
+
+ @Test
+ public void testGetContainersWrongFormat() {
+ContainersInfo response =
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: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] slfan1989 commented on a diff in pull request #4543: YARN-8900. [Router] Federation: routing getContainers REST invocations transparently to multiple RMs
slfan1989 commented on code in PR #4543:
URL: https://github.com/apache/hadoop/pull/4543#discussion_r922600391
##
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/main/java/org/apache/hadoop/yarn/server/router/webapp/FederationInterceptorREST.java:
##
@@ -1366,4 +1386,49 @@ public void shutdown() {
threadpool.shutdown();
}
}
+
+ Map invokeConcurrent(Collection
clusterIds,
+ ClientMethod request, Class clazz) throws YarnException, IOException
{
+ArrayList clusterIdList = new ArrayList<>(clusterIds);
+return invokeConcurrent(clusterIdList, request, clazz);
+ }
+
+ private Map
invokeConcurrent(ArrayList subClusterInfo,
+ ClientMethod request, Class clazz) {
+
+Map results = new HashMap<>();
+
+// Send the requests in parallel
+CompletionService compSvc = new
ExecutorCompletionService<>(this.threadpool);
+
+for (final SubClusterInfo info : subClusterInfo) {
+ compSvc.submit(() -> {
+DefaultRequestInterceptorREST interceptor =
getOrCreateInterceptorForSubCluster(
+info.getSubClusterId(), info.getRMWebServiceAddress());
+try {
+ Method method = DefaultRequestInterceptorREST.class.
+ getMethod(request.getMethodName(), request.getTypes());
+ return clazz.cast(method.invoke(interceptor, request.getParams()));
Review Comment:
I added cast to R.
--
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: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] slfan1989 commented on a diff in pull request #4543: YARN-8900. [Router] Federation: routing getContainers REST invocations transparently to multiple RMs
slfan1989 commented on code in PR #4543:
URL: https://github.com/apache/hadoop/pull/4543#discussion_r922600169
##
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/main/java/org/apache/hadoop/yarn/server/router/webapp/FederationInterceptorREST.java:
##
@@ -1366,4 +1386,49 @@ public void shutdown() {
threadpool.shutdown();
}
}
+
+ Map invokeConcurrent(Collection
clusterIds,
+ ClientMethod request, Class clazz) throws YarnException, IOException
{
+ArrayList clusterIdList = new ArrayList<>(clusterIds);
+return invokeConcurrent(clusterIdList, request, clazz);
+ }
+
+ private Map
invokeConcurrent(ArrayList subClusterInfo,
+ ClientMethod request, Class clazz) {
+
+Map results = new HashMap<>();
+
+// Send the requests in parallel
+CompletionService compSvc = new
ExecutorCompletionService<>(this.threadpool);
+
+for (final SubClusterInfo info : subClusterInfo) {
Review Comment:
Thanks a lot for the advice, let me learn a lot, the code has been modified.
--
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: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] slfan1989 commented on a diff in pull request #4543: YARN-8900. [Router] Federation: routing getContainers REST invocations transparently to multiple RMs
slfan1989 commented on code in PR #4543:
URL: https://github.com/apache/hadoop/pull/4543#discussion_r922596234
##
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/main/java/org/apache/hadoop/yarn/server/router/webapp/FederationInterceptorREST.java:
##
@@ -1336,7 +1331,32 @@ public AppAttemptInfo getAppAttempt(HttpServletRequest
req,
@Override
public ContainersInfo getContainers(HttpServletRequest req,
HttpServletResponse res, String appId, String appAttemptId) {
-throw new NotImplementedException("Code is not implemented");
+
+ContainersInfo containersInfo = new ContainersInfo();
+
+Map subClustersActive;
+try {
+ subClustersActive = getActiveSubclusters();
+} catch (NotFoundException e) {
+ LOG.error("Get all active sub cluster(s) error.", e);
+ return containersInfo;
+}
+
+try {
+ ClientMethod remoteMethod = new ClientMethod("getContainers",
Review Comment:
your suggestion is great!
--
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: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] slfan1989 commented on a diff in pull request #4543: YARN-8900. [Router] Federation: routing getContainers REST invocations transparently to multiple RMs
slfan1989 commented on code in PR #4543:
URL: https://github.com/apache/hadoop/pull/4543#discussion_r922595838
##
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/webapp/dao/ContainersInfo.java:
##
@@ -46,4 +47,7 @@ public ArrayList getContainers() {
return container;
}
+ public void addAll(Collection containersInfo) {
+container.addAll(new ArrayList<>(containersInfo));
Review Comment:
Thanks for the suggestion, I will modify the code.
--
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: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] slfan1989 commented on a diff in pull request #4543: YARN-8900. [Router] Federation: routing getContainers REST invocations transparently to multiple RMs
slfan1989 commented on code in PR #4543:
URL: https://github.com/apache/hadoop/pull/4543#discussion_r921598290
##
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/main/java/org/apache/hadoop/yarn/server/router/webapp/FederationInterceptorREST.java:
##
@@ -1336,7 +1336,51 @@ public AppAttemptInfo getAppAttempt(HttpServletRequest
req,
@Override
public ContainersInfo getContainers(HttpServletRequest req,
HttpServletResponse res, String appId, String appAttemptId) {
-throw new NotImplementedException("Code is not implemented");
+ContainersInfo containersInfo = new ContainersInfo();
+
+Map subClustersActive = null;
+try {
+ subClustersActive = federationFacade.getSubClusters(true);
+} catch (YarnException e) {
+ LOG.error("Get All active sub cluster(s) error.", e);
+ return containersInfo;
+}
+
+// Send the requests in parallel
+CompletionService compSvc =
+new ExecutorCompletionService<>(this.threadpool);
+
+for (final SubClusterInfo info : subClustersActive.values()) {
+ compSvc.submit(() -> {
+DefaultRequestInterceptorREST interceptor =
+getOrCreateInterceptorForSubCluster(info.getSubClusterId(),
info.getRMWebServiceAddress());
+try {
+ ContainersInfo containers =
+ interceptor.getContainers(req, res, appId, appAttemptId);
+ return containers;
+} catch (Exception e) {
+ LOG.error("SubCluster {} failed to return GetContainers.",
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: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] slfan1989 commented on a diff in pull request #4543: YARN-8900. [Router] Federation: routing getContainers REST invocations transparently to multiple RMs
slfan1989 commented on code in PR #4543:
URL: https://github.com/apache/hadoop/pull/4543#discussion_r921597854
##
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/webapp/dao/ContainersInfo.java:
##
@@ -46,4 +46,7 @@ public ArrayList getContainers() {
return container;
}
+ public void addAll(ArrayList containersInfo) {
Review Comment:
Thanks for helping to review the code, I will modify the code.
--
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: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] slfan1989 commented on a diff in pull request #4543: YARN-8900. [Router] Federation: routing getContainers REST invocations transparently to multiple RMs
slfan1989 commented on code in PR #4543:
URL: https://github.com/apache/hadoop/pull/4543#discussion_r921598076
##
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/main/java/org/apache/hadoop/yarn/server/router/webapp/FederationInterceptorREST.java:
##
@@ -1336,7 +1336,51 @@ public AppAttemptInfo getAppAttempt(HttpServletRequest
req,
@Override
public ContainersInfo getContainers(HttpServletRequest req,
HttpServletResponse res, String appId, String appAttemptId) {
-throw new NotImplementedException("Code is not implemented");
+ContainersInfo containersInfo = new ContainersInfo();
+
+Map subClustersActive = null;
+try {
+ subClustersActive = federationFacade.getSubClusters(true);
+} catch (YarnException e) {
+ LOG.error("Get All active sub cluster(s) error.", e);
+ return containersInfo;
+}
+
+// Send the requests in parallel
+CompletionService compSvc =
+new ExecutorCompletionService<>(this.threadpool);
+
+for (final SubClusterInfo info : subClustersActive.values()) {
+ compSvc.submit(() -> {
+DefaultRequestInterceptorREST interceptor =
+getOrCreateInterceptorForSubCluster(info.getSubClusterId(),
info.getRMWebServiceAddress());
+try {
+ ContainersInfo containers =
+ interceptor.getContainers(req, res, appId, appAttemptId);
+ return containers;
+} catch (Exception e) {
+ LOG.error("SubCluster {} failed to return GetContainers.",
+ info.getSubClusterId());
+ return null;
+}
+ });
+}
+
+// Collect all the responses in parallel
+for (int i = 0; i < subClustersActive.size(); i++) {
+ try {
+Future future = compSvc.take();
+ContainersInfo containersResponse = future.get();
+
+if (containersResponse != null) {
+ containersInfo.addAll(containersResponse.getContainers());
+}
+ } catch (Throwable e) {
+LOG.warn("Failed to get containers report. ", e);
Review Comment:
OK, 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: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] slfan1989 commented on a diff in pull request #4543: YARN-8900. [Router] Federation: routing getContainers REST invocations transparently to multiple RMs
slfan1989 commented on code in PR #4543:
URL: https://github.com/apache/hadoop/pull/4543#discussion_r920748434
##
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/main/java/org/apache/hadoop/yarn/server/router/webapp/FederationInterceptorREST.java:
##
@@ -1336,7 +1336,51 @@ public AppAttemptInfo getAppAttempt(HttpServletRequest
req,
@Override
public ContainersInfo getContainers(HttpServletRequest req,
HttpServletResponse res, String appId, String appAttemptId) {
-throw new NotImplementedException("Code is not implemented");
+ContainersInfo containersInfo = new ContainersInfo();
+
+Map subClustersActive = null;
+try {
+ subClustersActive = federationFacade.getSubClusters(true);
+} catch (YarnException e) {
+ LOG.error(e.getLocalizedMessage());
Review Comment:
I will refactor this code to make it easier for other method calls.
--
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: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] slfan1989 commented on a diff in pull request #4543: YARN-8900. [Router] Federation: routing getContainers REST invocations transparently to multiple RMs
slfan1989 commented on code in PR #4543:
URL: https://github.com/apache/hadoop/pull/4543#discussion_r918436233
##
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/main/java/org/apache/hadoop/yarn/server/router/webapp/FederationInterceptorREST.java:
##
@@ -1336,7 +1336,51 @@ public AppAttemptInfo getAppAttempt(HttpServletRequest
req,
@Override
public ContainersInfo getContainers(HttpServletRequest req,
HttpServletResponse res, String appId, String appAttemptId) {
-throw new NotImplementedException("Code is not implemented");
+ContainersInfo containersInfo = new ContainersInfo();
+
+Map subClustersActive = null;
+try {
+ subClustersActive = federationFacade.getSubClusters(true);
+} catch (YarnException e) {
+ LOG.error(e.getLocalizedMessage());
Review Comment:
Thanks for your help reviewing the code, I will fix it asap.
--
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: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
