[SPARK-6305][CORE][TEST][FOLLOWUP] Add LoggingSuite and some improvements #34965

viirya · 2021-12-21T03:35:44Z

What changes were proposed in this pull request?

This patch proposes to add LoggingSuite back and also does some other improvements. In summary:

Add LoggingSuite back
Refactor logging related change based on community suggestion, e.g. let SparkShellLoggingFilter inherit from AbstractFilter instead of Filter.
Fix maven test failures for hive-thriftserver module and restore previous log level to reduce log size
Fix K8S decommision integration tests which check log output
A few places in code/doc which refer/mention log4j.properties

Why are the changes needed?

LoggingSuite was wrongly removed in previous PR. We should add it back. There are a few places we can also simplify the code. A few places in code which programmingly write out log4j properties files are also changed to log4j2 here.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Pass all tests.

SparkQA · 2021-12-21T04:29:30Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50892/

SparkQA · 2021-12-21T05:27:59Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50892/

SparkQA · 2021-12-21T07:40:11Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50896/

SparkQA · 2021-12-21T08:54:15Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50896/

SparkQA · 2021-12-21T09:29:12Z

Test build #146421 has finished for PR 34965 at commit 0ac57d6.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

...ftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/HiveThriftServer2Suites.scala

SparkQA · 2021-12-21T10:34:51Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50908/

SparkQA · 2021-12-21T11:42:05Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50908/

SparkQA · 2021-12-21T12:26:16Z

Test build #146433 has finished for PR 34965 at commit 7067976.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2021-12-21T18:33:52Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50923/

SparkQA · 2021-12-21T19:29:11Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50923/

SparkQA · 2021-12-21T19:37:14Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50927/

viirya · 2021-12-21T20:01:09Z

retest this please

SparkQA · 2021-12-21T20:04:52Z

Test build #146453 has started for PR 34965 at commit 452515e.

SparkQA · 2021-12-21T20:34:19Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50927/

SparkQA · 2021-12-21T20:44:56Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50928/

SparkQA · 2021-12-21T21:35:42Z

Test build #146452 has finished for PR 34965 at commit 452515e.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2021-12-21T21:36:44Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50928/

SparkQA · 2021-12-22T19:40:08Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50969/

SparkQA · 2021-12-22T20:44:54Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50969/

SparkQA · 2021-12-22T20:53:49Z

Test build #146488 has finished for PR 34965 at commit 8d79c65.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

viirya · 2021-12-22T21:10:48Z

cc @dongjoon-hyun @JoshRosen @srowen

viirya · 2021-12-22T21:12:30Z

resource-managers/kubernetes/integration-tests/dev/dev-run-integration-tests.sh

  -Dspark.kubernetes.test.jvmImage=$JVM_IMAGE_NAME
  -Dspark.kubernetes.test.pythonImage=$PYTHON_IMAGE_NAME
  -Dspark.kubernetes.test.rImage=$R_IMAGE_NAME
-  -Dlog4j.logger.org.apache.spark=DEBUG


This seems not work with log4j 2.x. So for K8S decommission integration tests which check logs, I updated the test code to set up a log4j2.properties.

viirya · 2021-12-22T21:13:50Z

sql/hive-thriftserver/src/test/resources/log4j2.properties


 appender.console.filter.1.a.type = ThresholdFilter
-appender.console.filter.1.a.level = info
+appender.console.filter.1.a.level = warn


followed original log4j1 properties file.

viirya · 2021-12-22T21:14:19Z

resource-managers/kubernetes/integration-tests/src/test/resources/log4j2.properties

 appender.file.type = File
 appender.file.name = File
-appender.file.fileName = target/unit-tests.log
+appender.file.fileName = target/integration-tests.log


I coped it wrongly in previous PR.

SparkQA · 2021-12-22T21:52:41Z

Test build #146493 has finished for PR 34965 at commit bd5ef16.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

