You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by dongjoon-hyun <gi...@git.apache.org> on 2018/01/23 05:03:35 UTC
[GitHub] spark pull request #20359: [SPARK-23186][SQL] Initialize DriverManager first...
GitHub user dongjoon-hyun opened a pull request:
https://github.com/apache/spark/pull/20359
[SPARK-23186][SQL] Initialize DriverManager first before loading JDBC Drivers
## What changes were proposed in this pull request?
Since some JDBC Drivers have class initialization code to call `DriverManager`, we need to initialize DriverManager first in order to avoid potential executor-side deadlock situations like [STORM-2527](https://issues.apache.org/jira/browse/STORM-2527).
## How was this patch tested?
N/A
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/dongjoon-hyun/spark SPARK-23186
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/20359.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 #20359
----
commit 234a637085451b27c7f7c5ca18d94d46c0815d9d
Author: Dongjoon Hyun <do...@...>
Date: 2018-01-23T04:59:31Z
[SPARK-23186][SQL] Initialize DriverManager first before loading Drivers
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20359: [SPARK-23186][SQL] Initialize DriverManager first...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/20359
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20359: [SPARK-23186][SQL] Initialize DriverManager first before...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20359
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/557/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20359: [SPARK-23186][SQL] Initialize DriverManager first before...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20359
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 #20359: [SPARK-23186][SQL] Initialize DriverManager first before...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20359
**[Test build #87020 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87020/testReport)** for PR 20359 at commit [`52e6f19`](https://github.com/apache/spark/commit/52e6f19a660be4d6a1589d42a452884a8caf26ac).
* This patch **fails Spark 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 #20359: [SPARK-23186][SQL] Initialize DriverManager first before...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20359
**[Test build #86530 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86530/testReport)** for PR 20359 at commit [`234a637`](https://github.com/apache/spark/commit/234a637085451b27c7f7c5ca18d94d46c0815d9d).
* 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 #20359: [SPARK-23186][SQL] Initialize DriverManager first before...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20359
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 #20359: [SPARK-23186][SQL] Initialize DriverManager first...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/20359#discussion_r163885202
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/DriverRegistry.scala ---
@@ -32,6 +32,9 @@ import org.apache.spark.util.Utils
*/
object DriverRegistry extends Logging {
+ // Initialize DriverManager first to prevent potential deadlocks between DriverManager and Driver
+ DriverManager.getDrivers
--- End diff --
Thank you for review, @cloud-fan .
So far, this deadlock situation is reported intermittently without any logs.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20359: [SPARK-23186][SQL] Initialize DriverManager first before...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/20359
thanks, merging to master/2.3!
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20359: [SPARK-23186][SQL] Initialize DriverManager first before...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20359
**[Test build #86514 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86514/testReport)** for PR 20359 at commit [`234a637`](https://github.com/apache/spark/commit/234a637085451b27c7f7c5ca18d94d46c0815d9d).
* This patch **fails due to an unknown error code, -9**.
* 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 #20359: [SPARK-23186][SQL] Initialize DriverManager first before...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the issue:
https://github.com/apache/spark/pull/20359
Thank you for review and approval, @srowen .
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20359: [SPARK-23186][SQL] Initialize DriverManager first before...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20359
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/87036/
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 #20359: [SPARK-23186][SQL] Initialize DriverManager first...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/20359#discussion_r163675854
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/DriverRegistry.scala ---
@@ -32,6 +32,9 @@ import org.apache.spark.util.Utils
*/
object DriverRegistry extends Logging {
+ // Initialize DriverManager first to prevent potential deadlocks between DriverManager and Driver
+ DriverManager.getDrivers
--- End diff --
We need a test; otherwise, it is not the right thing to merge such a PR.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20359: [SPARK-23186][SQL] Initialize DriverManager first before...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20359
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 #20359: [SPARK-23186][SQL] Initialize DriverManager first...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/20359#discussion_r163305903
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/DriverRegistry.scala ---
@@ -32,6 +32,9 @@ import org.apache.spark.util.Utils
*/
object DriverRegistry extends Logging {
+ // Initialize DriverManager first to prevent potential deadlocks between DriverManager and Driver
+ DriverManager.getDrivers
--- End diff --
Hi, @rxin .
Could you give me some comments on this?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20359: [SPARK-23186][SQL] Initialize DriverManager first before...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20359
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 #20359: [SPARK-23186][SQL] Initialize DriverManager first before...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20359
**[Test build #87036 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87036/testReport)** for PR 20359 at commit [`52e6f19`](https://github.com/apache/spark/commit/52e6f19a660be4d6a1589d42a452884a8caf26ac).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20359: [SPARK-23186][SQL] Initialize DriverManager first before...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the issue:
https://github.com/apache/spark/pull/20359
Thank you!
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20359: [SPARK-23186][SQL] Initialize DriverManager first...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/20359#discussion_r163678423
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/DriverRegistry.scala ---
@@ -32,6 +32,9 @@ import org.apache.spark.util.Utils
*/
object DriverRegistry extends Logging {
+ // Initialize DriverManager first to prevent potential deadlocks between DriverManager and Driver
+ DriverManager.getDrivers
--- End diff --
I'm wondering if you can merge this PR if I can test this patch in somewhere of our customer cluster. :)
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20359: [SPARK-23186][SQL] Initialize DriverManager first...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/20359#discussion_r163614580
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/DriverRegistry.scala ---
@@ -32,6 +32,9 @@ import org.apache.spark.util.Utils
*/
object DriverRegistry extends Logging {
+ // Initialize DriverManager first to prevent potential deadlocks between DriverManager and Driver
+ DriverManager.getDrivers
--- End diff --
Unfortunately, so far, I only have this log only. It's difficult to reproduce this deadlock.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20359: [SPARK-23186][SQL] Initialize DriverManager first before...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20359
**[Test build #86530 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86530/testReport)** for PR 20359 at commit [`234a637`](https://github.com/apache/spark/commit/234a637085451b27c7f7c5ca18d94d46c0815d9d).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20359: [SPARK-23186][SQL] Initialize DriverManager first before...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20359
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 #20359: [SPARK-23186][SQL] Initialize DriverManager first before...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20359
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/126/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20359: [SPARK-23186][SQL] Initialize DriverManager first before...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20359
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 #20359: [SPARK-23186][SQL] Initialize DriverManager first before...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20359
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 #20359: [SPARK-23186][SQL] Initialize DriverManager first before...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/20359
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 #20359: [SPARK-23186][SQL] Initialize DriverManager first before...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20359
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 #20359: [SPARK-23186][SQL] Initialize DriverManager first before...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the issue:
https://github.com/apache/spark/pull/20359
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 #20359: [SPARK-23186][SQL] Initialize DriverManager first before...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20359
**[Test build #87020 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87020/testReport)** for PR 20359 at commit [`52e6f19`](https://github.com/apache/spark/commit/52e6f19a660be4d6a1589d42a452884a8caf26ac).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20359: [SPARK-23186][SQL] Initialize DriverManager first...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/20359#discussion_r163631508
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/DriverRegistry.scala ---
@@ -32,6 +32,9 @@ import org.apache.spark.util.Utils
*/
object DriverRegistry extends Logging {
+ // Initialize DriverManager first to prevent potential deadlocks between DriverManager and Driver
+ DriverManager.getDrivers
--- End diff --
To be honest, it's not tested yet.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20359: [SPARK-23186][SQL] Initialize DriverManager first before...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20359
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/87020/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20359: [SPARK-23186][SQL] Initialize DriverManager first before...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20359
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/141/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20359: [SPARK-23186][SQL] Initialize DriverManager first before...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20359
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 #20359: [SPARK-23186][SQL] Initialize DriverManager first before...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the issue:
https://github.com/apache/spark/pull/20359
Thank you for review, @cloud-fan and @gatorsmile .
So far, there is no test case to generate this kind of Spark job hung (in executor-side).
I fully understand your viewpoints. I'll try this way in our customer production environment first after internal QE. Since this is intermittent deadlock situation, we don't know whether this is clearly removed or not. But, we can monitor that at least.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20359: [SPARK-23186][SQL] Initialize DriverManager first...
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/20359#discussion_r163750346
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/DriverRegistry.scala ---
@@ -32,6 +32,9 @@ import org.apache.spark.util.Utils
*/
object DriverRegistry extends Logging {
+ // Initialize DriverManager first to prevent potential deadlocks between DriverManager and Driver
--- End diff --
We need to say more about why this can avoid deadlock.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20359: [SPARK-23186][SQL] Initialize DriverManager first before...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/20359
let's send a new PR
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20359: [SPARK-23186][SQL] Initialize DriverManager first before...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the issue:
https://github.com/apache/spark/pull/20359
It's a well-known irrelevant failure.
- org.apache.spark.sql.kafka010.KafkaContinuousSourceStressForDontFailOnDataLossSuite.stress test for failOnDataLoss=false
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20359: [SPARK-23186][SQL] Initialize DriverManager first before...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20359
**[Test build #87025 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87025/testReport)** for PR 20359 at commit [`52e6f19`](https://github.com/apache/spark/commit/52e6f19a660be4d6a1589d42a452884a8caf26ac).
* This patch **fails due to an unknown error code, -9**.
* 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 #20359: [SPARK-23186][SQL] Initialize DriverManager first before...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the issue:
https://github.com/apache/spark/pull/20359
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 #20359: [SPARK-23186][SQL] Initialize DriverManager first before...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20359
**[Test build #87025 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87025/testReport)** for PR 20359 at commit [`52e6f19`](https://github.com/apache/spark/commit/52e6f19a660be4d6a1589d42a452884a8caf26ac).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20359: [SPARK-23186][SQL] Initialize DriverManager first...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/20359#discussion_r163604903
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/DriverRegistry.scala ---
@@ -32,6 +32,9 @@ import org.apache.spark.util.Utils
*/
object DriverRegistry extends Logging {
+ // Initialize DriverManager first to prevent potential deadlocks between DriverManager and Driver
+ DriverManager.getDrivers
--- End diff --
More context about this change? Based on your PR description, this is to resolve the deadlocks among executors? How does it work after applying this change?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20359: [SPARK-23186][SQL] Initialize DriverManager first before...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the issue:
https://github.com/apache/spark/pull/20359
Ping, @gatorsmile and @cloud-fan .
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20359: [SPARK-23186][SQL] Initialize DriverManager first...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/20359#discussion_r163678186
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/DriverRegistry.scala ---
@@ -32,6 +32,9 @@ import org.apache.spark.util.Utils
*/
object DriverRegistry extends Logging {
+ // Initialize DriverManager first to prevent potential deadlocks between DriverManager and Driver
+ DriverManager.getDrivers
--- End diff --
Do you mean a unit test case?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20359: [SPARK-23186][SQL] Initialize DriverManager first before...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the issue:
https://github.com/apache/spark/pull/20359
FYI, this is tested on a production cluster for two weeks and the deadlock issue is not reported until now.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20359: [SPARK-23186][SQL] Initialize DriverManager first before...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20359
**[Test build #87036 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87036/testReport)** for PR 20359 at commit [`52e6f19`](https://github.com/apache/spark/commit/52e6f19a660be4d6a1589d42a452884a8caf26ac).
* 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 #20359: [SPARK-23186][SQL] Initialize DriverManager first before...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20359
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/87025/
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 #20359: [SPARK-23186][SQL] Initialize DriverManager first...
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/20359#discussion_r163750551
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/DriverRegistry.scala ---
@@ -32,6 +32,9 @@ import org.apache.spark.util.Utils
*/
object DriverRegistry extends Logging {
+ // Initialize DriverManager first to prevent potential deadlocks between DriverManager and Driver
+ DriverManager.getDrivers
--- End diff --
if it's too hard to write a UT, I think a manual test is also fie.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20359: [SPARK-23186][SQL] Initialize DriverManager first before...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20359
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/559/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20359: [SPARK-23186][SQL] Initialize DriverManager first before...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the issue:
https://github.com/apache/spark/pull/20359
@cloud-fan . Can we have this in branch-2.2, too?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20359: [SPARK-23186][SQL] Initialize DriverManager first before...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20359
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 #20359: [SPARK-23186][SQL] Initialize DriverManager first before...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20359
**[Test build #86514 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86514/testReport)** for PR 20359 at commit [`234a637`](https://github.com/apache/spark/commit/234a637085451b27c7f7c5ca18d94d46c0815d9d).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20359: [SPARK-23186][SQL] Initialize DriverManager first before...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20359
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/86514/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20359: [SPARK-23186][SQL] Initialize DriverManager first before...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the issue:
https://github.com/apache/spark/pull/20359
Thank you for merging, @cloud-fan .
And thank you again, @HyukjinKwon , @gatorsmile , and @srowen !
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20359: [SPARK-23186][SQL] Initialize DriverManager first...
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/20359#discussion_r163750452
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/DriverRegistry.scala ---
@@ -32,6 +32,9 @@ import org.apache.spark.util.Utils
*/
object DriverRegistry extends Logging {
+ // Initialize DriverManager first to prevent potential deadlocks between DriverManager and Driver
--- End diff --
we can copy something from the storm PR: https://github.com/apache/storm/pull/2134/files
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20359: [SPARK-23186][SQL] Initialize DriverManager first...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/20359#discussion_r163630976
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/DriverRegistry.scala ---
@@ -32,6 +32,9 @@ import org.apache.spark.util.Utils
*/
object DriverRegistry extends Logging {
+ // Initialize DriverManager first to prevent potential deadlocks between DriverManager and Driver
+ DriverManager.getDrivers
--- End diff --
How did you test it?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20359: [SPARK-23186][SQL] Initialize DriverManager first before...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20359
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/86530/
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 #20359: [SPARK-23186][SQL] Initialize DriverManager first...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/20359#discussion_r163613813
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/DriverRegistry.scala ---
@@ -32,6 +32,9 @@ import org.apache.spark.util.Utils
*/
object DriverRegistry extends Logging {
+ // Initialize DriverManager first to prevent potential deadlocks between DriverManager and Driver
+ DriverManager.getDrivers
--- End diff --
It's the same situation like STORM.
In the Spark executor, the stacks shows the deadlock between `DriverManager` and `Driver`.
- Pheonix library call `DriverManager.loadInitialDrivers()`
- Spark `DriverRegistry` call `PhoenixDriver` constructor before `DriverManager` created.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20359: [SPARK-23186][SQL] Initialize DriverManager first before...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20359
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/565/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org