Skip to content

Conversation

@sander-goos
Copy link
Contributor

Backport of: #30783

What changes were proposed in this pull request?

This PR introduces a helper method withExecutor that handles the creation of an Executor object and ensures that it is always stopped in a finally block. The tests in ExecutorSuite have been refactored to use this method.

Why are the changes needed?

Recently an issue was discovered that leaked Executors (which are not explicitly stopped after a test) can cause other tests to fail due to the JVM being killed after 10 min. It is therefore crucial that tests always stop the Executor. By introducing this helper method, a simple pattern is established that can be easily adopted in new tests, which reduces the risk of regressions.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Run the ExecutorSuite locally.

…ng sure they are always stopped after the test.
@HyukjinKwon
Copy link
Member

ok to test

@HyukjinKwon HyukjinKwon changed the title [SPARK-33793][TESTS] Introduce withExecutor to ensure proper cleanup in tests [SPARK-33793][TESTS][3.0] Introduce withExecutor to ensure proper cleanup in tests Dec 16, 2020
@SparkQA
Copy link

SparkQA commented Dec 16, 2020

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

@SparkQA
Copy link

SparkQA commented Dec 16, 2020

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

HyukjinKwon pushed a commit that referenced this pull request Dec 16, 2020
…anup in tests

Backport of: #30783

### What changes were proposed in this pull request?
This PR introduces a helper method `withExecutor` that handles the creation of an Executor object and ensures that it is always stopped in a finally block. The tests in ExecutorSuite have been refactored to use this method.

### Why are the changes needed?
Recently an issue was discovered that leaked Executors (which are not explicitly stopped after a test) can cause other tests to fail due to the JVM being killed after 10 min. It is therefore crucial that tests always stop the Executor. By introducing this helper method, a simple pattern is established that can be easily adopted in new tests, which reduces the risk of regressions.

### Does this PR introduce _any_ user-facing change?
No

### How was this patch tested?
Run the ExecutorSuite locally.

Closes #30801 from sander-goos/SPARK-33793-close-executors-3.0.

Authored-by: Sander Goos <[email protected]>
Signed-off-by: HyukjinKwon <[email protected]>
@HyukjinKwon
Copy link
Member

Merged to branch-3.0.

@SparkQA
Copy link

SparkQA commented Dec 16, 2020

Test build #132885 has finished for PR 30801 at commit 8e04da0.

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

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