[ https://issues.apache.org/jira/browse/YARN-4428?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15120181#comment-15120181 ]
Jason Lowe commented on YARN-4428: ---------------------------------- Thanks for updating the patch. We're almost there! - ToDo should be TODO to be consistent with other TODO's in the source - The point of using slf4j instead of the commons logging is to _not_ have to check for debug enabled before logging. The log statement should use the slf4j string formatting directives and pass the parts as arguments rather than construct it explicitly, which is why we don't have to check first. For example, this: {code} if (LOG.isDebugEnabled()) { LOG.debug("String " + parts[3] + " fails to be parsed as ApplicationAttemptId.", e); } {code} should become something simpler like this: {code} LOG.debug("Error parsing '{}' as an ApplicationAttemptId", parts[3], e); {code} - ahsRedirectPath should just early-out with null if the ahs is not enabled rather than having each switch case check for it. There's no need to waste time trying to parse the URL looking for apps, attempts, or containers if the AHS isn't enabled, and that's an extremely fast check to do up front now that it's cached. > Redirect RM page to AHS page when AHS turned on and RM page is not avaialable > ----------------------------------------------------------------------------- > > Key: YARN-4428 > URL: https://issues.apache.org/jira/browse/YARN-4428 > Project: Hadoop YARN > Issue Type: Bug > Reporter: Chang Li > Assignee: Chang Li > Attachments: YARN-4428.1.2.patch, YARN-4428.1.patch, > YARN-4428.2.2.patch, YARN-4428.2.patch, YARN-4428.3.patch, YARN-4428.3.patch, > YARN-4428.4.patch, YARN-4428.5.patch, YARN-4428.6.patch, YARN-4428.7.patch > > > When AHS is turned on, if we can't view application in RM page, RM page > should redirect us to AHS page. For example, when you go to > cluster/app/application_1, if RM no longer remember the application, we will > simply get "Failed to read the application application_1", but it will be > good for RM ui to smartly try to redirect to AHS ui > /applicationhistory/app/application_1 to see if it's there. The redirect > usage already exist for logs in nodemanager UI. > Also, when AHS is enabled, WebAppProxyServlet should redirect to AHS page on > fall back of RM not remembering the app. YARN-3975 tried to do this only when > original tracking url is not set. But there are many cases, such as when app > failed at launch, original tracking url will be set to point to RM page, so > redirect to AHS page won't work. -- This message was sent by Atlassian JIRA (v6.3.4#6332)