Select from large lob table with embedded gives OutOfMemoryError

• Key: DERBY-3354
• Type: Bug
• Status: Closed
• Resolution: Fixed
• Priority: Major
• Assignee: Anurag Shekhar
• Reporter: Kathey Marsden
• Component/s: SQL
• Affects Version/s: 10.3.1.4, 10.3.2.1, 10.4.1.3
• Fix Version/s: 10.3.3.0, 10.4.1.3, 10.5.1.1

Description

Retrieving from a large table with lobs gives an OutOfMemoryException, even if free() is explictly called on the lob. I believe this is because EmbedConnection.addLobMapping is called for every lob creation but is never cleared until commit or rollback, even if the lob is freed.

Comments

[Kathey Marsden]

Atttached is a program to reproduce this issue.
Run java -Xmx16M LocLeak. I got the error on the 68000 row of retreival with the free(). Sooner if I didn't call free.

Exception in thread "main" java.lang.OutOfMemoryError: Java heap space
        at java.util.Arrays.copyOf(Arrays.java:2882)
        at java.lang.AbstractStringBuilder.expandCapacity(AbstractStringBuilder.java:100)
        at java.lang.AbstractStringBuilder.append(AbstractStringBuilder.java:390)
        at java.lang.StringBuilder.append(StringBuilder.java:119)
        at java.lang.Object.toString(Object.java:219)
        at java.lang.String.valueOf(String.java:2827)
        at java.lang.StringBuffer.append(StringBuffer.java:219)
        at org.apache.derby.impl.jdbc.EmbedConnection.restoreContextStack(EmbedConnection.java:1960)
        at org.apache.derby.impl.jdbc.ConnectionChild.restoreContextStack(ConnectionChild.java:131)
        at org.apache.derby.impl.jdbc.UTF8Reader.fillBuffer(UTF8Reader.java:567)
        at org.apache.derby.impl.jdbc.UTF8Reader.read(UTF8Reader.java:245)
        at org.apache.derby.impl.jdbc.ClobUpdatableReader.read(ClobUpdatableReader.java:154)
        at org.apache.derby.impl.jdbc.EmbedClob.getSubString(EmbedClob.java:225)
        at LocLeak.dump_db(LocLeak.java:50)
        at LocLeak.main(LocLeak.java:13)

Note there may be problems freeing the locators on garbage collection, because client relies upon them even after there is no embedded reference.

[Kathey Marsden]

Marking as regression as I confirmed this does not occur with 10.2.2.0

[Kathey Marsden]

This regression was caused by the following checkin:
r546881 | bernt | 2007-06-13 07:06:30 -0700 (Wed, 13 Jun 2007) | 1 line

DERBY-2787 make entry for clob in connection so that temporary file is removed when a connectionn is commited/rolledback. Submitted by Anurag Shekhar

[Knut Anders Hatlen]

> I believe this is because EmbedConnection.addLobMapping is called for every lob creation but is never cleared until commit or rollback, even if the lob is freed.

I believe your analysis is correct. free() should remove the mapping. Unfortunately, the lob doesn't know about its locator value, but it seems like your about to change that by adding a getLocator() method in DERBY-3243.

On DERBY-3243 I wrote:
> Another thing I came to think about: Would it be better to remove addLOBMapping() from the constructors in EmbedBlob/EmbedClob and instead call it lazily from getLocator()? (...) Then we'd also remove the overhead of maintaining the lob mapping in embedded mode.

I think that change would fix the OOME (on embedded, but we'd probably still see it in a client/server environment).

Now, it seems like DERBY-2787 added the call to addLOBMapping() intentionally to ensure that free() was called on commit/rollback and temporary files would be deleted. Moving addLOBMapping() out of the constructors would probably reintroduce the problem with temporary files not being deleted. However, the lobs only create temporary files when they are modified and the new size of the lob is greater than the buffer in LOBStreamControl, so it shouldn't be necessary to have commit/rollback call free() on all lobs to delete the temporary files. It would probably be enough if each lob added itself to a list in the connection each time a temporary file was created in LOBStreamControl.init(). (addLOBMapping() is probably OK, but it feels a bit strange to piggyback on a locator mechanism to do this cleanup when we don't actually care about the locators.)

[Øystein Grøvlen]

The attached patch makes the repro run without getting an OutOfMemory error.
This is achieved by removing the LOB object from the locator mapping.

I am not setting patch available since test cases should also be added,
and I have not run the regression test suites.

[Kathey Marsden]

Thanks for the patch. I tried it out and it and have a few comments.

1) In EmbedBlob should the call to localConn.removeLOBMapping(locator); be in a finally block like it is in EmbedClob?

2) In jdk15 we don't have free() so would still have a leak. Do you have any suggestions for a jdk15 solution?

