[jira] [Comment Edited] (YARN-4767) Network issues can cause persistent RM UI outage

["Karthik Kambatla (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/YARN-4767?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15300422#comment-15300422 ] 

Karthik Kambatla edited comment on YARN-4767 at 5/25/16 5:01 PM:
-----------------------------------------------------------------

The patch looks good to me, but for the following nits:
# More an observation. We seem to be appending "R" at the end to capture a redirection. It seems a little hacky. The alternative of defining a special class with a boolean to capture redirection seems rather heavy and unnecessary. Can we add a more simple test that verifies this addition of "R" so it is easy for the developer changing surrounding code in the future. 
# AppBlock#generateOverviewTable: The ternary operators aren't following the coding conventions and are also hard to read. Mind updating them to make sure "?" and the associated value are on the same line, and ":" and the associated value are on the same line? And, for nested ternary operator, additional indentation is good. For instance, 
{code}
        app.getTrackingUrl() == null || app.getTrackingUrl().equals(UNAVAILABLE) 
            ? null 
            : root_url(app.getTrackingUrl()),
        app.getTrackingUrl() == null || app.getTrackingUrl().equals(UNAVAILABLE)
            ? "Unassigned"
            : app.getAppState() == YarnApplicationState.FINISHED ||
              app.getAppState() == YarnApplicationState.FAILED ||
              app.getAppState() == YarnApplicationState.KILLED
              ? "History"
              : "ApplicationMaster");
{code}

[~xgong], [~vinodkv] - you are more familiar with the redirection and proxy code. Mind taking a quick look? I think we should try and get this into 2.8.0. 


was (Author: kasha):
The patch looks good to me, but for the following nits:
# More an observation. We seem to be appending "R" at the end to capture a redirection. It seems a little hacky. The alternative of defining a special class with a boolean to capture redirection seems rather heavy and unnecessary. Can we add a more simple test that verifies this addition of "R" so it is easy for the developer changing surrounding code in the future. 
# AppBlock#generateOverviewTable: The ternary operators aren't following the coding conventions and are also hard to read. Mind updating them to make sure "?" and the associated value are on the same line, and ":" and the associated value are on the same line? And, for nested ternary operator, additional indentation is good. For instance, 
{code}
        app.getTrackingUrl() == null || app.getTrackingUrl().equals(UNAVAILABLE) 
            ? null 
            : root_url(app.getTrackingUrl()),
        app.getTrackingUrl() == null || app.getTrackingUrl().equals(UNAVAILABLE)
            ? "Unassigned"
            : app.getAppState() == YarnApplicationState.FINISHED ||
              app.getAppState() == YarnApplicationState.FAILED ||
              app.getAppState() == YarnApplicationState.KILLED
              ? "History"
              : "ApplicationMaster");
{code}

[~xgong], [~vinodkv] - you are more familiar with the redirection and proxy good. Mind taking a quick look? I think we should try and get this into 2.8.0. 

> Network issues can cause persistent RM UI outage
> ------------------------------------------------
>
>                 Key: YARN-4767
>                 URL: https://issues.apache.org/jira/browse/YARN-4767
>             Project: Hadoop YARN
>          Issue Type: Bug
>          Components: webapp
>    Affects Versions: 2.7.2
>            Reporter: Daniel Templeton
>            Assignee: Daniel Templeton
>            Priority: Critical
>         Attachments: YARN-4767.001.patch, YARN-4767.002.patch, YARN-4767.003.patch, YARN-4767.004.patch, YARN-4767.005.patch, YARN-4767.006.patch
>
>
> If a network issue causes an AM web app to resolve the RM proxy's address to something other than what's listed in the allowed proxies list, the AmIpFilter will 302 redirect the RM proxy's request back to the RM proxy.  The RM proxy will then consume all available handler threads connecting to itself over and over, resulting in an outage of the web UI.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: yarn-issues-help@hadoop.apache.org