[
https://issues.apache.org/jira/browse/YARN-11137?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
fanshilun updated YARN-11137:
-----------------------------
Description:
While reading the relevant yarn-federation-router's code, I found the following
issues with logging in FederationClientInterceptor:
*Q1:*
The implemented methods are inconsistent with the use of RouterAuditLogger，as
follows:
org.apache.hadoop.yarn.server.router.clientrmsubmit.FederationClientInterceptor#submitApplication，After
judging that the request is null, directly throws YarnException
{code:java}
if (request == null || request.getApplicationId() == null) {
routerMetrics.incrAppsFailedKilled();
String message = "Missing forceKillApplication request or ApplicationId.";
RouterAuditLogger.logFailure(user.getShortUserName(),
RouterAuditLogger.AuditConstants.FORCE_KILL_APP, "UNKNOWN",
"RouterClientRMService", message);
throw new YarnException(message);
} {code}
org.apache.hadoop.yarn.server.router.clientrmsubmit.FederationClientInterceptor#getApplications，After
judging that the request is null, Use RouterServerUtil.logAndThrowException
for handling
{code:java}
if (request == null) {
routerMetrics.incrMultipleAppsFailedRetrieved();
RouterServerUtil.logAndThrowException(
"Missing getApplications request.",
null);
} {code}
I think the second way is better.
*Q2:*
The logging methods are inconsistent, some use the splicing method, and some
use the placeholder method，as follows:
org.apache.hadoop.yarn.server.router.clientrmsubmit.FederationClientInterceptor#getNewApplication
{code:java}
for (int i = 0; i < numSubmitRetries; ++i) {
SubClusterId subClusterId = getRandomActiveSubCluster(subClustersActive);
LOG.debug(
"getNewApplication try #{} on SubCluster {}", i, subClusterId);
ApplicationClientProtocol clientRMProxy =
getClientRMProxyForSubCluster(subClusterId);
...
}{code}
org.apache.hadoop.yarn.server.router.clientrmsubmit.FederationClientInterceptor#submitApplication
{code:java}
for (int i = 0; i < numSubmitRetries; ++i) { SubClusterId subClusterId =
policyFacade.getHomeSubcluster(
request.getApplicationSubmissionContext(), blacklist);
LOG.info("submitApplication appId" + applicationId + " try #" + i
+ " on SubCluster " + subClusterId);
...
} {code}
I think the first way is better.
was:
While reading the relevant yarn-federation-router's code, I found the following
issues with logging in FederationClientInterceptor:
*Q1:*
The implemented methods are inconsistent with the use of RouterAuditLogger，as
follows:
org.apache.hadoop.yarn.server.router.clientrmsubmit.FederationClientInterceptor#submitApplication，After
judging that the request is null, directly throws YarnException
{code:java}
if (request == null || request.getApplicationId() == null) {
routerMetrics.incrAppsFailedKilled();
String message = "Missing forceKillApplication request or ApplicationId.";
RouterAuditLogger.logFailure(user.getShortUserName(),
RouterAuditLogger.AuditConstants.FORCE_KILL_APP, "UNKNOWN",
"RouterClientRMService", message);
throw new YarnException(message);
} {code}
org.apache.hadoop.yarn.server.router.clientrmsubmit.FederationClientInterceptor#getApplications，After
judging that the request is null, Use RouterServerUtil.logAndThrowException
for handling
{code:java}
if (request == null) {
routerMetrics.incrMultipleAppsFailedRetrieved();
RouterServerUtil.logAndThrowException(
"Missing getApplications request.",
null);
} {code}
*Q2:*
The logging methods are inconsistent, some use the splicing method, and some
use the placeholder method，as follows:
org.apache.hadoop.yarn.server.router.clientrmsubmit.FederationClientInterceptor#getNewApplication
{code:java}
for (int i = 0; i < numSubmitRetries; ++i) {
SubClusterId subClusterId = getRandomActiveSubCluster(subClustersActive);
LOG.debug(
"getNewApplication try #{} on SubCluster {}", i, subClusterId);
ApplicationClientProtocol clientRMProxy =
getClientRMProxyForSubCluster(subClusterId);
...
}{code}
org.apache.hadoop.yarn.server.router.clientrmsubmit.FederationClientInterceptor#submitApplication
{code:java}
for (int i = 0; i < numSubmitRetries; ++i) { SubClusterId subClusterId =
policyFacade.getHomeSubcluster(
request.getApplicationSubmissionContext(), blacklist);
LOG.info("submitApplication appId" + applicationId + " try #" + i
+ " on SubCluster " + subClusterId);
...
} {code}
*Q3:*
> Improve log message in FederationClientInterceptor
> --------------------------------------------------
>
> Key: YARN-11137
> URL: https://issues.apache.org/jira/browse/YARN-11137
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: federation
> Affects Versions: 3.4.0
> Reporter: fanshilun
> Assignee: fanshilun
> Priority: Minor
> Labels: pull-request-available
> Fix For: 3.4.0
>
> Attachments: YARN-11137.01.patch
>
> Time Spent: 1h 50m
> Remaining Estimate: 0h
>
> While reading the relevant yarn-federation-router's code, I found the
> following issues with logging in FederationClientInterceptor:
> *Q1:*
> The implemented methods are inconsistent with the use of RouterAuditLogger，as
> follows:
> org.apache.hadoop.yarn.server.router.clientrmsubmit.FederationClientInterceptor#submitApplication，After
> judging that the request is null, directly throws YarnException
> {code:java}
> if (request == null || request.getApplicationId() == null) {
> routerMetrics.incrAppsFailedKilled();
> String message = "Missing forceKillApplication request or
> ApplicationId.";
> RouterAuditLogger.logFailure(user.getShortUserName(),
> RouterAuditLogger.AuditConstants.FORCE_KILL_APP, "UNKNOWN",
> "RouterClientRMService", message);
> throw new YarnException(message);
> } {code}
> org.apache.hadoop.yarn.server.router.clientrmsubmit.FederationClientInterceptor#getApplications，After
> judging that the request is null, Use RouterServerUtil.logAndThrowException
> for handling
> {code:java}
> if (request == null) {
> routerMetrics.incrMultipleAppsFailedRetrieved();
> RouterServerUtil.logAndThrowException(
> "Missing getApplications request.",
> null);
> } {code}
> I think the second way is better.
> *Q2:*
> The logging methods are inconsistent, some use the splicing method, and some
> use the placeholder method，as follows:
> org.apache.hadoop.yarn.server.router.clientrmsubmit.FederationClientInterceptor#getNewApplication
> {code:java}
> for (int i = 0; i < numSubmitRetries; ++i) {
> SubClusterId subClusterId =
> getRandomActiveSubCluster(subClustersActive);
> LOG.debug(
> "getNewApplication try #{} on SubCluster {}", i, subClusterId);
> ApplicationClientProtocol clientRMProxy =
> getClientRMProxyForSubCluster(subClusterId);
> ...
> }{code}
> org.apache.hadoop.yarn.server.router.clientrmsubmit.FederationClientInterceptor#submitApplication
> {code:java}
> for (int i = 0; i < numSubmitRetries; ++i) { SubClusterId subClusterId =
> policyFacade.getHomeSubcluster(
> request.getApplicationSubmissionContext(), blacklist);
> LOG.info("submitApplication appId" + applicationId + " try #" + i
> + " on SubCluster " + subClusterId);
> ...
> } {code}
> I think the first way is better.
>
>
>
>
>
--
This message was sent by Atlassian Jira
(v8.20.7#820007)
---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