3) I removed the call to free() from the repro and ran
java LocLeak and got.
Retrieving row 99000
Exception in thread "main" java.sql.SQLException: Java exception: ': java.util.ConcurrentModificationException'.
        at org.apache.derby.impl.jdbc.SQLExceptionFactory40.getSQLException(SQLExceptionFactory40.java:95)
        at org.apache.derby.impl.jdbc.Util.newEmbedSQLException(Util.java:88)
        at org.apache.derby.impl.jdbc.Util.javaException(Util.java:245)
        at org.apache.derby.impl.jdbc.TransactionResourceImpl.wrapInSQLException(TransactionResourceImpl.java:403)
        at org.apache.derby.impl.jdbc.TransactionResourceImpl.handleException(TransactionResourceImpl.java:346)
        at org.apache.derby.impl.jdbc.EmbedConnection.handleException(EmbedConnection.java:1946)
        at org.apache.derby.impl.jdbc.EmbedConnection.rollback(EmbedConnection.java:1521)
        at LocLeak.main(LocLeak.java:14)
Caused by: java.sql.SQLException: Java exception: ': java.util.ConcurrentModificationException'.
        at org.apache.derby.impl.jdbc.SQLExceptionFactory.getSQLException(SQLExceptionFactory.java:45)
        at org.apache.derby.impl.jdbc.SQLExceptionFactory40.wrapArgsForTransportAcrossDRDA(SQLExceptionFactory40.java:13
5)
        at org.apache.derby.impl.jdbc.SQLExceptionFactory40.getSQLException(SQLExceptionFactory40.java:70)
        ... 7 more
Caused by: java.util.ConcurrentModificationException
        at java.util.HashMap$HashIterator.nextEntry(HashMap.java:793)
        at java.util.HashMap$ValueIterator.next(HashMap.java:821)
        at org.apache.derby.impl.jdbc.EmbedConnection.clearLOBMapping(EmbedConnection.java:2737)
        at org.apache.derby.impl.jdbc.EmbedConnection.rollback(EmbedConnection.java:1519)
        ... 1 more

[Kathey Marsden]

Also should the removeLobMapping calls be removed from LobStoredProcedures.BLOBRELEASELOCATOR and CLOBRELEASELOCATOR now that the release happens in free()?

[Øystein Grøvlen]

Kathey, I agreed that more needs to be done for a final patch. I just put together a patch with the core fix to make it possible to try out the fix.

To your issues:
1) Creating a finally block in EmbedBlob seems like a good idea. I also agree that we can remove similar code from the stored procedures.

2) I will think about solution for jdk15. The problem introduced by 10.3 is that the lob mapping keeps Blob/Clob objects alive that earlier would be gc'ed when it was no longer referred by the user. I guess another level of indirection would make it possible to use finalizers to clean up, but I am not sure that is a good idea.
A work-around is to not create Blob objects in the first place, but use ResultSet#getBytes etc instead. So the problem is limited to the case where a Blob object need to be accessible after a call to ResultSet#next, but not until commit.

3) Seems like there is come concurrent access to the HashMap. I would guess that this is because the iterator created by rollback, detects that the underlying collection has been changed. Probably through the calls to free in the loop using the iterator. I think this show that something needs to be reorganized a bit here.

[Anurag Shekhar]

LobMapping and locater is used by the stored procedures (on behalf of
network driver) to get hold of a particular lob object. LobMapping is additionally
used to clear the lob object during commit/rollback and connection close.

Separating these two functionality in two different collections may solve OOM problem.

We can continue to have LobMapping only if the LOB is created through client driver (entry will
be made in LOBMapiing in getLocator)

Create methods will make an entry to a new collection holding week references of the LOBs.

This will ensure that LOB will get garbage collected if they unreferenced in Embedded
mode but in case of client server mode the references will be protected from being garbage
collected unless free is called by the stored procedure.

[Anurag Shekhar]

In this patch I have introduced a WeakHashMap to hold lob objects.
The WeakHashMap has lob objects as key and null as values.
The existing lobMapping is used only if a locater is requested (ie when accessed from client driver).

With this patch the attached test case (LocLeak.java) runs without getting out of memory exception (without free call from LocLeak).

This patch has changes related to Clob only and I havn;t run any test other that attached LocLeak. I will submit a complete patch shortly.

[Øystein Grøvlen]

If I understand correctly, the idea behind using a WeakHashMap is that if this is the only reference, it should not prevent the referred object from being garbage-collected.
In other words, as long as a user thread or the network server through locators, refers to the object, it will not be garbage-collected, but if such references does not exist, the object will be garbage-collected. This is an interesting idea.

My question is whether it is OK that free is not called on the LOB objects that are garbage-collected this way. Are you certain that all associated resources (e.g., temp files) will be released? Maybe some finalizer clean-up will also be needed?

Other comments/questions:
  * It seems getLocator will create a new locator on every call. I think there should be just a single locator per object. Otherwise, I think the clean-up will be difficult.
  * free: Willl removeLOBMapping handle correctly the cases where a locator has not been set?
  * typo: Refrence

[Anurag Shekhar]

Thanks Øystein for look at the patch.

