db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Daniel John Debrunner <...@apache.org>
Subject Re: [jira] Commented: (DERBY-210) Network Server will leak prepared statements if not explicitly closed by the user until the connection is closed
Date Wed, 15 Feb 2006 15:17:43 GMT
Deepa Remesh wrote:

> On 2/13/06, Daniel John Debrunner (JIRA) <derby-dev@db.apache.org> wrote:
> 
>>    [ http://issues.apache.org/jira/browse/DERBY-210?page=comments#action_12366232
]
>>
>>Daniel John Debrunner commented on DERBY-210:
>>---------------------------------------------
>>
>>The use of close() for close and reset is confusing (I know it's existing code).
>>
>>Your addition is now the only  code in close() that actually generates new objects.
This code
>>will be called even when the statement is being closed in order that it no longer
be used.
>>This might have a performance impact, I don't know how often in a well behaved application
>>this internal close method would be called. Once per statement execution or creation?
>>
>>I'm unclear on what this patch ('derby-210-patch3.diff') is addrressing. The comments
for
>>item 10) above don't actually help me much. Was the old code successfully re-using
the old
>>DRDAResultSet() but now for some reason we force it to use a new one every time?


Thanks for the explanation.


> The old code was successfully re-using DRDAResultSet. This patch will
> be required after my changes to the finalization code in client
> driver. With these changes, a CLSQRY is not sent for each result set
> to network server during finalization. The cleanup of result sets is
> expected to happen when the DRDAStatement is reset. But this was not
> happening and I was getting a failure in derbynet/prepStmt.java when I
> was running tests with the full set of changes. One solution to this
> was to create a new DRDAResultSet, same as what would be done for a
> new statement. I added this and a comment to say "close() method is
> also called before re-using a statement in the method
> Database.newDRDAStatement. It has to ensure that the statement state
> is restored. ". I hope this explains the reason for this patch.

I guess it's still not clear to me. It's also not clear from the
comments in the code, if someone looked in six months at this code and
compared it to the previous version, they would not see any reason as to
why the code stopped re-using an object.

I'm pushing for an explaination because I think fixing 210 is important
for Derby but it's an area we need to get correct. My gut feeling is
telling me that the fix is of the form "doing this make the test pass
but I don't really know why", but it could be because I don't know the
code, I'm missing context/implications that you see because you do know
the code. I feel if the coder can not explain the fix clearly, then
there's a good chance the fix is incorrect, leading to bugs in the future.

> As I mentioned before, I was thinking if it was okay to add a "new"
> into the close method. I did not see any performance impact since
> close() gets called when re-using statements or when the session is
> closed. During a session, DRDAStatement.close() method gets called
> only when the statement is re-used. When session is closed,
> DRDAStatement.close() gets called for each statement in the statement
> table and the statement also gets removed from the hash table. From
> what I understood, I think there is no performance impact because of
> this change. Please correct me if I am wrong here.

Is it possible to re-write this to be clear if you are referring to the
JDBC statement object the application is using or the DRDAStatement.
It's another unfortunate situation where the code overloads the term
statement. If the relationship is written up elsewhere, please point me
to it. For example, with this piece of JDBC code, at what points is the
DRDAstatement closed, re-used etc.

 Statement s = conn.createStatement();

  ResultSet rs = s.execute(Q1);
  .. process rs
  rs.close();

  rs = s.execute(Q1);
  .. process rs
  rs.close();

  rs = s.execute(Q2);
  .. process rs
  rs.close();

  s.close();

I'm interested in knowing when (and hence how often) with this change
Derby will be creating a new object instead of re-using one, and with
the change as it is, how often it is creating a new object just for it
to be thrown away.

Thanks for all the effort you are putting into 210.
Dan.





Mime
View raw message