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 <[email protected]> wrote:
> Thanks. Those are definitely great problems to fix!
>
>
>
> On Tue, Aug 5, 2014 at 1:11 PM, Stephen Boesch <[email protected]> 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 <[email protected]>:
> >
> > 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 <[email protected]>
> >> 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.
> >>>
> >>
> >>
> >
>
