Skip to content

Conversation

@zsxwing
Copy link
Member

@zsxwing zsxwing commented Feb 6, 2017

What changes were proposed in this pull request?

Signaling.cancelOnInterrupt leaks a SparkContext per call and it makes ReplSuite unstable.

This PR adds SparkContext.getActive to allow Signaling.cancelOnInterrupt to get the active SparkContext to avoid the leak.

How was this patch tested?

Jenkins

@zsxwing zsxwing changed the title Avoid leak SparkContext in Signaling.cancelOnInterrupt [SPARK-19481][REPL]Avoid to leak SparkContext in Signaling.cancelOnInterrupt Feb 6, 2017
@zsxwing zsxwing changed the title [SPARK-19481][REPL]Avoid to leak SparkContext in Signaling.cancelOnInterrupt [SPARK-19481][REPL][maven]Avoid to leak SparkContext in Signaling.cancelOnInterrupt Feb 6, 2017
@SparkQA
Copy link

SparkQA commented Feb 7, 2017

Test build #72472 has finished for PR 16825 at commit 3554e33.

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

@zsxwing
Copy link
Member Author

zsxwing commented Feb 7, 2017

cc @davies

@zsxwing
Copy link
Member Author

zsxwing commented Feb 8, 2017

This PR doesn't fix all leaks though. I noticed that there are many Finalizers and it slows down GC. Most of them are JarFile and Inflater.

} else {
false
}
def cancelOnInterrupt(): Unit = SignalUtils.register("INT") {
Copy link
Contributor

Choose a reason for hiding this comment

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

Who is using this one? Is this a breaking change?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's used by REPL to cancel the running job if any.

@davies
Copy link
Contributor

davies commented Feb 9, 2017

lgtm, merging this into master and 2.1 branch, thanks

@asfgit asfgit closed this in 303f00a Feb 9, 2017
asfgit pushed a commit that referenced this pull request Feb 9, 2017
…cancelOnInterrupt

## What changes were proposed in this pull request?

`Signaling.cancelOnInterrupt` leaks a SparkContext per call and it makes ReplSuite unstable.

This PR adds `SparkContext.getActive` to allow `Signaling.cancelOnInterrupt` to get the active `SparkContext` to avoid the leak.

## How was this patch tested?

Jenkins

Author: Shixiong Zhu <[email protected]>

Closes #16825 from zsxwing/SPARK-19481.

(cherry picked from commit 303f00a)
Signed-off-by: Davies Liu <[email protected]>
@zsxwing zsxwing deleted the SPARK-19481 branch February 9, 2017 19:30
cmonkey pushed a commit to cmonkey/spark that referenced this pull request Feb 15, 2017
…cancelOnInterrupt

## What changes were proposed in this pull request?

`Signaling.cancelOnInterrupt` leaks a SparkContext per call and it makes ReplSuite unstable.

This PR adds `SparkContext.getActive` to allow `Signaling.cancelOnInterrupt` to get the active `SparkContext` to avoid the leak.

## How was this patch tested?

Jenkins

Author: Shixiong Zhu <[email protected]>

Closes apache#16825 from zsxwing/SPARK-19481.
asfgit pushed a commit that referenced this pull request Mar 8, 2017
…cancelOnInterrupt

## What changes were proposed in this pull request?

`Signaling.cancelOnInterrupt` leaks a SparkContext per call and it makes ReplSuite unstable.

This PR adds `SparkContext.getActive` to allow `Signaling.cancelOnInterrupt` to get the active `SparkContext` to avoid the leak.

## How was this patch tested?

Jenkins

Author: Shixiong Zhu <[email protected]>

Closes #16825 from zsxwing/SPARK-19481.
@zsxwing
Copy link
Member Author

zsxwing commented Mar 8, 2017

Also cherry-picked to branch 2.0 as I saw ReplSuite failed several times on branch-2.0.

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.

3 participants