spark-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Reynold Xin <r...@databricks.com>
Subject Re: Tiny curiosity question on closing the jdbc connection
Date Tue, 05 Aug 2014 21:01:33 GMT
Yes it is. I actually commented on it:
https://github.com/apache/spark/pull/1792/files#r15840899



On Tue, Aug 5, 2014 at 1:58 PM, Cody Koeninger <cody@koeninger.org> wrote:

> The stmt.isClosed just looks like stupidity on my part, no secret
> motivation :)  Thanks for noticing it.
>
> As for the leaking in the case of malformed statements, isn't that
> addressed by
>
> context.addOnCompleteCallback{ () => closeIfNeeded() }
>
> or am I misunderstanding?
>
>
> On Tue, Aug 5, 2014 at 3:15 PM, Reynold Xin <rxin@databricks.com> wrote:
>
> > Thanks. Those are definitely great problems to fix!
> >
> >
> >
> > On Tue, Aug 5, 2014 at 1:11 PM, Stephen Boesch <javadba@gmail.com>
> wrote:
> >
> > > Thanks Reynold, Ted Yu did mention offline and I put in a jira already.
> > > Another small concern: there appears to be no exception handling from
> the
> > > creation of the prepared statement (line 74) through to the
> executeQuery
> > > (line 86).   In case of error/exception it would seem to be leaking
> > > connections (/statements).  If that were the case then I would include
> a
> > > small patch for the exception trapping in that section of code as well.
> > >  BTW I was looking at this code for another reason, not intending to
> be a
> > > bother ;)
> > >
> > >
> > >
> > >
> > > 2014-08-05 13:03 GMT-07:00 Reynold Xin <rxin@databricks.com>:
> > >
> > > I'm pretty sure it is an oversight. Would you like to submit a pull
> > >> request to fix that?
> > >>
> > >>
> > >>
> > >> On Tue, Aug 5, 2014 at 12:14 PM, Stephen Boesch <javadba@gmail.com>
> > >> wrote:
> > >>
> > >>> Within its compute.close method, the JdbcRDD class has this
> interesting
> > >>> logic for closing jdbc connection:
> > >>>
> > >>>
> > >>>       try {
> > >>>         if (null != conn && ! stmt.isClosed()) conn.close()
> > >>>         logInfo("closed connection")
> > >>>       } catch {
> > >>>         case e: Exception => logWarning("Exception closing
> connection",
> > >>> e)
> > >>>       }
> > >>>
> > >>> Notice that the second check is on stmt  having been closed - not on
> > the
> > >>> connection.
> > >>>
> > >>> I would wager this were not a simple oversight and there were some
> > >>> motivation for this logic- curious if anyone would be able to shed
> some
> > >>> light?   My particular interest is that I have written custom ORM's
> in
> > >>> jdbc
> > >>> since late 90's  and never did it this way.
> > >>>
> > >>
> > >>
> > >
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message