[SPARK-20558][CORE] clear InheritableThreadLocal variables in SparkContext when stopping it

[cloud-fan]

What changes were proposed in this pull request?

To better understand this problem, let's take a look at an example first:

object Main {
  def main(args: Array[String]): Unit = {
    var t = new Test
    new Thread(new Runnable {
      override def run() = {}
    }).start()
    println("first thread finished")

    t.a = null
    t = new Test
    new Thread(new Runnable {
      override def run() = {}
    }).start()
  }

}

class Test {
  var a = new InheritableThreadLocal[String] {
    override protected def childValue(parent: String): String = {
      println("parent value is: " + parent)
      parent
    }
  }
  a.set("hello")
}

The result is:

parent value is: hello
first thread finished
parent value is: hello
parent value is: hello

Once an InheritableThreadLocal has been set value, child threads will inherit its value as long as it has not been GCed, so setting the variable which holds the InheritableThreadLocal to null doesn't work as we expected.

In SparkContext, we have an InheritableThreadLocal for local properties, we should clear it when stopping SparkContext, or all the future child threads will still inherit it and copy the properties and waste memory.

This is the root cause of https://issues.apache.org/jira/browse/SPARK-20548 , which creates/stops SparkContext many times and finally have a lot of InheritableThreadLocal alive, and cause OOM when starting new threads in the internal thread pools.

How was this patch tested?

N/A

[cloud-fan]

cc @JoshRosen @rxin @zsxwing

[SparkQA]

Test build #76384 has finished for PR 17833 at commit 310ad75.

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

[sameeragarwal]

LGTM

[zsxwing]

I think it only cleans localProperties in the current thread. localProperties overrides childValue and always clones a new Properties for child threads.

In addition, I think it doesn't fix the flaky REPL tests. Last time I checked the head dump, I observed most of memory usage comes from REPLs and their class loaders referred by SparkContexts. And the GC roots of SparkContexts are JVM internal threads in this thread pool: http://grepcode.com/file/repository.grepcode.com/java/root/jdk/openjdk/8u40-b25/java/lang/UNIXProcess.java#220 That's why we can only observe OOM in REPL tests. It's a cache thread pool. I could not figure out a fix except adding Thread.sleep to wait for these threads being killed automatically.

[zsxwing]

By the way, this PR is good to me since it does reduce a little memory footprint. But we still cannot close https://issues.apache.org/jira/browse/SPARK-20548 though.

[zsxwing]

In a second thought, for https://issues.apache.org/jira/browse/SPARK-20548 , we can just combine some tests into one test to reduce the number of SparkContexts and REPLs.

[cloud-fan]

I think it only cleans localProperties in the current thread. localProperties overrides childValue and always clones a new Properties for child threads.

Yea, that's true. If some child threads are already there and cloned the local properties, we can't clean them. But we can avoid future child threads to inherit this local properties, which can reduce the memory footprint a lot if users create new SparkContext and stop it, and repeat this many times.

Anyway, I'll merge this PR and see if it can fix the flaky test.

[cloud-fan]

merging to master/2.2/2.1/2.0

[sameeragarwal]

Are we still investigating the root cause? If not, perhaps try re-enabling the test in 2.2 to see if it's failing?