[ http://issues.apache.org/jira/browse/DERBY-210?page=all ]
Deepa Remesh updated DERBY-210:
-------------------------------
Attachment: derby-210-v2-draft.diff
derby-210-v2-draft.status
I am attaching a draft patch 'derby-210-v2-draft.diff' for review. It is not
ready for commit. This patch contains changes in my first patch and some
additional changes which I am describing below. I would appreciate if someone
can read this and let me know if this is the right approach.
The changes numbered 1-7 were done as part of first patch. I am copying it from
my previous comment:
1. Eliminates the below references to PreparedStatement objects by using
WeakHashMap instead of LinkedList. When there are no other references to the
keys in a WeakHashMap, they will get removed from the map and can thus get
garbage-collected. They do not have to wait till the Connection object is
collected.
- 'openStatements_' in org.apache.derby.client.am.Connection
- 'CommitAndRollbackListeners_' in org.apache.derby.client.am.Connection
2. Removes the list 'RollbackOnlyListeners_' since this is not being used.
3. Updates the following comment for openStatements_:
// Since DERBY prepared statements must be re-prepared after a commit,
// then we must traverse this list after a commit and notify statements
// that they are now in an un-prepared state.
final java.util.LinkedList openStatements_ = new java.util.LinkedList();
In the code, I did not see this list being traversed after a commit to
re-prepare statements. Also, I think this is not needed since Derby does not
require re-prepare of statements after a commit. Currently, this list is used
to close all open statements when the originating connection is closed.
4. Removes all ResultSets from HashTable
'positionedUpdateCursorNameToResultSet_' in SectionManager. Only result sets of
positioned update statements were being removed from this hashtable whereas all
result sets were added. Because of this, client driver was holding on to result
sets and statements even after rs.close() was called.
5. Adds a test 'derbyStress.java' to jdbcapi suite. This test is based on the
repro for this patch. Without this patch, it fails when run with client driver.
Kathey had suggested in another mail that tests for client memory leak problems
(DERBY-557, DERBY-210) can be added to same test. I did not see an existing
test. So I created this new test. If DERBY-557 does not have a test, I think it
can be added to this new test.
6. Excludes the new test from running with jcc because jcc gives out of memory
error.
7. Creates 'derbyStress_app.properties' with following property
'jvmflags=-Xmx64M' to guarantee the test fails on all machines.
With these changes, the memory leak mentioned in this issue was solved but it
was causing an intermittent failure in lang/updatableResultSet.java as
described in https://issues.apache.org/jira/browse/DERBY-210#action_12363439.
To solve the regression caused by the first patch, I changed the finalizer in
Statement and PreparedStatement classes to avoid network operations. Changes
are:
8. In PreparedStatement class, the finalizer was calling closeX method, which
was doing:
* Call super.closeX() ---> Statement.closeX()
* Cleanup parameter objects - parameterMetaData_, sql_, parameters_
array
* Remove the PreparedStatement from
connection_.CommitAndRollbackListeners_ list
Changes done by patch:
* Move cleanup of objects to a new method cleanupParameters()
* Call the new method cleanupParameters() from closeX() and finalize()
* In finalize() method, call super.finalize() which is
Statement.finalize() and it will do the remaining cleanup
* Since WeakHashMap is used for Connection.CommitAndRollbackListeners_,
the prepared statement object would already have been removed from this list
before entering the finalizer. So there is no need to explicitly call a remove
in the finalizer.
9. In Statement class, the finalizer was calling closeX method, which was doing:
* Close any open cursors for this statement on the server.
- If result set is open on server, send CLSQRY to the server.
- check if autocommit is required when closing result sets and
flow a commit to server, if required
* Call Statement.markClosed(), which does
- Mark close the result sets on the client
- If cursor name was set on the statement, remove it from
Connection.clientCursorNameCache_
- Call markClosed() on prepared statements for auto generated
keys
- Call markClosedOnServer(), which frees up the section. The
freed section will be re-used by new statements.
* Remove the Statement from Connection.openStatements_ list
* Cleanup ResultSetMetaData
Changes done by patch:
* Move the cleanup of ResultSetMetaData into markClosed() method. This
will keep all client-side cleanup in markClosed().
* Change the finalizer to just call markClosed(). This method frees up
client-side resources and operates on synchronized collections. So I have
removed the synchronize block from the finalizer. markClosed() in turn calls
markClosedOnServer(), which frees up a section. When the section gets re-used
by new statement on client, server re-uses the same statement. newDrdaStatement
in org.apache.derby.impl.drda.Database has this code:
/**
* Get a new DRDA statement and store it in the
stmtTable if stortStmt is true
* If possible recycle an existing statement
* If we are asking for one with the same name it means
it
* was already closed.
* @param pkgnamcsn Package name and section
* @return DRDAStatement
*/
protected DRDAStatement newDRDAStatement(Pkgnamcsn
pkgnamcsn)
throws SQLException
{
DRDAStatement stmt =
getDRDAStatement(pkgnamcsn);
if (stmt != null)
stmt.close();
else
{
stmt = new DRDAStatement(this);
stmt.setPkgnamcsn(pkgnamcsn);
storeStatement(stmt);
}
return stmt;
}
In the above method, close() is called before re-using the DRDAStatement. This
will ensure the statement state is restored. DRDAStatement.close() also ensures
the result set objects are closed and result set tables and all result sets are
freed on server side. So there is no need to send explicit CLSQRY from
client-side statement finalizer.
* Since WeakHashMap is used for Connection.openStatements_, the
statement object would already have been removed from this list before entering
the finalizer. So there is no need to explicitly call a remove in the finalizer.
* The autocommit logic does not exist in the finalizer since only
markClosed() is called from finalizer. This will avoid untimely commits which
was causing the regression in the test lang/updatableResultSet.java.
With the above changes, I ran derbynetclientmats few times with jdk14 and
jdk15. All tests passed except an intermittent failure in
derbynet/prepStmt.java. This was happening in the test added for Jira125 where
it calls rs.next(). I was getting the following exception:
java.lang.StringIndexOutOfBoundsException: String index out of range: 25242
at java.lang.String.checkBounds(String.java:372)
at java.lang.String.<init>(String.java:404)
at
org.apache.derby.client.net.NetCursor.readFdocaString(NetCursor.java:753)
at org.apache.derby.client.net.NetCursor.parseVCS(NetCursor.java:726)
at
org.apache.derby.client.net.NetCursor.parseSQLCAXGRP(NetCursor.java:693)
at
org.apache.derby.client.net.NetCursor.parseSQLCAGRP(NetCursor.java:622)
at
org.apache.derby.client.net.NetCursor.parseSQLCARD(NetCursor.java:595)
at
org.apache.derby.client.net.NetCursor.calculateColumnOffsetsForRow_(NetCursor.java:112)
at org.apache.derby.client.am.Cursor.next(Cursor.java:165)
at org.apache.derby.client.am.ResultSet.nextX(ResultSet.java:287)
at org.apache.derby.client.am.ResultSet.next(ResultSet.java:259)
at
org.apache.derbyTesting.functionTests.tests.derbynet.prepStmt.jira125Test_a(prepStmt.java:904)
at
org.apache.derbyTesting.functionTests.tests.derbynet.prepStmt.jira125Test(prepStmt.java:820)
at
org.apache.derbyTesting.functionTests.tests.derbynet.prepStmt.main(prepStmt.java:313)
It seemed to me that on the client side, the cleanup was happening correctly
but on server side, the statement re-use was not happening correctly.
DRDAStatement.close() was restoring all members except 'currentDrdaRs' and
'needsToSendParamData'.
10. I added the following to DRDAStatement.close() method. This will ensure the
previous result set is freed.
currentDrdaRs = new DRDAResultSet();
needsToSendParamData = false;
With this change, I am running derbynetclientmats in a loop. I did not see any
failures so far. I also ran the repro derbyStress.java with 50,000 prepared
statements and did not get an out of memory error.
I am attaching this interim draft patch to get feedback about the changes.
Please let me know if this approach is okay. Thanks much for reading this.
> Network Server will leak prepared statements if not explicitly closed by the
> user until the connection is closed
> ----------------------------------------------------------------------------------------------------------------
>
> Key: DERBY-210
> URL: http://issues.apache.org/jira/browse/DERBY-210
> Project: Derby
> Type: Bug
> Components: Network Client
> Reporter: Kathey Marsden
> Assignee: Deepa Remesh
> Attachments: DOTS_ATCJ2_Derby-noPatch.png, DOTS_ATCJ2_Derby-withPatch.png,
> derby-210-v2-draft.diff, derby-210-v2-draft.status, derby-210.diff,
> derby-210.status, derbyStress.java
>
> Network server will not garbage collect prepared statements that are not
> explicitly closed by the user. So a loop like this will leak.
> ...
> PreparedStatement ps;
> for (int i = 0 ; i < numPs; i++)
> {
> ps = conn.prepareStatement(selTabSql);
> rs =ps.executeQuery();
> while (rs.next())
> {
> rs.getString(1);
> }
> rs.close();
> // I'm a sloppy java programmer
> //ps.close();
> }
>
> To reproduce run the attached program
> java derbyStress
> Both client and server will grow until the connection is closed.
>
> It is likely that the fix for this will have to be in the client. The client
> does not send protocol to close the prepared statement, but rather reuses the
> PKGNAMCSN on the PRPSQLSTT request once the prepared statement has been
> closed. This is how the server knows to close the old statement and create a
> new one.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira
