Issue Details (XML | Word | Printable)

Key: DERBY-912
Type: Sub-task Sub-task
Status: Closed Closed
Resolution: Fixed
Priority: Major Major
Assignee: Knut Anders Hatlen
Reporter: Manish Khettry
Votes: 0
Watchers: 1
Operations

If you were logged in you would be able to see more operations.
Derby
DERBY-756

OutOfMemory error on continuous execution of SQL statement

Created: 03/Feb/06 04:00 AM   Updated: 24/Jan/08 01:10 PM
Component/s: Store
Affects Version/s: 10.0.2.0
Fix Version/s: 10.2.2.0, 10.3.1.4

Time Tracking:
Not Specified

File Attachments:
  Size
Text File Licensed for inclusion in ASF works derby-912-v2.diff 2006-10-24 09:34 AM Knut Anders Hatlen 2 kB
File Licensed for inclusion in ASF works derby-912.diff 2006-10-20 06:32 PM Knut Anders Hatlen 2 kB
Environment: cygwin, jdk 1.4
Issue Links:
Duplicate
 
Reference
 

Resolution Date: 25/Oct/06 07:55 AM


 Description  « Hide
After fixing the original memory leak, I still run into problems on repeated execution of a sql statement. Take the sample program in the bug and run it with a small heap size (4m). After around 80-90K executions an outofmemory error is thrown. I took snapshots of the heap while the program was running but couldn't find anything obviously wrong.

 All   Comments   Work Log   Change History   Subversion Commits      Sort Order: Ascending order - Click to sort in descending order
Manish Khettry added a comment - 18/Feb/06 11:42 AM
It looks like elements from the sorts vector in RAMTransaction are never removed. dropSort merely nulls out the element. If you repeatedly open/execute/close a query that uses a sorter you will leak one reference every iteration.

Mike Matrigali added a comment - 21/Feb/06 04:04 AM
Is the comment about the sort memory being used in AccessManger an observation about the original test program?
Yes sorts currently require one array entry per sort per transaction. At end of transaction this memory is freed. When
originally coded the expectation was small number of sorts per transaction. Note it
is not as simple as shrinking the array in dropSort, as the array index is currently the "sort id". So a little more work
would be necessary. A couple of options:
1) see if we could reuse sort id's.
2) allocate sort id's differently and then probably store sort in hash table rather than array/vector

Any idea if this is the "only" remaining growing memory issue for this test. The growth should be only one array entry
per statement, the contents are null'd so garbage collection should happen on everything else.

Manish Khettry added a comment - 22/Feb/06 03:16 AM
Yes, it is from the original test program. BTW, I added a conn.commit() to the original program-- so it basically looks like this:

while (true)
{
  ps = conn.prepareStatement(sql);
  rs = ps.executeQuery();
  // read from rs.
  conn.commit();
  // close rs, ps
}

and it still runs out of memory. Shouldn't doing a commit create a new transaction?

Knut Anders Hatlen added a comment - 19/Oct/06 08:55 PM
The problem seems to be that RAMTransaction.sorts is only cleared when the connection is closed, not when the transaction is committed. RAMTransaction.commit() calls closeControllers() with closeHeldControllers=false, so sorts.removeAllElements() is not called on commit.

Knut Anders Hatlen added a comment - 19/Oct/06 09:41 PM
One simple solution is to remove all trailing null elements from the sorts vector on commit. That should leave the held controllers open and not interfere with the sort ids. While it does not solve the general case (a held controller may prevent space reclaimation at the start of the vector), it does solve the most common cases. With that change, the test program completed without running out of memory (-Xmx10M).

Knut Anders Hatlen added a comment - 20/Oct/06 06:32 PM
Attaching a patch which implements reuse of sort identifiers, as suggested by Mike.

I have successfully run the repro for DERBY-756 without getting OutOfMemoryError. I have also run the full set of regression tests (derbyall + JUnit) with no failures.

The patch is ready for review. Thanks!

Dyre Tjeldvoll added a comment - 24/Oct/06 08:29 AM
I think the patch looks good. Is there a reason why freeSortIds cannot be a valid ArrayList at all times? If it was, you would not need the null-tests. I guess always having a valid ArrayList would increase footprint unnecessarily if it is rarely used. Is that added footprint still a problem if freeSortIds is initialized to an ArrayList with zero capacity (new ArrayList(0))?

+1 to commit

Knut Anders Hatlen added a comment - 24/Oct/06 09:34 AM
Thanks for reviewing the patch, Dyre. I agree with your comment about freeSortIds and null tests. Uploading a new patch which initializes freeSortIds when the sorts vector is initialized. Since freeSortIds is only used when sorts != null, no tests for freeSortIds != null are needed. I have started a derbyall run.

Knut Anders Hatlen added a comment - 25/Oct/06 07:55 AM
Committed to trunk with revision 467578.
Committed to 10.2 with revision 467580.

John H. Embretsen added a comment - 21/Dec/06 12:19 PM
Linking DERBY-912 and DERBY-2176, as preliminary test results show that both issues seem to be fixed between 10.2.16 and 10.2.2.0.

Dyre Tjeldvoll added a comment - 24/Jan/08 01:10 PM
This issue is resolved and has not been updated in the last 12 months (since 24/Jan/07).