[GitHub] spark issue #21185: [SPARK-23894][CORE][SQL] Defensively clear ActiveSession...

2018-05-08 Thread squito
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...

2018-05-02 Thread squito
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...

2018-05-02 Thread SparkQA
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...

2018-05-02 Thread squito
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...

2018-05-02 Thread SparkQA
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...

2018-05-02 Thread cloud-fan
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...

2018-05-01 Thread jkbradley
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...

2018-05-01 Thread AmplabJenkins
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...

2018-05-01 Thread AmplabJenkins
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...

2018-05-01 Thread SparkQA
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...

2018-05-01 Thread squito
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...

2018-05-01 Thread AmplabJenkins
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...

2018-05-01 Thread AmplabJenkins
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...

2018-05-01 Thread SparkQA
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...

2018-05-01 Thread gatorsmile
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...

2018-05-01 Thread gatorsmile
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...

2018-05-01 Thread jiangxb1987
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...

2018-04-30 Thread gatorsmile
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...

2018-04-30 Thread SparkQA
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...

2018-04-30 Thread squito
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...

2018-04-30 Thread ericl
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...

2018-04-30 Thread gatorsmile
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...

2018-04-30 Thread SparkQA
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...

2018-04-30 Thread SparkQA
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...

2018-04-30 Thread vanzin
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...

2018-04-30 Thread vanzin
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...

2018-04-30 Thread squito
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...

2018-04-27 Thread cloud-fan
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...

2018-04-27 Thread AmplabJenkins
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...

2018-04-27 Thread AmplabJenkins
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...

2018-04-27 Thread SparkQA
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...

2018-04-27 Thread AmplabJenkins
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...

2018-04-27 Thread AmplabJenkins
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...

2018-04-27 Thread SparkQA
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