resource-managers/yarn/src/main/scala/org/apache/spark/deploy/yarn/Client.scala

SparkQA · 2021-12-23T10:38:32Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50988/

SparkQA · 2021-12-23T11:15:52Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50988/

SparkQA · 2021-12-23T12:49:30Z

Test build #146512 has finished for PR 34965 at commit 371f27e.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

srowen

Looks good. This could be a follow-up on the main log4j 2.x JIRA, as some downstream may be looking for a clear place to find the log4j 2 changes, but not important

core/src/test/scala/org/apache/spark/SparkFunSuite.scala

viirya · 2021-12-23T18:46:50Z

Thanks @srowen . Changed the PR title to a follow-up on the main log4j 2.x JIRA.

SparkQA · 2021-12-23T19:37:28Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/51011/

SparkQA · 2021-12-23T20:38:30Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/51011/

SparkQA · 2021-12-23T21:40:27Z

Test build #146536 has finished for PR 34965 at commit 2c233c9.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

viirya · 2021-12-23T22:14:30Z

Thanks for review. I will merge this tonight.

viirya · 2021-12-24T03:40:40Z

Merging to master.

### What changes were proposed in this pull request? This reverts commit c0cedb1. ### Why are the changes needed? #35011 changes log level and causes some tests that check log message failed. Actually #34965 was merged not long ago which fixes logging issue for hive-thriftserver module that produces large log output. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? Pass tests. Closes #35014 from viirya/revert-SPARK-37733. Authored-by: Liang-Chi Hsieh <[email protected]> Signed-off-by: Hyukjin Kwon <[email protected]>

…ents This patch proposes to add `LoggingSuite` back and also does some other improvements. In summary: 1. Add `LoggingSuite` back 2. Refactor logging related change based on community suggestion, e.g. let `SparkShellLoggingFilter` inherit from `AbstractFilter` instead of `Filter`. 3. Fix maven test failures for hive-thriftserver module 4. Fix K8S decommision integration tests which check log output 5. A few places in code/doc which refer/mention log4j.properties `LoggingSuite` was wrongly removed in previous PR. We should add it back. There are a few places we can also simplify the code. A few places in code which programmingly write out log4j properties files are also changed to log4j2 here. No Pass all tests. Closes apache#34965 from viirya/log4j2_improvement. Authored-by: Liang-Chi Hsieh <[email protected]> Signed-off-by: Liang-Chi Hsieh <[email protected]>

…ents This patch proposes to add `LoggingSuite` back and also does some other improvements. In summary: 1. Add `LoggingSuite` back 2. Refactor logging related change based on community suggestion, e.g. let `SparkShellLoggingFilter` inherit from `AbstractFilter` instead of `Filter`. 3. Fix maven test failures for hive-thriftserver module 4. Fix K8S decommision integration tests which check log output 5. A few places in code/doc which refer/mention log4j.properties `LoggingSuite` was wrongly removed in previous PR. We should add it back. There are a few places we can also simplify the code. A few places in code which programmingly write out log4j properties files are also changed to log4j2 here. No Pass all tests. Closes apache#34965 from viirya/log4j2_improvement. Authored-by: Liang-Chi Hsieh <[email protected]> Signed-off-by: Liang-Chi Hsieh <[email protected]> (cherry picked from commit 7fd3619)

Add LoggingSuite and some improvements.

47cffb9

github-actions bot added the CORE label Dec 21, 2021

This comment has been minimized.

Sign in to view

Copy log event first.

0ac57d6

For hive thriftserver

7067976

github-actions bot added the SQL label Dec 21, 2021

viirya mentioned this pull request Dec 21, 2021

[SPARK-6305][BUILD][FOLLOWUP] Some cleanups for log4j properties #34959

Closed

LuciferYang reviewed Dec 21, 2021

View reviewed changes

...ftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/HiveThriftServer2Suites.scala Show resolved Hide resolved

This comment has been minimized.

Sign in to view

viirya added 2 commits December 21, 2021 09:58

Update UISeleniumSuite

96c870c

Set console filter to warn

452515e

viirya force-pushed the log4j2_improvement branch from 3c058d6 to 452515e Compare December 21, 2021 18:11

viirya changed the title ~~[SPARK-37700][CORE][TEST] Add LoggingSuite and some improvements~~ [SPARK-37700][CORE][TEST][test-maven] Add LoggingSuite and some improvements Dec 21, 2021

viirya commented Dec 22, 2021

View reviewed changes

viirya mentioned this pull request Dec 23, 2021

[SPARK-6305][BUILD][FOLLOWUP] Remove log4j1 properties files #34941

Closed

LuciferYang reviewed Dec 23, 2021

View reviewed changes

resource-managers/yarn/src/main/scala/org/apache/spark/deploy/yarn/Client.scala Outdated Show resolved Hide resolved

Upload all log4j2 configuration files.

371f27e

srowen reviewed Dec 23, 2021

View reviewed changes

core/src/test/scala/org/apache/spark/SparkFunSuite.scala Outdated Show resolved Hide resolved

viirya changed the title ~~[SPARK-37700][CORE][TEST] Add LoggingSuite and some improvements~~ [SPARK-6305][CORE][TEST][FOLLOWUP] Add LoggingSuite and some improvements Dec 23, 2021

For review comment.

2c233c9

viirya closed this in 7fd3619 Dec 24, 2021

This was referenced Dec 24, 2021

[SPARK-37733][BUILD] Change log level of tests to WARN #35011

Closed

Revert "[SPARK-37733][BUILD] Change log level of tests to WARN" #35014

Closed

[SPARK-6305][CORE][TEST][FOLLOWUP] Add LoggingSuite and some improvements #34965

[SPARK-6305][CORE][TEST][FOLLOWUP] Add LoggingSuite and some improvements #34965

Uh oh!

Conversation

viirya commented Dec 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

SparkQA commented Dec 21, 2021

Uh oh!

SparkQA commented Dec 21, 2021

Uh oh!

This comment has been minimized.

SparkQA commented Dec 21, 2021

Uh oh!

SparkQA commented Dec 21, 2021

Uh oh!

SparkQA commented Dec 21, 2021

Uh oh!

Uh oh!

SparkQA commented Dec 21, 2021

Uh oh!

SparkQA commented Dec 21, 2021

Uh oh!

SparkQA commented Dec 21, 2021

Uh oh!

This comment has been minimized.

SparkQA commented Dec 21, 2021

Uh oh!

SparkQA commented Dec 21, 2021

Uh oh!

SparkQA commented Dec 21, 2021

Uh oh!

viirya commented Dec 21, 2021

Uh oh!

SparkQA commented Dec 21, 2021

Uh oh!

SparkQA commented Dec 21, 2021

Uh oh!

SparkQA commented Dec 21, 2021

Uh oh!

SparkQA commented Dec 21, 2021

Uh oh!

SparkQA commented Dec 21, 2021

Uh oh!

SparkQA commented Dec 22, 2021

Uh oh!

SparkQA commented Dec 22, 2021

Uh oh!

SparkQA commented Dec 22, 2021

Uh oh!

viirya commented Dec 22, 2021

Uh oh!

viirya Dec 22, 2021

Choose a reason for hiding this comment

Uh oh!

viirya Dec 22, 2021

Choose a reason for hiding this comment

Uh oh!

viirya Dec 22, 2021

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Dec 22, 2021

Uh oh!

Uh oh!

SparkQA commented Dec 23, 2021

Uh oh!

SparkQA commented Dec 23, 2021

Uh oh!

SparkQA commented Dec 23, 2021

Uh oh!

srowen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

viirya commented Dec 23, 2021

Uh oh!

viirya commented Dec 21, 2021 •

edited

Loading

viirya commented Dec 23, 2021 •

edited

Loading