[SPARK-16139][TEST] Add logging functionality for leaked threads in tests #19893

gaborgsomogyi · 2017-12-05T11:28:49Z

What changes were proposed in this pull request?

Lots of our tests don't properly shutdown everything they create, and end up leaking lots of threads. For example, TaskSetManagerSuite doesn't stop the extra TaskScheduler and DAGScheduler it creates. There are a couple more instances, eg. in DAGSchedulerSuite.

This PR adds the possibility to print out the not properly stopped thread list after a test suite executed. The format is the following:

===== FINISHED o.a.s.scheduler.DAGSchedulerSuite: 'task end event should have updated accumulators (SPARK-20342)' =====

...

===== Global thread whitelist loaded with name /thread_whitelist from classpath: rpc-client.*, rpc-server.*, shuffle-client.*, shuffle-server.*' =====

ScalaTest-run: 

===== THREADS NOT STOPPED PROPERLY =====

ScalaTest-run: dag-scheduler-event-loop
ScalaTest-run: globalEventExecutor-2-5
ScalaTest-run: 

===== END OF THREAD DUMP =====

ScalaTest-run: 

===== EITHER PUT THREAD NAME INTO THE WHITELIST FILE OR SHUT IT DOWN PROPERLY =====

With the help of this leaking threads has been identified in TaskSetManagerSuite. My intention is to hunt down and fix such bugs in later PRs.

How was this patch tested?

Manual: TaskSetManagerSuite test executed and found out where are the leaking threads.
Automated: Pass the Jenkins.

…ests

gaborgsomogyi · 2017-12-05T12:12:08Z

cc @squito @srowen @HyukjinKwon

smurakozi · 2017-12-05T13:17:42Z

Logging the leaked threads in a more grep friendly format would be nice, you could easily create a thread leak report.
It would be also nice to see the leaks on the console.

gaborgsomogyi · 2017-12-05T13:24:37Z

Good point, I've also struggled to collect all actual problems from Jenkins build :)
Format changed to the following:

===== FINISHED o.a.s.scheduler.DAGSchedulerSuite: 'task end event should have updated accumulators (SPARK-20342)' =====

...

ScalaTest-run: 

===== Global thread whitelist loaded with name /thread_whitelist from classpath: rpc-client.*, rpc-server.*, shuffle-client.*, shuffle-server.*' =====

ScalaTest-run: 

===== POSSIBLE THREAD LEAK IN SUITE o.a.s.scheduler.DAGSchedulerSuite, thread names: dag-scheduler-event-loop, globalEventExecutor-2-6 =====

srowen · 2017-12-05T14:17:53Z

core/src/test/resources/thread_whitelist

+# Each line contains a new regex string which will be evaluated with matches
+# Empty lines or starting with # will me skipped
+
+rpc-client.*


If this is just for testing support, I personally think there's no need to create a config file and read it. Hard-coding filtering rules may be just fine. Neutral on this.

Agree with Sean here - there's not a really obvious use case for having this independent of the class where it's used. Putting it into the code means that the whitelist feature is self-documenting, and you don't have to go through any indirection to find this file.

Plus I think moving this into SparkFunSuite means you can get rid of the file loading logic in 'object SparkFunSuite'.

srowen · 2017-12-05T14:18:26Z

core/src/test/scala/org/apache/spark/scheduler/TaskSetManagerSuite.scala

@@ -683,7 +683,7 @@ class TaskSetManagerSuite extends SparkFunSuite with LocalSparkContext with Logg
    val conf = new SparkConf().set("spark.speculation", "true")
    sc = new SparkContext("local", "test", conf)

-    val sched = new FakeTaskScheduler(sc, ("execA", "host1"), ("execB", "host2"))
+    sched = new FakeTaskScheduler(sc, ("execA", "host1"), ("execB", "host2"))


What was this change about? not shadowing?

Here originally the newly created instance was stored in a local variable which was never saved in member and freed properly. With this change the afterEach method stops it and frees up the resources.

gaborgsomogyi · 2017-12-06T10:26:50Z

