Skip to content
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

[SPARK-37293][TESTS] Remove explicit GC options from Scala tests #34560

Closed
wants to merge 1 commit into from
Closed

[SPARK-37293][TESTS] Remove explicit GC options from Scala tests #34560

wants to merge 1 commit into from

Conversation

dongjoon-hyun
Copy link
Member

@dongjoon-hyun dongjoon-hyun commented Nov 11, 2021

What changes were proposed in this pull request?

This PR aims to remove the explicit GC options in Scala tests.

Why are the changes needed?

At Apache Spark 3.0, SPARK-29282 introduced the explicit GC options in Scala tests to see the consistent result between Java8/Java11. Now, we have Java 17 additionally and SPARK-37120 added Java 11/17 GitHub Action jobs.

This PR aims to use the default GC options for each JVM. This is a partial revert of SPARK-29282.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Pass the GitHub Action CIs without memory issues.

@github-actions github-actions bot added the BUILD label Nov 11, 2021
@dongjoon-hyun
Copy link
Member Author

dongjoon-hyun commented Nov 11, 2021

Hi, @HyukjinKwon and @LuciferYang .
The daily Java11 and 17 runs seems to work, but the only exception looks like the known the memory issue.
So I hope this try can mitigate the situation at least the following warning.

[warn] In the last 3393 seconds, 6.757 (0.2%) were spent in GC. [Heap: 3.49GB free of 4.00GB, max 4.00GB]
Consider increasing the JVM heap using `-Xmx` or try a different collector, e.g. `-XX:+UseG1GC`, for better performance.

@SparkQA
Copy link

SparkQA commented Nov 11, 2021

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

@SparkQA
Copy link

SparkQA commented Nov 11, 2021

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

@SparkQA
Copy link

SparkQA commented Nov 11, 2021

Test build #145117 has finished for PR 34560 at commit a3103a2.

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

@dongjoon-hyun
Copy link
Member Author

Could you review this, @mridulm ?

Copy link
Member

@HyukjinKwon HyukjinKwon left a comment

Choose a reason for hiding this comment

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

My only concern is the tests being flaky - the current GA builds are flaky because of hitting memory issues. I vaguely remember that using UseParallelGC arguably less reaches the memory limit (I did take a quick look before).

I am going to approve this but we might have to monitor the tests for the time being..

Copy link
Contributor

@LuciferYang LuciferYang left a comment

Choose a reason for hiding this comment

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

LGTM +1

@dongjoon-hyun
Copy link
Member Author

dongjoon-hyun commented Nov 12, 2021

Thanks. Yes, GA jobs are flaky due to the memory limits and I'll monitor this carefully.

Note that this will affect only Java 11/Java 17 which means

  • Java11 build job (without tests)
  • Java17 build job (without tests)
  • Daily Java11/17 build and test jobs which is added recently.

The other jobs are running Java 8 whose default GC is ParallelGC. In addition, the flakiness mainly occurs at the build&test jobs.

Especially, this PR aims to stablize the daily Java11 and Java17 jobs. I'll keep improving those test coverage for Spark 3.3..

@dongjoon-hyun
Copy link
Member Author

Thank you, @LuciferYang , too. Merged to master.

@dongjoon-hyun dongjoon-hyun deleted the SPARK-37293 branch November 12, 2021 02:50
sunchao pushed a commit to sunchao/spark that referenced this pull request Dec 8, 2021
This PR aims to remove the explicit GC options in Scala tests.

At Apache Spark 3.0, SPARK-29282 introduced the explicit GC options in Scala tests to see the consistent result between Java8/Java11. Now, we have Java 17 additionally and SPARK-37120 added Java 11/17 GitHub Action jobs.

This PR aims to use the default GC options for each JVM. This is a partial revert of SPARK-29282.

No.

Pass the GitHub Action CIs without memory issues.

Closes apache#34560 from dongjoon-hyun/SPARK-37293.

Authored-by: Dongjoon Hyun <dongjoon@apache.org>
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
(cherry picked from commit 5b002c8)
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants