You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by squito <gi...@git.apache.org> on 2018/04/27 20:11:07 UTC
[GitHub] spark pull request #21185: [SPARK-23894][CORE][SQL] Defensively clear Active...
GitHub user squito opened a pull request:
https://github.com/apache/spark/pull/21185
[SPARK-23894][CORE][SQL] Defensively clear ActiveSession in Executors
Because SparkSession.getActiveSession uses an InheritableThreadLocal,
the ThreadPool in executors might end up inheriting the SparkSession
from a driver thread. This leads to test failures as executors should
never have an active SparkSession. So in local mode, defensively clear
the active session.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/squito/spark SPARK-23894
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/21185.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #21185
----
commit 2a4944ffe5836408b80f9aa06e9b28e57aa16649
Author: Imran Rashid <ir...@...>
Date: 2018-04-27T20:03:14Z
[SPARK-23894][CORE][SQL] Defensively clear ActiveSession in Executors
Because SparkSession.getActiveSession uses an InheritableThreadLocal,
the ThreadPool in executors might end up inheriting the SparkSession
from a driver thread. This leads to test failures as executors should
never have an active SparkSession. So in local mode, defensively clear
the active session.
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21185
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on the issue:
https://github.com/apache/spark/pull/21185
@jkbradley still seeing flakiness in R tests, in other PRs too. I'm not even sure how to interpret the failure. is it safe to ignore those? I can't see how this would be effecting R tests ...
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21185: [SPARK-23894][CORE][SQL] Defensively clear Active...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/21185#discussion_r186772234
--- Diff: core/src/main/scala/org/apache/spark/executor/Executor.scala ---
@@ -229,6 +229,23 @@ private[spark] class Executor(
ManagementFactory.getGarbageCollectorMXBeans.asScala.map(_.getCollectionTime).sum
}
+ /**
+ * Only in local mode, we have to prevent the driver from setting the active SparkSession
+ * in the executor threads. See SPARK-23894.
+ */
+ private lazy val clearActiveSparkSessionMethod = if (Utils.isLocalMaster(conf)) {
--- End diff --
agreed, I closed this PR
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on the issue:
https://github.com/apache/spark/pull/21185
I'm closing this in favor of https://github.com/apache/spark/pull/21190
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/21185
test this please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21185: [SPARK-23894][CORE][SQL] Defensively clear Active...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/21185#discussion_r186358733
--- Diff: core/src/main/scala/org/apache/spark/executor/Executor.scala ---
@@ -229,6 +229,23 @@ private[spark] class Executor(
ManagementFactory.getGarbageCollectorMXBeans.asScala.map(_.getCollectionTime).sum
}
+ /**
+ * Only in local mode, we have to prevent the driver from setting the active SparkSession
+ * in the executor threads. See SPARK-23894.
+ */
+ private lazy val clearActiveSparkSessionMethod = if (Utils.isLocalMaster(conf)) {
--- End diff --
I've added this check in https://github.com/apache/spark/pull/21190
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21185: [SPARK-23894][CORE][SQL] Defensively clear Active...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/21185#discussion_r184813243
--- Diff: core/src/main/scala/org/apache/spark/executor/Executor.scala ---
@@ -229,6 +229,23 @@ private[spark] class Executor(
ManagementFactory.getGarbageCollectorMXBeans.asScala.map(_.getCollectionTime).sum
}
+ /**
+ * Only in local mode, we have to prevent the driver from setting the active SparkSession
+ * in the executor threads. See SPARK-23894.
+ */
+ lazy val clearActiveSparkSessionMethod = if (Utils.isLocalMaster(conf)) {
--- End diff --
private?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21185
**[Test build #4169 has finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/4169/testReport)** for PR 21185 at commit [`49783c5`](https://github.com/apache/spark/commit/49783c5e3a6fa835fced52fb9cfb478065c3c044).
* This patch **fails SparkR unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/21185
I think `SparkSession` is driver only, how do we access it in executor?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21185
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/2728/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21185
**[Test build #4169 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/4169/testReport)** for PR 21185 at commit [`49783c5`](https://github.com/apache/spark/commit/49783c5e3a6fa835fced52fb9cfb478065c3c044).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21185
**[Test build #89939 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/89939/testReport)** for PR 21185 at commit [`2a4944f`](https://github.com/apache/spark/commit/2a4944ffe5836408b80f9aa06e9b28e57aa16649).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on the issue:
https://github.com/apache/spark/pull/21185
@cloud-fan
> I think SparkSession is driver only, how do we access it in executor?
that's the whole problem. Its only meant to be available on the driver, but it ends up getting set on the executor when running in local mode. Because it uses an *inheritable* thread local, when the executor thread pool creates a new thread, the executor thread ends up inheriting the active session of the driver.
In cluster mode, when the executor is a totally separate JVM, there is no problem.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21185
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/2752/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21185
**[Test build #4161 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/4161/testReport)** for PR 21185 at commit [`49783c5`](https://github.com/apache/spark/commit/49783c5e3a6fa835fced52fb9cfb478065c3c044).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on the issue:
https://github.com/apache/spark/pull/21185
I don't understand the comments about "leaked threads". the executor thread pool is allowed to create threads whenever it wants. You can play around with this example if you like: https://gist.github.com/squito/8fc6533b1eeeb48559302c5898ae2c1d
The only "leak" here is potentially using an InheritableThreadLocal for the activeSession at all.
`SparkSession.getActiveSession` is a public api so we probably don't want to change the thread-inheritance behavior that it currently has. You could change `SparkSession.getActiveSession` to instead do the same thing itself, walking up the parent threads, but stopping if it hits an executor thread. But, seems more complex for no gain.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21185
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
Posted by jkbradley <gi...@git.apache.org>.
Github user jkbradley commented on the issue:
https://github.com/apache/spark/pull/21185
There have been several of these R tests. May be from flakiness with CRAN; testing locally now (since I didn't see any recent bad commits in R).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/21185
ok to test
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/21185
Tests haven't triggered, weird. LGTM pending tests.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/21185
> that's the whole problem. Its only meant to be available on the driver, but it ends up getting set on the executor when running in local mode.
Yea I think this is the root cause. I'm making a [PR](https://github.com/apache/spark/pull/21190) to ban `SQLConf.get` at executor side, shall we do the same thing for SparkSession? And fixes all the places that mistakenly access `SparkSession` in executor.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
Posted by jiangxb1987 <gi...@git.apache.org>.
Github user jiangxb1987 commented on the issue:
https://github.com/apache/spark/pull/21185
retest this please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on the issue:
https://github.com/apache/spark/pull/21185
> Yea I think this is the root cause. I'm making a PR to ban SQLConf.get at executor side, shall we do the same thing for SparkSession? And fixes all the places that mistakenly access SparkSession in executor.
ah that makes sense. All the failures I observed w/ the executor accessing the active session were actually via `SQLConf.get`. I don't know of any others.
I think it makes sense to actively ban `SparkSession.getActiveSession` the same way. I can include that in this PR, or make another one (or I guess you can put it into your PR), whatever you prefer. I don't think there will be any existing uses that need to be fixed, as I'm pretty sure if you did access it, you'd get the exception we've seen in tests related to the listener bus:
```
08:36:34.694 Executor task launch worker for task 436 ERROR Executor: Exception in task 0.0 in stage 402.0 (TID 436)
java.lang.IllegalStateException: LiveListenerBus is stopped.
at org.apache.spark.scheduler.LiveListenerBus.addToQueue(LiveListenerBus.scala:97)
at org.apache.spark.scheduler.LiveListenerBus.addToStatusQueue(LiveListenerBus.scala:80)
at org.apache.spark.sql.internal.SharedState.<init>(SharedState.scala:93)
at org.apache.spark.sql.SparkSession$$anonfun$sharedState$1.apply(SparkSession.scala:117)
...
```
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21185
**[Test build #4160 has finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/4160/testReport)** for PR 21185 at commit [`49783c5`](https://github.com/apache/spark/commit/49783c5e3a6fa835fced52fb9cfb478065c3c044).
* This patch **fails SparkR unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
Posted by ericl <gi...@git.apache.org>.
Github user ericl commented on the issue:
https://github.com/apache/spark/pull/21185
This makes sense to me. It would be slightly to clear it where where the session is getting leaked through threads, but if that's hard then this looks good.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21185: [SPARK-23894][CORE][SQL] Defensively clear Active...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/21185#discussion_r185612118
--- Diff: core/src/main/scala/org/apache/spark/executor/Executor.scala ---
@@ -229,6 +229,23 @@ private[spark] class Executor(
ManagementFactory.getGarbageCollectorMXBeans.asScala.map(_.getCollectionTime).sum
}
+ /**
+ * Only in local mode, we have to prevent the driver from setting the active SparkSession
+ * in the executor threads. See SPARK-23894.
+ */
+ private lazy val clearActiveSparkSessionMethod = if (Utils.isLocalMaster(conf)) {
--- End diff --
Do we still need to call this if we can resolve the root cause in https://github.com/apache/spark/pull/21190, and issue an error promptly like what you said in https://github.com/apache/spark/pull/21185#issuecomment-385996077?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on the issue:
https://github.com/apache/spark/pull/21185
Jenkins, retest this please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/21185
LGTM pending Jenkins
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21185
**[Test build #89978 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/89978/testReport)** for PR 21185 at commit [`49783c5`](https://github.com/apache/spark/commit/49783c5e3a6fa835fced52fb9cfb478065c3c044).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21185
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/89978/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21185: [SPARK-23894][CORE][SQL] Defensively clear Active...
Posted by squito <gi...@git.apache.org>.
Github user squito closed the pull request at:
https://github.com/apache/spark/pull/21185
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21185
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/21185
cc @ericl too
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/21185
retest this please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21185
**[Test build #89978 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/89978/testReport)** for PR 21185 at commit [`49783c5`](https://github.com/apache/spark/commit/49783c5e3a6fa835fced52fb9cfb478065c3c044).
* This patch **fails SparkR unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21185
**[Test build #89939 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/89939/testReport)** for PR 21185 at commit [`2a4944f`](https://github.com/apache/spark/commit/2a4944ffe5836408b80f9aa06e9b28e57aa16649).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21185
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21185: [SPARK-23894][CORE][SQL] Defensively clear Active...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/21185#discussion_r184813348
--- Diff: core/src/main/scala/org/apache/spark/executor/Executor.scala ---
@@ -299,6 +316,9 @@ private[spark] class Executor(
Thread.currentThread.setContextClassLoader(replClassLoader)
val ser = env.closureSerializer.newInstance()
logInfo(s"Running $taskName (TID $taskId)")
+ // When running in local mode, we might end up with the active session from the driver set on
+ // this thread, though we never should, so we defensively clear it. See SPARK-23894.
+ clearActiveSparkSessionMethod.foreach(_.invoke(null))
--- End diff --
Can this be done in the thread pool's thread factory instead?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21185
**[Test build #4160 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/4160/testReport)** for PR 21185 at commit [`49783c5`](https://github.com/apache/spark/commit/49783c5e3a6fa835fced52fb9cfb478065c3c044).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21185: [SPARK-23894][CORE][SQL] Defensively clear Active...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/21185#discussion_r184993529
--- Diff: core/src/main/scala/org/apache/spark/executor/Executor.scala ---
@@ -299,6 +316,9 @@ private[spark] class Executor(
Thread.currentThread.setContextClassLoader(replClassLoader)
val ser = env.closureSerializer.newInstance()
logInfo(s"Running $taskName (TID $taskId)")
+ // When running in local mode, we might end up with the active session from the driver set on
+ // this thread, though we never should, so we defensively clear it. See SPARK-23894.
+ clearActiveSparkSessionMethod.foreach(_.invoke(null))
--- End diff --
ThreadFactories only create threads, they don't start them, so you can't do it directly there, as I need this to run *inside* the thread.
I thought about trying to do something fancier, like having the thread pool also start the thread and submit one runnable to do this, or having threads get promoted to the correct thread pool after this ran, but I felt like it was too error-prone for minimal gain.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21185
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/89939/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org