-
Notifications
You must be signed in to change notification settings - Fork 28.6k
[SPARK-12048][SQL] Part 2 Prevent to close JDBC resources twice #10320
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Test build #47787 has finished for PR 10320 at commit
|
@andrewor14 @zsxwing |
@@ -100,6 +101,7 @@ class JdbcRDD[T: ClassTag]( | |||
} | |||
|
|||
override def close() { | |||
if (closed) return | |||
try { | |||
if (null != rs) { | |||
rs.close() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's another approach, though the other implementation opted for a close flag
I don't think we need this patch. I saw the following comments in javadoc
|
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 |
I compared JDBCRDD.scala with JdbcRDD.scala |
Looks |
Looks like the case is covered by NextIterator already |
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