Naganarasimha G R updated YARN-2934:
    Attachment: YARN-2934.v1.007.patch

Thanks for the review [~jira.shegalov],
bq. Please make sure that the patch does not introduce new problems. Both 
checkstyle and findbugs report problems related to the patch. 
Well, was earlier not sure whether the findbugs issue reported was valid one, 
went through the description given in the finds bugs report got the 
understanding about it!  Thought of correcting the synchronization issue but 
based on your last comment made it to fetch everytime from the conf, in the 
latest patch.
Most of the issues reported in the checkstyle are not directly induced by the 
patch or is ok to live with. Valid issues are incorporated in the latest patch

bq. I'd rather have a longer config value than adding more code to make 
patterns case-insensitive. In practice we mostly need stderr
Would like to differ here, IMHO code added is not much and anyway i have 
already finished coding it and when there is a way possible to avoid 
configuring multiple cases, then why expect the user to configure for both 
cases ? Though most cases {{stderr}} is sufficient, its like any kind of app 
can be submitted and the pattern can be mix and match of the cases too, ex . in 
distributed shell client for am log {{"AppMaster.stderr"}} is used as the error 
file name (emphasizing different cases here though it has stderr).
I am open to modify to glob approach if there is any flaw/disadvantage in the 
current approach !

 bq. In general, don't try optimize for the failure case. Things like look like 
a bug. Simply get it from conf exactly when it's needed.
Ok have corrected for the tail size and in the similar lines for {{pattern}} 
too, in the latest patch.

> 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, YARN-2934.v1.005.patch, 
> YARN-2934.v1.006.patch, YARN-2934.v1.007.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

Reply via email to