My question is whether it is OK that free is not called on the LOB objects that are garbage-collected this way. Are you certain that all associated resources (e.g., temp files) will be released? Maybe some finalizer clean-up will also be needed?

If there is no free called (or release on internal clob) the temporary file will not be cleaned. But the temporary file
system is cleaned every time db boots up. I was under impression that disk operations are not allowed from
finalize. Probably I was wrong. I will check and if its not restricted I will add finalizer for EmbedClob and
EmbedBlob.

Other comments/questions:
  * It seems getLocator will create a new locator on every call. I think there should be just a single locator per object. Otherwise, I think the clean-up will be difficult.

I will be checking it to check if the locator is 0 before calling addMapping.

  * free: Willl removeLOBMapping handle correctly the cases where a locator has not been set?
HashMap ignore remove call if key is not found in the Map So there is no problem If we call
remove with invalid key.

  * typo: Refrence
I will fix this,

[Øystein Grøvlen]

I do think that it is a good idea to put disk operations in a finalizer, but it should be possible to record work that needs to be done (e.g., at transaction commit). I do not think delaying clean-up of files until db reboot is acceptable since a server may be running for months without a reboot.

[Anurag Shekhar]

Description of derby-3354_v1.diff
This patch introduces a new WeakHashMap in EmbedConnection. EmbedBlob and EmbedClob objects references are stored in this
map (objects as key and null as value). Adding entry to locater map is
differed till the first call of getLocater.
This ensures that there is entry of LOB objects in locater map if they are invoked in embedded mode.
As the keys of WeakHashMap doesn't prevents the objects from being
garbage collected, once the lob objects are unreferenced lob objects will
be garbage collected releasing the memory.

During commit/rollback or Connection.close, free is invoked on all the lob
objects from WeakHashMap and the map is cleared.

Modified files
java/engine/org/apache/derby/impl/jdbc/EmbedConnection.java
Added a new attribute lobRefrences of type WeakHashMap.
Added a new method addLOBReference to make an entry in new
 hash map.
Modified clearLOBMapping to use lobRefrences to fetch and invoke free on lob objects instead of lobHashMap.

java/engine/org/apache/derby/impl/jdbc/EmbedBlob.java
java/engine/org/apache/derby/impl/jdbc/EmbedClob.java

Modified constructs to call connection.lobRefrences instead of conn.addLOBMapping.
Modified getLocater method to check if the locater value is non zero
before returning and if its zero calling conn.addLOBMapping to make
entry of lob objects and getting locater value.
Calling removeLOBMapping in free method.


Cleanup of temporary file is already being taken care by the finalizer of
LOBStreamControl so I haven't added any new cleanup code for
finalizer.

lang and jdbcapi junit tests running clean with this patch applied. I am running rest of the test suite and will update the results of the same.

[Kristian Waagan]

I haven't validated the approach, but it seems reasonable. I applied the patch and compiled Derby.

Some *very minor* things you can consider changing:
 1) Typos: "refrences" -> "references" (5 occurrences)
 2) Use of a space between the method call / declaration and the following parenthesis is not consistent.
 3) Trailing whitespace at diff lines 28, 107 and 148.
 4) You could change the comment for the weak hash map to JavaDoc.

Nice and small change :)
Is this the final patch, or are the more coming up?

[Anurag Shekhar]

Thanks Kristian for looking at the patch.
I will make the changes you have suggested and upload the patch.
I am not planing for any more version of this patch (except for review changes.)

[Anurag Shekhar]

in derby-3354_v2.diff I have fixed the issues Kristian had pointed out.

junit suites runs without any failure wit this patch.

[Øystein Grøvlen]

Thanks for the patch Anurag. Comments:

 - I guess you need to do removeLOBMapping in EmbedBlob#free too.
 - Any resaon to keep references for already freed objects around until end of transaction?

[Anurag Shekhar]

Thanks Oystein for pointing out. I missed it, I will upload the patch after adding this.

[Anurag Shekhar]

Added localConn.removeLOBMapping in Blob.free
BlobClob4BlobTest runs fine with this change. I am
running suites.All now.

[Øystein Grøvlen]

Is the removeLOBMapping put in the right plcae?
Don't you have to do it regardless of whether it is materialized or not?
Maybe it should have been put in a finally block like for EmbedClob? (Ref. Kathey's comment on my initial patch)

[Anurag Shekhar]

Putting remove mapping in else was a mistake. Sorry about that.
I have moved it above other cleanup operation so that hash table is
cleared irrespective of the result of other clean up.

[Øystein Grøvlen]

Thanks, for the updates, Anurag
Patch v4 committed at revision 644764.

[Øystein Grøvlen]

It would be good if a test code be made based on the program that reproduced this error.

[Øystein Grøvlen]

Fix merged to 10.4 branch at revision 644916.
I am not setting this to resolve yet, since I think a test case for this should be added to the test suite.

[Kathey Marsden]

I was thinking about whether to close this issue. Is there still a regression test coming?

[Kathey Marsden]

Going ahead and closing this issue. It would be good to have a regression test though.