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

Varun Saxena commented on YARN-2934:
------------------------------------

Thanks [~Naganarasimha] for uploading the patch. Sorry could not do a thorough 
review earlier.
Had a cursory glance at the latest patch. A few quick comments.

* In the code below, *instead of compiling the pattern again and again*, we can 
compile it once and store it in a static variable(because its taken from config 
and hence wont change).
Pattern#compile incurs a performance overhead if called again and again.
{code}
    String errorFileNameRegexPattern =
       conf.get(YarnConfiguration.NM_CONTAINER_ERROR_FILE_NAME_PATTERN,
             YarnConfiguration.DEFAULT_NM_CONTAINER_ERROR_FILE_NAME_PATTERN);
    Pattern pattern = null;
    try {
      pattern =
          Pattern.compile(errorFileNameRegexPattern, Pattern.CASE_INSENSITIVE);
    } catch (PatternSyntaxException e) {
      pattern = Pattern.compile(
          YarnConfiguration.DEFAULT_NM_CONTAINER_ERROR_FILE_NAME_PATTERN,
          Pattern.CASE_INSENSITIVE);
   }
{code}

* Also IMO, atleast a warning log should be printed if configured pattern 
cannot compile. This can alert the user about wrong configuration.
Should we consider not starting up NM in this case(if config is wrong) ? Maybe 
its not that important a config to not start NM. An alert message should be 
enough.
* Moreover, you can also consider using Configuration#getPattern, but take care 
of using it only once.

> Improve handling of container's stderr 
> ---------------------------------------
>
>                 Key: YARN-2934
>                 URL: https://issues.apache.org/jira/browse/YARN-2934
>             Project: Hadoop YARN
>          Issue Type: Improvement
>            Reporter: Gera Shegalov
>            Assignee: Naganarasimha G R
>            Priority: Critical
>         Attachments: YARN-2934.v1.001.patch, YARN-2934.v1.002.patch, 
> YARN-2934.v1.003.patch, YARN-2934.v1.004.patch
>
>
> Most YARN applications redirect stderr to some file. That's why when 
> container launch fails with {{ExitCodeException}} the message is empty.



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

Reply via email to