hadoop-yarn-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Gera Shegalov (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (YARN-2934) Improve handling of container's stderr
Date Fri, 18 Dec 2015 00:06:46 GMT

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

Gera Shegalov commented on YARN-2934:
-------------------------------------

Thanks for the latest patch. good to see the patch lose 3kb, most of all there are no more
changes to the common Configuration class.

One checkstyle issue is the 80-column warning is from the patch around:
{code}
        long tailSizeInBytes = conf.getLong(
            YarnConfiguration.NM_CONTAINER_ERROR_FILE_TAIL_SIZE_IN_BYTES,
            YarnConfiguration.DEFAULT_NM_CONTAINER_ERROR_FILE_TAIL_SIZE_IN_BYTES);
{code}

Those are pretty long names:
Can we do: 
container.stderr.tail.bytes 
NM_CONTAINER_STDERR_BYTES
and the corresponding default. Having stderr in name is also great for users to understand
what error file is meant in 99% of the cases.
Same thing is for container.stderr.pattern

Still don't see any value in this, please drop:
{code}
        if (listStatus.length > 1) {
          LOG.error("Multiple files in " + containerLogDir
              + ", seems to match the error file name pattern configured. "
              + Arrays.toString(listStatus));
        }
{code}

Let us not guard the tail read:
{code}
        if (fileSize != 0) {
{code}
and there is a value in seeing that the file is empty already on the client-side.

Instead of 
{code}
IOUtils.closeStream(errorFileIS) 
{code}

call cleanup so we can pass the logger
{code}
IOUtils.cleanup(LOG, errorFileIS)
{code}

Since the trunk is on JDK7 min:
We can drop the constant UTF_8 and use
in 
{code}
new String(tailBytes, StandardCharsets.UTF_8)
{code}

listStatus as a name for a variable is not intuitive. Maybe use errFileStatus for that. 

Obviously I meant tailSizeInBytes, thanks for paying attention. Agree that RLFS file status
toString might look too ugly.

We can FileUtil.stat2Paths or add a loop here to extract just the last path component. 

Also realizing that we should have a low cap on the tail size to prevent a misconfiguration
to knock out NM with OOM on container failures since we do:
{code}
byte[] tailBytes = new byte[bufferSize];
{code}

One can easily see why I initially confused tailBytes for an int. It should be called along
the lines {code}tailBuffer{code} 






> 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,
YARN-2934.v1.008.patch, YARN-2934.v2.001.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)

Mime
View raw message