I have gathered statistics manually about the actual stand. I've grep-ed unit-tests.log in the whole build:

bash-3.2$ find . -type f | xargs grep "POSSIBLE THREAD LEAK" | wc -l
370

srowen · 2017-12-06T12:50:16Z

Do any of those leaked threads look like they might be real issues to fix? you could paste the results here, minus anything you know isn't a problem.

gaborgsomogyi · 2017-12-06T13:53:20Z

I've just started to take a look at it deeper and found some patterns. Namely we can exclude all netty.* threads + ForkJoinPool.* is most of the time but not always created inside scala by the global ExecutionContext. All in all far from have a good picture but I'll exclude these entries.

gaborgsomogyi · 2017-12-06T13:58:16Z

On the other side globalEventExecutor.* and dag-scheduler-event-loop was an issue in the tests what I've taken a look at.

gaborgsomogyi · 2017-12-06T14:05:15Z

Here is a list but it definitely contains false positives.

SPARK-16139.txt

SparkQA · 2017-12-06T18:57:09Z

Test build #4006 has finished for PR 19893 at commit 0d45a5b.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

gaborgsomogyi · 2017-12-06T19:05:36Z

I've taken a look at the failed test but seems like unrelated.

henryr · 2017-12-06T20:55:17Z

core/src/test/scala/org/apache/spark/SparkFunSuite.scala

@@ -52,6 +62,23 @@ abstract class SparkFunSuite
    getTestResourceFile(file).getCanonicalPath
  }

