[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
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-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
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-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
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-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
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.(SharedState.scala:93) at org.apache.spark.sql.SparkSession$$anonfun$sharedState$1.apply(SparkSession.scala:117) ... ``` --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
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-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
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-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
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-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
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-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/21185 Merged build finished. Test FAILed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
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-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
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-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/21185 Merged build finished. Test PASSed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
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-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
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-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
Github user gatorsmile commented on the issue: https://github.com/apache/spark/pull/21185 ok to test --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
Github user gatorsmile commented on the issue: https://github.com/apache/spark/pull/21185 test this please --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
Github user jiangxb1987 commented on the issue: https://github.com/apache/spark/pull/21185 retest this please --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
Github user gatorsmile commented on the issue: https://github.com/apache/spark/pull/21185 LGTM pending Jenkins --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
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-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
Github user squito commented on the issue: https://github.com/apache/spark/pull/21185 Jenkins, retest this please --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
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-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
Github user gatorsmile commented on the issue: https://github.com/apache/spark/pull/21185 cc @ericl too --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
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-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
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-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
Github user vanzin commented on the issue: https://github.com/apache/spark/pull/21185 retest this please --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
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-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
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-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
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-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/21185 Merged build finished. Test PASSed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
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-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
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-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/21185 Merged build finished. Test PASSed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
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-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...
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-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org