-
Notifications
You must be signed in to change notification settings - Fork 28.9k
[SPARK-27460][FOLLOW-UP][TESTS] Fix flaky tests #24434
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Test build #104794 has finished for PR 24434 at commit
|
|
LGTM, I did see some of these flaky tests in #24373 . Increasing the timeout makes the parallel tests more robust. |
|
Test build #4760 has finished for PR 24434 at commit
|
|
Test build #4757 has finished for PR 24434 at commit
|
|
Test build #4759 has finished for PR 24434 at commit
|
|
Test build #4758 has finished for PR 24434 at commit
|
|
Test build #4761 has finished for PR 24434 at commit
|
|
retest this please |
|
Test build #104798 has finished for PR 24434 at commit
|
|
Test build #4762 has finished for PR 24434 at commit
|
|
|
||
| try { | ||
| eventually(timeout(30.seconds), interval(100.milliseconds)) { | ||
| eventually(timeout(180.seconds), interval(100.milliseconds)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can write "3.minutes" in cases like this and others below, but it doesn't matter.
|
Test build #4763 has finished for PR 24434 at commit
|
|
Test build #4764 has finished for PR 24434 at commit
|
|
Test build #4765 has finished for PR 24434 at commit
|
|
Test build #104810 has finished for PR 24434 at commit
|
|
Test build #4767 has finished for PR 24434 at commit
|
|
That's a weird error... it's from javadoc: It's from javadoc'ing the Java code auto-generated from Scala, and this kind of thing is kind of a known problem; I thought we didn't run javadoc on this part. I have no idea so far why it came up in running these tests! |
|
ok to test |
|
If it still fails, let me fix it against this branch since I am kind of used to it. |
|
Test build #104819 has finished for PR 24434 at commit
|
|
Will take a look late today (KST) |
|
Test build #104820 has finished for PR 24434 at commit
|
|
Note that these errors also appear in 'normal' builds; they're just warnings. I still have no idea why they are rendered as errors due to this change. It doesn't touch the build. I could try separately updating sbt-unidoc, but don't think that's the issue. Still looking... |
|
Yes .. I think we talked about this few times long long ago @srowen. I made some analysis in gatorsmile#5 |
|
retest this please |
|
Test build #104853 has finished for PR 24434 at commit
|
|
Test build #104857 has finished for PR 24434 at commit
|
|
retest this please |
|
Test build #104860 has finished for PR 24434 at commit
|
|
thanks, merging to master! |
…rked JVMs for higher parallelism ## What changes were proposed in this pull request? This is a backport of #24373 , #24404 and #24434 This patch modifies SparkBuild so that the largest / slowest test suites (or collections of suites) can run in their own forked JVMs, allowing them to be run in parallel with each other. This opt-in / whitelisting approach allows us to increase parallelism without having to fix a long-tail of flakiness / brittleness issues in tests which aren't performance bottlenecks. See comments in SparkBuild.scala for information on the details, including a summary of why we sometimes opt to run entire groups of tests in a single forked JVM . The time of full new pull request test in Jenkins is reduced by around 53%: before changes: 4hr 40min after changes: 2hr 13min ## How was this patch tested? Unit test Closes #25861 from dongjoon-hyun/SPARK-27460. Lead-authored-by: Gengliang Wang <[email protected]> Co-authored-by: gatorsmile <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>
What changes were proposed in this pull request?
This patch makes several test flakiness fixes.
How was this patch tested?
N/A