[
https://issues.apache.org/jira/browse/TEZ-3326?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15450372#comment-15450372
]
Eric Badger commented on TEZ-3326:
----------------------------------
[~hitesh], thanks for the comments and review!
{quote}
I would assume that downstream projects should not change atleast initially and
for some period, both YARN and the app would do some duplicate logging. Over
time as the feature stabilizes and becomes default enabled with the required
configs, the apps could change to disable their internal logging.
In any case, for this particular patch, I think we can push it into Tez in any
case as YARN does not have the full required functionality today.
{quote}
That all makes sense to me. I think we can make this change so that we don't
have to wait for YARN.
bq. minor nit: helper function belongs to TezUtils or TezCommonUtils - dont
believe if it is related to YARN.
Agreed. I think it fits in TezUtils, since it's related to
addLog4jSystemProperties.
bq. was the main reason of the function returning a string instead of logging
directly to log as part of the callee class and also to aid unit testing? The
function could easily be changed to do the conf parsing and the logging? Just
curious about the reason - code does not need to change.
This function is mimicking getSystemPropertiesToLog in MRApps. It appears that
the motivation for returning a string as opposed to directly logging is to
avoid logging a line with no message. And as you pointed out, it does make the
unit testing easier, since the test can check the return value instead of
having to scrape the log itself.
bq. Should we be using system.out to log this info to ensure that it always
gets logged e.g for some reason log4j props is misconfigured or log level set
to error?
Not really sure about this one. I think I would lean on the side of leaving it
up to the user to define the log level, but it wouldn't bother me either way.
bq. conf.getStrings() as compared to conf.get() ?
It seemed more intuitive to use conf.getStrings(), but I used conf.get() since
I'm getting a single string and would like to use a default value if the
parameter isn't set. Is there a way I can do that with conf.getStrings()?
> Display JVM system properties in AM and task logs
> -------------------------------------------------
>
> Key: TEZ-3326
> URL: https://issues.apache.org/jira/browse/TEZ-3326
> Project: Apache Tez
> Issue Type: Improvement
> Reporter: Ming Ma
> Assignee: Eric Badger
> Attachments: TEZ-3326.001.patch, TEZ-3326.002.patch,
> TEZ-3326.003.patch
>
>
> MapReduce displays JVM system properties via config
> {{mapreduce.jvm.system-properties-to-log}} in both AM and task log . This is
> useful to debug env setting such as java version, etc. It is useful to have
> such logging in Tez.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
