You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by tedyu <gi...@git.apache.org> on 2015/12/16 04:53:17 UTC
[GitHub] spark pull request: [SPARK-12048][SQL] Part 2 Prevent to close JDB...
GitHub user tedyu opened a pull request:
https://github.com/apache/spark/pull/10320
[SPARK-12048][SQL] Part 2 Prevent to close JDBC resources twice
PR #10101 dealt with sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JDBCRDD.scala
This PR applies the same technique on JdbcRDD.scala
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/tedyu/spark master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/10320.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 #10320
----
commit 522afb7a4c255f62d02612b1235b402a1f0f8c5f
Author: tedyu <yu...@gmail.com>
Date: 2015-12-16T03:51:35Z
[SPARK-12048][SQL] Part 2 Prevent to close JDBC resources twice
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12048][SQL] Part 2 Prevent to close JDB...
Posted by tedyu <gi...@git.apache.org>.
Github user tedyu commented on the pull request:
https://github.com/apache/spark/pull/10320#issuecomment-165544488
Looks like the case is covered by NextIterator already
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12048][SQL] Part 2 Prevent to close JDB...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on the pull request:
https://github.com/apache/spark/pull/10320#issuecomment-165209606
> Javadoc of which? the JDK interfaces? I think the issue was that SQLite didn't actually seem to like being closed twice in practice.
Forgot to read the JIRA at first. Didn't know that `SQLite` doesn't follow the Javadoc of the JDK interfaces.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12048][SQL] Part 2 Prevent to close JDB...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/10320#issuecomment-165206325
Javadoc of which? the JDK interfaces? I think the issue was that SQLite didn't actually seem to like being closed twice in practice.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12048][SQL] Part 2 Prevent to close JDB...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10320#issuecomment-165005649
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12048][SQL] Part 2 Prevent to close JDB...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on the pull request:
https://github.com/apache/spark/pull/10320#issuecomment-165539190
Looks `NextIterator` already has a `closed` field.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12048][SQL] Part 2 Prevent to close JDB...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10320#issuecomment-164985229
**[Test build #47787 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47787/consoleFull)** for PR 10320 at commit [`522afb7`](https://github.com/apache/spark/commit/522afb7a4c255f62d02612b1235b402a1f0f8c5f).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12048][SQL] Part 2 Prevent to close JDB...
Posted by tedyu <gi...@git.apache.org>.
Github user tedyu commented on the pull request:
https://github.com/apache/spark/pull/10320#issuecomment-165209976
I compared JDBCRDD.scala with JdbcRDD.scala
From what I can tell according to the usage of java.sql.Connection and java.sql.ResultSet, the proposed change is needed
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12048][SQL] Part 2 Prevent to close JDB...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10320#issuecomment-165005652
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/47787/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12048][SQL] Part 2 Prevent to close JDB...
Posted by tedyu <gi...@git.apache.org>.
Github user tedyu closed the pull request at:
https://github.com/apache/spark/pull/10320
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12048][SQL] Part 2 Prevent to close JDB...
Posted by jerryshao <gi...@git.apache.org>.
Github user jerryshao commented on a diff in the pull request:
https://github.com/apache/spark/pull/10320#discussion_r47741408
--- Diff: core/src/main/scala/org/apache/spark/rdd/JdbcRDD.scala ---
@@ -100,6 +101,7 @@ class JdbcRDD[T: ClassTag](
}
override def close() {
+ if (closed) return
try {
if (null != rs) {
rs.close()
--- End diff --
From my understanding, we could call `rs = null` after it is closed, also for `stmt` and `conn` in the same way to make this function is reenterable.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12048][SQL] Part 2 Prevent to close JDB...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/10320#discussion_r47745421
--- Diff: core/src/main/scala/org/apache/spark/rdd/JdbcRDD.scala ---
@@ -100,6 +101,7 @@ class JdbcRDD[T: ClassTag](
}
override def close() {
+ if (closed) return
try {
if (null != rs) {
rs.close()
--- End diff --
That's another approach, though the other implementation opted for a close flag
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12048][SQL] Part 2 Prevent to close JDB...
Posted by tedyu <gi...@git.apache.org>.
Github user tedyu commented on the pull request:
https://github.com/apache/spark/pull/10320#issuecomment-165006066
@andrewor14 @zsxwing
Please take a look
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12048][SQL] Part 2 Prevent to close JDB...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on the pull request:
https://github.com/apache/spark/pull/10320#issuecomment-165204404
I don't think we need this patch. I saw the following comments in javadoc
```
* Calling the method <code>close</code> on a <code>Connection</code>
* object that is already closed is a no-op.
* Calling the method <code>close</code> on a <code>Statement</code>
* object that is already closed has no effect.
* Calling the method <code>close</code> on a <code>ResultSet</code>
* object that is already closed is a no-op.
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12048][SQL] Part 2 Prevent to close JDB...
Posted by tedyu <gi...@git.apache.org>.
Github user tedyu commented on the pull request:
https://github.com/apache/spark/pull/10320#issuecomment-165516736
@zsxwing @srowen
Anything I need to do for this PR ?
Thanks
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12048][SQL] Part 2 Prevent to close JDB...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10320#issuecomment-165005571
**[Test build #47787 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47787/consoleFull)** for PR 10320 at commit [`522afb7`](https://github.com/apache/spark/commit/522afb7a4c255f62d02612b1235b402a1f0f8c5f).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:\n * `trait JobSubmitter `\n * `class ComplexFutureAction[T](run : JobSubmitter => Future[T])`\n * ` case class AttachCompletedRebuildUI(appId: String)`\n * `case class WrapOption(child: Expression, optType: DataType)`\n
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org