Skip to content

YARN-11228. [Federation] Add getAppAttempts, getAppAttempt REST APIs for Router. #4695

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
goiri merged 7 commits into from
Aug 6, 2022
Merged

YARN-11228. [Federation] Add getAppAttempts, getAppAttempt REST APIs for Router. #4695

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
01c679e
YARN-11228. [Federation] Add getAppAttempts, getAppAttempt REST APIs …
Aug 4, 2022
a4052f6
YARN-11228. Fix Some CodeStyle.
Aug 4, 2022
854758d
YARN-11228. Fix Some CodeStyle.
Aug 4, 2022
4eb1a5e
YARN-11228. Fix Junit Test.
Aug 4, 2022
7005b71
YARN-11228. Fix Junit Test & CheckStyle.
Aug 4, 2022
b4ad85e
YARN-11228. Fix CheckStyle.
Aug 4, 2022
a856cfe
YARN-11228. Fix CheckStyle.
Aug 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 59 additions & 7 deletions .../src/main/java/org/apache/hadoop/yarn/server/router/webapp/FederationInterceptorREST.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1365,7 +1365,30 @@ public Response updateApplicationTimeout(AppTimeoutInfo appTimeout,

@Override
public AppAttemptsInfo getAppAttempts(HttpServletRequest hsr, String appId) {
throw new NotImplementedException("Code is not implemented");
if (appId == null || appId.isEmpty()) {
throw new IllegalArgumentException("Parameter error, the appId is empty or null.");
}

try {
ApplicationId applicationId = ApplicationId.fromString(appId);
SubClusterInfo subClusterInfo = getHomeSubClusterInfoByAppId(applicationId);
Copy link
Member

@goiri goiri Aug 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should've proposed this before but we keep doing this string to ApplicationId.
We could just have this getHomeSubClusterInfoByAppId() to take a string and do the transformation there.

Copy link
Contributor Author

@slfan1989 slfan1989 Aug 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your suggestion, I will modify the code.


if (subClusterInfo == null) {
RouterServerUtil.logAndThrowRunTimeException("Unable to get subCluster by applicationId = "
+ applicationId, null);
}

DefaultRequestInterceptorREST interceptor = getOrCreateInterceptorForSubCluster(
subClusterInfo.getSubClusterId(), subClusterInfo.getRMWebServiceAddress());
return interceptor.getAppAttempts(hsr, appId);
} catch (IllegalArgumentException e) {
String msg = String.format("Unable to get the AppAttempt appId: %s.", appId);
RouterServerUtil.logAndThrowRunTimeException(msg, e);
} catch (YarnException e) {
RouterServerUtil.logAndThrowRunTimeException("getContainer Failed.", e);
}

return null;
}

@Override
Expand All @@ -1377,7 +1400,35 @@ public RMQueueAclInfo checkUserAccessToQueue(String queue, String username,
@Override
public AppAttemptInfo getAppAttempt(HttpServletRequest req,
HttpServletResponse res, String appId, String appAttemptId) {
throw new NotImplementedException("Code is not implemented");

if (appId == null || appId.isEmpty()) {
throw new IllegalArgumentException("Parameter error, the appId is empty or null.");
}
if (appAttemptId == null || appAttemptId.isEmpty()) {
throw new IllegalArgumentException("Parameter error, the appAttemptId is empty or null.");
}

try {
ApplicationId applicationId = ApplicationId.fromString(appId);
SubClusterInfo subClusterInfo = getHomeSubClusterInfoByAppId(applicationId);

if (subClusterInfo == null) {
RouterServerUtil.logAndThrowRunTimeException("Unable to get subCluster by applicationId = "
+ applicationId, null);
}

DefaultRequestInterceptorREST interceptor = getOrCreateInterceptorForSubCluster(
subClusterInfo.getSubClusterId(), subClusterInfo.getRMWebServiceAddress());
return interceptor.getAppAttempt(req, res, appId, appAttemptId);
} catch (IllegalArgumentException e) {
String msg = String.format("Unable to get the AppAttempt appId: %s, appAttemptId: %s.",
appId, appAttemptId);
RouterServerUtil.logAndThrowRunTimeException(msg, e);
Copy link
Member

@goiri goiri Aug 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we create a logAndThrowRunTimeException that already does the format?

Copy link
Contributor Author

@slfan1989 slfan1989 Aug 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good idea, I will modify the code.

} catch (YarnException e) {
RouterServerUtil.logAndThrowRunTimeException("getContainer Failed.", e);
}

return null;
}

@Override
Expand Down Expand Up @@ -1432,8 +1483,8 @@ public ContainerInfo getContainer(HttpServletRequest req,
SubClusterInfo subClusterInfo = getHomeSubClusterInfoByAppId(applicationId);

if (subClusterInfo == null) {
RouterServerUtil.logAndThrowRunTimeException("Unable to get subCluster by applicationId = " +
applicationId, null);
RouterServerUtil.logAndThrowRunTimeException("Unable to get subCluster by applicationId = "
+ applicationId, null);
}

DefaultRequestInterceptorREST interceptor = getOrCreateInterceptorForSubCluster(
Expand Down Expand Up @@ -1491,8 +1542,8 @@ public Response signalToContainer(String containerId, String command,
SubClusterInfo subClusterInfo = getHomeSubClusterInfoByAppId(applicationId);

if (subClusterInfo == null) {
RouterServerUtil.logAndThrowRunTimeException("Unable to get subCluster by applicationId = " +
applicationId, null);
RouterServerUtil.logAndThrowRunTimeException("Unable to get subCluster by applicationId = "
+ applicationId, null);
}

DefaultRequestInterceptorREST interceptor = getOrCreateInterceptorForSubCluster(
Expand Down Expand Up @@ -1565,7 +1616,8 @@ private <R> Map<SubClusterInfo, R> invokeConcurrent(Collection<SubClusterInfo> c
* @return HomeSubCluster
* @throws YarnException on failure
*/
private SubClusterInfo getHomeSubClusterInfoByAppId(ApplicationId applicationId) throws YarnException {
private SubClusterInfo getHomeSubClusterInfoByAppId(ApplicationId applicationId)
throws YarnException {
SubClusterInfo subClusterInfo = null;
SubClusterId subClusterId = null;
try {
Expand Down
53 changes: 53 additions & 0 deletions ...t/java/org/apache/hadoop/yarn/server/router/webapp/MockDefaultRequestInterceptorREST.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,11 @@
import org.apache.hadoop.yarn.api.records.ContainerReport;
import org.apache.hadoop.yarn.api.records.NodeLabel;
import org.apache.hadoop.yarn.api.records.SignalContainerCommand;
import org.apache.hadoop.yarn.api.records.ApplicationReport;
import org.apache.hadoop.yarn.api.records.YarnApplicationState;
import org.apache.hadoop.yarn.api.records.FinalApplicationStatus;
import org.apache.hadoop.yarn.api.records.ApplicationAttemptReport;
import org.apache.hadoop.yarn.api.records.YarnApplicationAttemptState;
import org.apache.hadoop.yarn.exceptions.ApplicationNotFoundException;
import org.apache.hadoop.yarn.exceptions.YarnException;
import org.apache.hadoop.yarn.server.federation.store.records.SubClusterId;
Expand All @@ -67,6 +72,8 @@
import org.apache.hadoop.yarn.server.resourcemanager.webapp.dao.ResourceOptionInfo;
import org.apache.hadoop.yarn.server.resourcemanager.webapp.dao.NodeToLabelsInfo;
import org.apache.hadoop.yarn.server.resourcemanager.webapp.dao.NodeLabelInfo;
import org.apache.hadoop.yarn.server.resourcemanager.webapp.dao.AppAttemptsInfo;
import org.apache.hadoop.yarn.server.webapp.dao.AppAttemptInfo;
import org.apache.hadoop.yarn.server.webapp.dao.ContainerInfo;
import org.apache.hadoop.yarn.server.webapp.dao.ContainersInfo;
import org.apache.hadoop.yarn.webapp.NotFoundException;
Expand Down Expand Up @@ -412,4 +419,50 @@ public Response signalToContainer(String containerId, String command,

return Response.status(Status.OK).build();
}

@Override
public org.apache.hadoop.yarn.server.webapp.dao.AppAttemptInfo getAppAttempt(HttpServletRequest req, HttpServletResponse res,
Copy link
Member

@goiri goiri Aug 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to specify the whole type? Where is the conflict?

Copy link
Contributor Author

@slfan1989 slfan1989 Aug 4, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found 2 conflicts

org.apache.hadoop.yarn.server.resourcemanager.webapp.dao.AppAttemptInfo
org.apache.hadoop.yarn.server.webapp.dao.AppAttemptInfo

The way to resolve the conflict, I will create the method in other classes(TestRouterWebServiceUtil#generateAppAttemptInfo)

String appId, String appAttemptId) {
if (!isRunning) {
throw new RuntimeException("RM is stopped");
}

ApplicationId applicationId = ApplicationId.fromString(appId);
if (!applicationMap.contains(applicationId)) {
throw new NotFoundException("app with id: " + appId + " not found");
}

ApplicationReport newApplicationReport = ApplicationReport.newInstance(
applicationId, ApplicationAttemptId.newInstance(applicationId, Integer.parseInt(appAttemptId)), "user",
"queue", "appname", "host", 124, null,
YarnApplicationState.RUNNING, "diagnostics", "url", 1, 2, 3, 4,
FinalApplicationStatus.SUCCEEDED, null, "N/A", 0.53789f, "YARN", null);

ApplicationAttemptReport attempt = ApplicationAttemptReport.newInstance(
ApplicationAttemptId.newInstance(applicationId, Integer.parseInt(appAttemptId)),
"host", 124, "url", "oUrl", "diagnostics",
YarnApplicationAttemptState.FINISHED, ContainerId.newContainerId(
newApplicationReport.getCurrentApplicationAttemptId(), 1));

return new AppAttemptInfo(attempt);
}

@Override
public AppAttemptsInfo getAppAttempts(HttpServletRequest hsr, String appId) {
if (!isRunning) {
throw new RuntimeException("RM is stopped");
}

ApplicationId applicationId = ApplicationId.fromString(appId);
if (!applicationMap.contains(applicationId)) {
throw new NotFoundException("app with id: " + appId + " not found");
}

AppAttemptsInfo infos = new AppAttemptsInfo();
org.apache.hadoop.yarn.server.resourcemanager.webapp.dao.AppAttemptInfo
appAttemptInfo1 = new org.apache.hadoop.yarn.server.resourcemanager.webapp.dao.AppAttemptInfo();
infos.add(appAttemptInfo1);

return infos;
}
}
34 changes: 34 additions & 0 deletions .../test/java/org/apache/hadoop/yarn/server/router/webapp/TestFederationInterceptorREST.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
import org.apache.hadoop.yarn.server.resourcemanager.webapp.dao.NodeLabelsInfo;
import org.apache.hadoop.yarn.server.resourcemanager.webapp.dao.NodeLabelInfo;
import org.apache.hadoop.yarn.server.resourcemanager.webapp.dao.LabelsToNodesInfo;
import org.apache.hadoop.yarn.server.resourcemanager.webapp.dao.AppAttemptsInfo;
import org.apache.hadoop.yarn.server.resourcemanager.webapp.NodeIDsInfo;
import org.apache.hadoop.yarn.server.webapp.dao.ContainerInfo;
import org.apache.hadoop.yarn.server.webapp.dao.ContainersInfo;
Expand Down Expand Up @@ -717,4 +718,37 @@ public void testGetContainer()
appId.toString(), appAttemptId.toString(), "0");
Assert.assertNotNull(containerInfo);
}

@Test
public void testGetAppAttempts()
throws IOException, InterruptedException, YarnException {
// Submit application to multiSubCluster
ApplicationId appId = ApplicationId.newInstance(Time.now(), 1);
ApplicationSubmissionContextInfo context = new ApplicationSubmissionContextInfo();
context.setApplicationId(appId.toString());

Assert.assertNotNull(interceptor.submitApplication(context, null));

AppAttemptsInfo appAttemptsInfo = interceptor.getAppAttempts(null, appId.toString());
Assert.assertNotNull(appAttemptsInfo);
Assert.assertTrue(!appAttemptsInfo.getAttempts().isEmpty());
Copy link
Member

@goiri goiri Aug 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we check before the submission too?
It would be nice to check something more specific than not empty.

Copy link
Contributor Author

@slfan1989 slfan1989 Aug 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your suggestion is right, this part needs to be refactored, I will fix it.

}

@Test
public void testGetAppAttempt()
throws IOException, InterruptedException, YarnException {
// Submit application to multiSubCluster
ApplicationId appId = ApplicationId.newInstance(System.currentTimeMillis(), 1);
ApplicationSubmissionContextInfo context = new ApplicationSubmissionContextInfo();
context.setApplicationId(appId.toString());
Assert.assertNotNull(interceptor.submitApplication(context, null));
ApplicationAttemptId appAttemptIdExcept = ApplicationAttemptId.newInstance(appId, 1);
Copy link
Member

@goiri goiri Aug 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Except? Expect?

Copy link
Member

@goiri goiri Aug 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably create the appId and attemptId one after the other.


org.apache.hadoop.yarn.server.webapp.dao.AppAttemptInfo
appAttemptInfo = interceptor.getAppAttempt(null, null, appId.toString(),
appAttemptIdExcept.toString());
Assert.assertNotNull(appAttemptInfo);

Assert.assertEquals(appAttemptIdExcept, appAttemptInfo.getAppAttemptId());
}
}