Skip to content

Conversation

renozhang
Copy link
Contributor

Assign value of yarn container log directory to java opts "spark.yarn.app.container.log.dir", So user defined log4j.properties can reference this value and write log to YARN container's log directory.
Otherwise, user defined file appender will only write to container's CWD, and log files in CWD will not be displayed on YARN UI,and either cannot be aggregated to HDFS log directory after job finished.

User defined log4j.properties reference example:
log4j.appender.rolling_file.File = ${spark.yarn.app.container.log.dir}/spark.log

@SparkQA
Copy link

SparkQA commented Jul 24, 2014

QA tests have started for PR 1573. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/17119/consoleFull

@SparkQA
Copy link

SparkQA commented Jul 24, 2014

QA results for PR 1573:
- This patch PASSES unit tests.
- This patch merges cleanly
- This patch adds no public classes

For more information see test ouptut:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/17119/consoleFull

@renozhang renozhang changed the title [YARN] SPARK-2668: Support log4j log to yarn container dir [YARN] SPARK-2668: Add variable of yarn log diectory to reference from the log4j configuration Jul 25, 2014
@renozhang renozhang changed the title [YARN] SPARK-2668: Add variable of yarn log diectory to reference from the log4j configuration [YARN] SPARK-2668: Add variable of yarn log directory to reference from the log4j configuration Jul 25, 2014
@renozhang renozhang changed the title [YARN] SPARK-2668: Add variable of yarn log directory to reference from the log4j configuration [YARN] SPARK-2668: Add variable of yarn log directory for reference from the log4j configuration Jul 25, 2014
@tgravescs
Copy link
Contributor

@renozhang sorry for the delay on this, could you upmerge to the latest?

@renozhang
Copy link
Contributor Author

@tgravescs I've update to the latest, thanks for review.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you rename this to be spark.yarn.app.container.log.dir

Can you also update the documentation in docs/running-on-yarn.md to have this config and a good description.

@renozhang
Copy link
Contributor Author

Sorry, I missed writing description in PR. I'll fill description in PR later.

As metioned in descritpion of Jira SPARK-2668:

Adding this varialbe is for user to define custom log4j.properties, eg:

log4j.appender.rolling_file.File = ${spark.yarn.log.dir}/spark.log

发件人: andrewor14 <notifications@github.commailto:notifications@github.com>
答复: apache/spark <reply@reply.github.commailto:reply@reply.github.com>
日期: Thu, 11 Sep 2014 18:08:37 -0700
至: apache/spark <spark@noreply.github.commailto:spark@noreply.github.com>
抄送: Peng <peng.zhang@xiaomi.commailto:peng.zhang@xiaomi.com>
主题: Re: [spark] [YARN] SPARK-2668: Add variable of yarn log directory for reference from the log4j configuration (#1573)

Where is this config being consumed? Am I missing something obvious?


Reply to this email directly or view it on GitHubhttps://github.com//pull/1573#issuecomment-55350201.

@andrewor14
Copy link
Contributor

Yup, thanks for answering my deleted question. I realized this afterwards after reading the JIRA.

@tgravescs
Copy link
Contributor

@renozhang can you address my comments

@renozhang
Copy link
Contributor Author

Sorry @tgravescs , these days very busy, I'll address them this weekend.

@SparkQA
Copy link

SparkQA commented Sep 21, 2014

QA tests have started for PR 1573 at commit c56aba6.

  • This patch merges cleanly.

@renozhang
Copy link
Contributor Author

@tgravescs patch updated, thanks for your review.

@SparkQA
Copy link

SparkQA commented Sep 21, 2014

QA tests have finished for PR 1573 at commit c56aba6.

  • This patch passes unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this could be used in more then just streaming applications perhaps we should reword this a little. Perhaps put the information about ${spark.yarn.app.container.log.dir} first and then give the example using RollingFileAppender with streaming.

Something more like: (note feel free to change the exact wording)

If you need a reference to the proper location to put the log files in the YARN so that YARN can properly display and aggregate them, use "${spark.yarn.app.container.log.dir}" in your log4j.properties. For example... (then explain the streaming example).

@tgravescs
Copy link
Contributor

thanks @renozhang, minor request about the documentation, otherwise looks good.

@SparkQA
Copy link

SparkQA commented Sep 23, 2014

QA tests have started for PR 1573 at commit 16c5cb8.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Sep 23, 2014

QA tests have finished for PR 1573 at commit 16c5cb8.

  • This patch fails unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Sep 23, 2014

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/20697/

@tgravescs
Copy link
Contributor

testfailure is unrelated to this pr

@tgravescs
Copy link
Contributor

+1 thanks @renozhang !

@asfgit asfgit closed this in 14f8c34 Sep 23, 2014
@renozhang renozhang deleted the yarn-log-dir branch June 28, 2016 10:02
sunchao pushed a commit to sunchao/spark that referenced this pull request Jun 2, 2023
…1573)

We recently hit by an issue due to a Hive upgrade doesn’t work with Iceberg. As Apple Spark is heavily used with Iceberg in the production, any change at Spark has a risk to affect Iceberg function. But we don’t run any tests against Iceberg at the moment.

To prevent similar issue on Iceberg side, it would be nice if we can run Iceberg unit tests in Apple Spark Rio pipeline.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants