[SPARK-35595][TESTS] Support multiple loggers in testing method withLogAppender #32725
Conversation
gengliangwang
changed the title
HyukjinKwon
changed the title
sql/core/src/test/scala/org/apache/spark/sql/execution/adaptive/AdaptiveQueryExecSuite.scala
Outdated
Show resolved
Hide resolved
|
Kubernetes integration test starting |
|
Kubernetes integration test status success |
|
Test build #139137 has finished for PR 32725 at commit
|
srowen
left a comment
srowen
left a comment
There was a problem hiding this comment.
I noticed AQE tests timing out after stalling for hours in another PR - could be related
| val restoreLevels = loggers.map(_.getLevel) | ||
| loggers.foreach { logger => | ||
| logger.addAppender(appender) | ||
| if (level.isDefined) { |
There was a problem hiding this comment.
This could be level.foreach in both places, but doesn't matter
gengliangwang
changed the title
Yes the logger appending can be slow |
|
@HyukjinKwon @cloud-fan @MaxGekk @zhongyu09 I use a better approach and update the PR. Please take a look if you are interested. |
|
This is a good improvement! Do we still need to set the size to |
Do you mean the default 1000 limit? |
|
Kubernetes integration test starting |
|
Kubernetes integration test status success |
|
Test build #139161 has finished for PR 32725 at commit
|
sql/core/src/test/scala/org/apache/spark/sql/execution/adaptive/AdaptiveQueryExecSuite.scala
Outdated
Show resolved
Hide resolved
|
Kubernetes integration test unable to build dist. exiting with code: 1 |
|
Test build #139178 has finished for PR 32725 at commit
|
|
Thanks for the reviews, merging to master |
7 participants
What changes were proposed in this pull request?
A test case of AdaptiveQueryExecSuite becomes flaky since there are too many debug logs in RootLogger:
https://github.com/Yikun/spark/runs/2715222392?check_suite_focus=true
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/139125/testReport/
To fix it, I suggest supporting multiple loggers in the testing method withLogAppender. So that the LogAppender gets clean target log outputs.
Why are the changes needed?
Fix a flaky test case.
Also, reduce unnecessary memory cost in tests.
Does this PR introduce any user-facing change?
No
How was this patch tested?
Unit test