+  private def saveThreadNames(): Unit = {


Suggest turning this into runningThreadNames(): Set[String], and then you can use this method both in beforeAll() and in printRemainingThreadNames() (line 70). And you can maybe put the whitelist logic here as well.

Config file removed and refactored as you suggested. It's much more simple now, thanks :)

henryr · 2017-12-06T20:56:06Z

core/src/test/scala/org/apache/spark/SparkFunSuite.scala

+
+  private def printRemainingThreadNames(): Unit = {
+    val currentThreadNames = Thread.getAllStackTraces.keySet().map(_.getName).toSet
+    val whitelistedThreadNames = currentThreadNames.


nit: '.' goes on next line

henryr · 2017-12-06T20:59:58Z

core/src/test/resources/thread_whitelist

+# Each line contains a new regex string which will be evaluated with matches
+# Empty lines or starting with # will me skipped
+
+rpc-client.*


Agree with Sean here - there's not a really obvious use case for having this independent of the class where it's used. Putting it into the code means that the whitelist feature is self-documenting, and you don't have to go through any indirection to find this file.

Plus I think moving this into SparkFunSuite means you can get rid of the file loading logic in 'object SparkFunSuite'.

vanzin · 2017-12-06T21:27:32Z

core/src/test/scala/org/apache/spark/SparkFunSuite.scala

+  private def printRemainingThreadNames(): Unit = {
+    val currentThreadNames = Thread.getAllStackTraces.keySet().map(_.getName).toSet
+    val whitelistedThreadNames = currentThreadNames.
+      filterNot(s => SparkFunSuite.threadWhiteList.exists(s.matches(_)))


style: .filterNot { s =>

Style applied.

vanzin · 2017-12-06T21:27:51Z

core/src/test/scala/org/apache/spark/SparkFunSuite.scala

+    val whitelistedThreadNames = currentThreadNames.
+      filterNot(s => SparkFunSuite.threadWhiteList.exists(s.matches(_)))
+    val remainingThreadNames = whitelistedThreadNames.diff(beforeAllTestThreadNames)
+    if (!remainingThreadNames.isEmpty) {


remainingThreadNames.nonEmpty

vanzin · 2017-12-06T21:28:29Z

core/src/test/scala/org/apache/spark/SparkFunSuite.scala

@@ -72,3 +99,27 @@ abstract class SparkFunSuite
  }

 }
+
+object SparkFunSuite
+  extends Logging {


move to previous line

Object removed due to previous review items.

gaborgsomogyi · 2017-12-06T21:53:47Z

In the meantime analysed a couple of cases and found netty related threads:

netty.*
globalEventExecutor.*
threadDeathWatcher.*

I've added them to the whitelist.

vanzin · 2017-12-06T21:56:25Z

core/src/test/scala/org/apache/spark/SparkFunSuite.scala

 import org.apache.spark.internal.Logging
 import org.apache.spark.util.AccumulatorContext
+import org.scalatest.{BeforeAndAfterAll, FunSuite, Outcome}
+
+import scala.collection.JavaConversions._


Wonder why the style checker didn't complain, but scala.* imports should be in the previous position.

I've executed reorganize imports. Shouldn't solve this such problems?

I don't know what that is.

The import order is described in http://spark.apache.org/contributing.html, section "Imports".

Thanks for the guidance. I've set up the intellij imports organizer as described and fixed with it.

vanzin · 2017-12-06T21:57:19Z

core/src/test/scala/org/apache/spark/SparkFunSuite.scala

@@ -34,12 +36,24 @@ abstract class SparkFunSuite
  with Logging {
 // scalastyle:on

+  val threadWhiteList = Set(
+    "rpc-client.*", "rpc-server.*", "shuffle-client.*", "shuffle-server.*",


It would be nice to add comments explaining why the threads are whitelisted. Without an explanation to the contrary, I don't think any of these should be whitelisted.

For the new netty related patterns added documentation. Could somebody help me out with rpc and shuffle? All I can see for example TaskSetManagerSuite.test("TaskSet with no preferences") creates a lot of them and I don't see any test issue.

Temporarily removed rpc and shuffle. I'll put them back when proper doc can be written.

I've made deepdive what these threads are and put documentation for each. I'll execute a build with them and let's see the new numbers.

vanzin · 2017-12-06T22:35:25Z

ok to test

henryr · 2017-12-06T22:43:56Z

core/src/test/scala/org/apache/spark/SparkFunSuite.scala

+    val currentThreadNames = runningThreadNames()
+    val whitelistedThreadNames = currentThreadNames
+      .filterNot { s => threadWhiteList.exists(s.matches(_)) }
+    val remainingThreadNames = whitelistedThreadNames.diff(beforeAllTestThreadNames)


nit: I think this would be better written as:

val remainingThreadNames = runningThreadNames.diff(beforeAllTestThreadNames).filterNot { s => threadWhiteList.exists(s.matches(_)) }

(although putting the whitelist filtering into runningThreadNames() would still make this more concise).

The reason is that it's not obvious to the reader why you whitelist 'after' threads but not 'before' - clearer to whitelist the diff.

Compressed.

SparkQA · 2017-12-06T22:59:39Z

Test build #84577 has finished for PR 19893 at commit a35a52f.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2017-12-07T01:55:02Z

Test build #84574 has finished for PR 19893 at commit 1a64209.

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

SparkQA · 2017-12-07T02:38:09Z

Test build #84580 has finished for PR 19893 at commit fe6cd0c.

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

SparkQA · 2017-12-07T15:51:53Z

Test build #84606 has finished for PR 19893 at commit 62cb32b.

This patch fails to generate documentation.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2017-12-07T16:07:59Z

Test build #84601 has finished for PR 19893 at commit 2b02d45.

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

gaborgsomogyi · 2017-12-19T17:47:50Z

@squito I mean another jira, because it needs deeper analysis and discussion.

gaborgsomogyi · 2017-12-21T22:30:00Z

gentle ping @jiangxb1987

SparkQA · 2017-12-22T04:11:16Z

Test build #85290 has finished for PR 19893 at commit ef00796.

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

jiangxb1987

LGTM only some nits. also cc @cloud-fan

jiangxb1987 · 2017-12-22T15:44:01Z

core/src/test/scala/org/apache/spark/ThreadAudit.scala

+  }
+
+  private def printRemainingThreadNames(): Unit = {
+    val suiteName = this.getClass.getName


nit:

val shortSuiteName = this.getClass.getName.replaceAll("org.apache.spark", "o.a.s")

jiangxb1987 · 2017-12-22T15:45:23Z

core/src/test/scala/org/apache/spark/ThreadAudit.scala

+          s"thread names: ${remainingThreadNames.mkString(", ")} =====\n")
+      }
+    } else {
+      logWarning(s"\n\n===== THREAD AUDIT POST ACTION CALLED " +


nit: remove 's' before the string.

jiangxb1987 · 2017-12-22T15:49:58Z

sql/core/src/test/scala/org/apache/spark/sql/test/SharedSQLContext.scala

+
+  protected override def beforeAll(): Unit = {
+    doThreadPreAudit
+    super.beforeAll


nit: super.beforeAll(), and also super.afterAll().

SparkQA · 2017-12-22T20:50:36Z

Test build #85315 has finished for PR 19893 at commit f7939fa.

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

vanzin

Most of the places where you're overriding doThreadAuditInSparkFunSuite, it seems like the code is just not correct, and that you can just fix it instead of overriding that behavior.

vanzin · 2018-01-03T20:46:10Z

core/src/test/scala/org/apache/spark/SparkFunSuite.scala

  with Logging {
 // scalastyle:on

+  protected val doThreadAuditInSparkFunSuite = true


Can we call this just doThreadAudit or enableThreadAudit?

Given the way this is being used elsewhere, a better name is probably enableAutoThreadAudit. or something.

I was thinking about proper naming before. The last suggested one is definitely better. No exact place where it happens but not suggesting that it's completely turned off.

Renamed to enableAutoThreadAudit.

vanzin · 2018-01-03T20:47:52Z

core/src/test/scala/org/apache/spark/ThreadAudit.scala

+/**
+ * Thread audit for test suites.
+ *
+ * Thread audit happens normally in [[SparkFunSuite]] automatically when a new test suite created.


You shouldn't describe the behavior of SparkFunSuite here. You should instead document the flag in SparkFunSuite that controls whether this code is triggered.

All the comments in the rest of this class are related to SparkFunSuite and overriding its default behavior, so they're better placed in SparkFunSuite and not here too.

Good point, moving.

I'd just remove this paragraph since this class is independent of SparkFunSuite.

vanzin · 2018-01-03T20:51:32Z

core/src/test/scala/org/apache/spark/ThreadAudit.scala

+
+    /**
+     * During [[SparkContext]] creation BlockManager
+     * creates event loops. One is wrapped inside


nit: line wrapped too early.

vanzin · 2018-01-03T20:52:16Z

core/src/test/scala/org/apache/spark/ThreadAudit.scala

+  protected def doThreadPostAudit(): Unit = printRemainingThreadNames
+
+  private def snapshotRunningThreadNames(): Unit = {
+    threadNamesSnapshot = runningThreadNames


nit: call with () if you declare the method with ().

vanzin · 2018-01-03T20:52:59Z

core/src/test/scala/org/apache/spark/ThreadAudit.scala

+  protected def doThreadPreAudit(): Unit = snapshotRunningThreadNames
+  protected def doThreadPostAudit(): Unit = printRemainingThreadNames
+
+  private def snapshotRunningThreadNames(): Unit = {


Can't you just inline this in doThreadPreAudit since it's the only call site and this is a private method?

vanzin · 2018-01-03T20:53:23Z

core/src/test/scala/org/apache/spark/ThreadAudit.scala

+    threadNamesSnapshot = runningThreadNames
+  }
+
+  private def printRemainingThreadNames(): Unit = {


Same reasoning as above. Just inline.

vanzin · 2018-01-03T21:06:07Z

sql/core/src/test/scala/org/apache/spark/sql/SessionStateSuite.scala

@@ -39,6 +41,7 @@ class SessionStateSuite extends SparkFunSuite
  protected var activeSession: SparkSession = _

  override def beforeAll(): Unit = {
+    doThreadPreAudit()


Isn't the problem here that this is not calling super.beforeAll()? If you do that, you don't need to override doThreadAuditInSparkFunSuite nor call doThreadPostAudit below.

Fixed. My intention was to change the least in the tests behaviour. This case doesn't matter.

vanzin · 2018-01-03T21:06:33Z

sql/core/src/test/scala/org/apache/spark/sql/sources/DataSourceAnalysisSuite.scala

  private var targetAttributes: Seq[Attribute] = _
  private var targetPartitionSchema: StructType = _

  override def beforeAll(): Unit = {
+    doThreadPreAudit()


Same thing here. This should be calling super.beforeAll().

vanzin · 2018-01-03T21:08:01Z

sql/core/src/test/scala/org/apache/spark/sql/test/SharedSQLContext.scala

+  override protected val doThreadAuditInSparkFunSuite = false
+
+  protected override def beforeAll(): Unit = {
+    doThreadPreAudit()


This looks like the same situation, but because this is a trait, it kinda relies on the suites to call beforeAll and afterAll correctly... if you don't want to audit all suites, you could write a comment explaining the situation.

It's kind of similar but not the same. Comment added.

vanzin · 2018-01-03T21:08:23Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveSessionStateSuite.scala

  override def beforeAll(): Unit = {
    // Reuse the singleton session
    activeSession = spark
+    doThreadPreAudit()


Same thing. Just call super.beforeAll() correctly.

SparkQA · 2018-01-05T15:02:53Z

Test build #85722 has finished for PR 19893 at commit 0851ef2.

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

vanzin · 2018-01-05T19:01:22Z

core/src/test/scala/org/apache/spark/SparkFunSuite.scala

+ *
+ * class MyTestSuite extends SparkFunSuite {
+ *
+ *   override val doThreadAuditInSparkFunSuite = false


enableAutoThreadAudit now

vanzin · 2018-01-05T19:02:39Z

core/src/test/scala/org/apache/spark/SparkFunSuite.scala

+ * Thread audit happens normally here automatically when a new test suite created.
+ * The only prerequisite for that is that the test class must extend [[SparkFunSuite]].
+ *
+ * There are some test suites which are doing initialization before [[SparkFunSuite#beforeAll]]


Better:

"
It is possible to override the default thread audit behavior by setting enableAutoThreadAudit to false and manually calling the audit methods, if desired. For example:

// Code
"

vanzin · 2018-01-05T19:04:14Z

core/src/test/scala/org/apache/spark/ThreadAudit.scala

+/**
+ * Thread audit for test suites.
+ *
+ * Thread audit happens normally in [[SparkFunSuite]] automatically when a new test suite created.


I'd just remove this paragraph since this class is independent of SparkFunSuite.

vanzin · 2018-01-05T19:07:15Z

sql/core/src/test/scala/org/apache/spark/sql/test/SharedSQLContext.scala

+trait SharedSQLContext extends SQLTestUtils with SharedSparkSession {
+
+  /**
+   * Auto thread audit is turned off here intentionally and done manually.


I'm still a little not convinced that this is needed.

I still think that any reported leaks here are caused by bugs in the test suites and not because of this. The code you have here is basically the same thing as SparkFunSuite.

For example, if a suite extending this does not call super.beforeAll() but calls super.afterAll(), won't you get false positives in the output?

Same thing but it meant to solve different problem (changes the execution order). Please see the execution order with and without this change described in my previous post:

As a next step analysed SQL test flow. Here are the steps: 1. SharedSparkSession.beforeAll called which initialise SparkSession and SQLContext 2. SparkFunSuite.beforeAll creates a thread snapshot 3. Test code runs 4. SparkFunSuite.afterAll prints out the possible leaks 5. SharedSparkSession.afterAll stops SparkSession Not sure if I understand right but this will not report false positives. The only problem what I see here as it's not gonna report SparkSession and SQLContext related leaks. As you mentioned before this code should find SparkContext related threading issues which applies here as well. This is not fulfilled at the moment and my proposal is to fix it this way: 1. SparkFunSuite.beforeAll creates a thread snapshot 2. SharedSparkSession.beforeAll called which initialise SparkSession and SQLContext 3. Test code runs 4. SharedSparkSession.afterAll stops SparkSession 5. SparkFunSuite.afterAll prints out the possible leaks With this change I don't see any false positives and missed threads. Please share your ideas related this topic.

Your concern is fully standing but this change is not intended to cover the mentioned issue. The problem you mentioned is addressed in ThreadAudit, namely it prints out the following message such cases:

THREAD AUDIT POST ACTION CALLED WITHOUT PRE ACTION IN SUITE...

I'm not sure I understand your explanation, and I definitely don't understand what's going on from the comment in the code. What I'm asking is for the comment here to explain not what the code is doing, but why it's doing it.

Basically, if instead of the code you have here, you just called super.beforeAll and super.afterAll, without disabling enableAutoThreadAudit, what will break and why? That's what the comment should explain.

Yeah, now I see your point. Description changed.

SparkQA · 2018-01-08T18:22:46Z

Test build #85802 has finished for PR 19893 at commit 87c4852.

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

SparkQA · 2018-01-12T17:13:48Z

Test build #86035 has finished for PR 19893 at commit 9c9c6ef.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

gaborgsomogyi · 2018-01-12T17:49:55Z

Checked the failure but seems like unrelated.

jiangxb1987 · 2018-01-12T18:11:41Z

retest this please

vanzin

Have you had a chance to look at the hive tests? There's a whole bunch of reported thread leaks there. Hive tests behave differently from all of the others in that they share a spark session across suites, not just within a suite.

Examples of reported thread leaks:

broadcast-exchange-441
block-manager-ask-thread-pool-22
ForkJoinPool-289-worker-13
HMSHandler Deprecated and added a few java api methods for corresponding scala api. #19

And a whole bunch of others.

vanzin · 2018-01-12T18:24:45Z

sql/core/src/test/scala/org/apache/spark/sql/test/SharedSQLContext.scala

+
+  /**
+   * Suites extending [[SharedSQLContext]] are sharing resources (eg. SparkSession) in their tests.
+   * Such resources are initialized by the suite before thread audit takes thread snapshot and


Sorry but this still does not explain why this is happening. It's just stating that it is.

For example, in SharedSparkSession, there is this code:

protected override def afterAll(): Unit = { super.afterAll() if (_spark != null) { _spark.sessionState.catalog.reset() _spark.stop() _spark = null } }

If you move super.afterAll() to after the session is stopped, won't that solve the problem and avoid this?

Your suggestion solves one part of the problem. The other one lies here:

protected override def beforeAll(): Unit = { initializeSession() // Ensure we have initialized the context before calling parent code super.beforeAll() }

Session initialized before thread snapshot. This should also happen in the opposite order. Because I've seen the comment in the code I decided not to change it.

Why is the latter a problem? At worst you'll have less threads after the suite finishes than when it started, which should be fine, no? The problem is having leaked threads, not the other way around.

Ok, I think I see your point. Still, the comment here is confusing. Can't this be done in SharedSparkSession instead, where that initialization happens, so that it's clear what it's talking about?

I think this is an easier to understand comment about what's going on here:

/** * Suites extending [[SharedSQLContext]] are sharing resources (eg. SparkSession) in their tests. * That trait initializes the spark session in its [[beforeAll()]] implementation before the * automatic thread snapshot is performed, so the audit code could fail to report threads leaked * by that shared session. * * The behavior is overridden here to take the snapshot before the spark session is initialized. */

Sorry for the noise.

Much better phrased and compressed explanation, applied. Agree that it would be better to move this functionality into SharedSparkSession on the other hand it would lead far in terms of number of modifications. SharedSparkSession has to extend has to extend SparkFunSuite which I don't see it worth the effort. The other option what I see also doesn't help in terms of understanding. Namely moving manual thread audit into SparkFunSuite and leaving enableAutoThreadAudit = false in SharedSQLContext but splitting functionality rarely can help. Ideas?

gaborgsomogyi · 2018-01-12T19:27:39Z

Related hive please see my comment on 11 Dec 2017.

vanzin · 2018-01-12T20:23:52Z

Why not disable the thread audit in the hive module? You added that functionality already, should be pretty trivial to use it.

SparkQA · 2018-01-12T22:07:53Z

Test build #86050 has finished for PR 19893 at commit 9c9c6ef.

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

…ancement

gaborgsomogyi · 2018-01-13T11:14:43Z

Thread audit disabled in hive.

SparkQA · 2018-01-13T14:32:10Z

Test build #86093 has finished for PR 19893 at commit 56a41df.

This patch fails Spark unit tests.
This patch does not merge cleanly.
This patch adds the following public classes (experimental):
* That trait initializes the spark session in its [[beforeAll()]] implementation before the

SparkQA · 2018-01-13T15:20:18Z

Test build #86094 has finished for PR 19893 at commit 68d0f3b.

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

vanzin · 2018-01-16T19:40:31Z

Merging to master.

It would be nice to file a separate bug to eventually look at how to do this on the spark-hive module (or maybe it's just not worth the effort).

gaborgsomogyi · 2018-01-17T13:26:27Z

@vanzin @squito @srowen @jiangxb1987 @henryr
Big thanks to everybody for the constructive comments, learned a lot from them.
I'll take a look at further possibilities like the suggested spark-hive module.

gaborgsomogyi added 2 commits December 5, 2017 09:52

[SPARK-16139][TEST] Add logging functionality for leaked threads in t…

9acbb62

…ests

Apache license header added

9058603

Review fix

0d45a5b

srowen reviewed Dec 5, 2017

View reviewed changes

henryr reviewed Dec 6, 2017

View reviewed changes

vanzin reviewed Dec 6, 2017

View reviewed changes

Review fixes

1a64209

vanzin reviewed Dec 6, 2017

View reviewed changes

henryr reviewed Dec 6, 2017

View reviewed changes

Review fixes

a35a52f

Doc indentation fix

fe6cd0c

gaborgsomogyi added 2 commits December 7, 2017 13:46

Review fix

2b02d45

Documentation added for whitelisted threads

62cb32b

jiangxb1987 reviewed Dec 22, 2017

View reviewed changes

Fixing nits

f7939fa

vanzin reviewed Jan 3, 2018

View reviewed changes

Review fixes

0851ef2

vanzin reviewed Jan 5, 2018

View reviewed changes

Review fix

87c4852

SharedSQLContext explanation added

9c9c6ef

vanzin reviewed Jan 12, 2018

View reviewed changes

Turn off default thread audit for hive + SharedSQLContext comment enh…

56a41df

…ancement

Merge branch 'master' into SPARK-16139

68d0f3b

asfgit closed this in 12db365 Jan 16, 2018

gaborgsomogyi deleted the SPARK-16139 branch January 17, 2018 13:26

[SPARK-16139][TEST] Add logging functionality for leaked threads in tests #19893

[SPARK-16139][TEST] Add logging functionality for leaked threads in tests #19893

Conversation

gaborgsomogyi commented Dec 5, 2017

What changes were proposed in this pull request?

How was this patch tested?

gaborgsomogyi commented Dec 5, 2017

smurakozi commented Dec 5, 2017

gaborgsomogyi commented Dec 5, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gaborgsomogyi commented Dec 6, 2017 • edited

srowen commented Dec 6, 2017

gaborgsomogyi commented Dec 6, 2017

gaborgsomogyi commented Dec 6, 2017

gaborgsomogyi commented Dec 6, 2017

SparkQA commented Dec 6, 2017

gaborgsomogyi commented Dec 6, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gaborgsomogyi commented Dec 6, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vanzin commented Dec 6, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SparkQA commented Dec 6, 2017

SparkQA commented Dec 7, 2017

SparkQA commented Dec 7, 2017

SparkQA commented Dec 7, 2017

SparkQA commented Dec 7, 2017

gaborgsomogyi commented Dec 19, 2017

gaborgsomogyi commented Dec 21, 2017

SparkQA commented Dec 22, 2017

jiangxb1987 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SparkQA commented Dec 22, 2017

vanzin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gaborgsomogyi Jan 5, 2018 • edited

Choose a reason for hiding this comment

gaborgsomogyi commented Dec 5, 2017 •

edited

gaborgsomogyi commented Dec 6, 2017 •

edited

gaborgsomogyi Jan 5, 2018 •

edited