deleteAll() calls getCompleteList(), and runs out of memory

Technical Discussion
1 
1 : import static org.moqui.util.ObjectUtilities.*
2 : import static org.moqui.util.CollectionUtilities.*
3 : import static org.moqui.util.StringUtilities.*
4 : import java.sql.Timestamp
5 : import java.sql.Time
6 : import java.time.*
7 : // these are in the context by default: ExecutionContext ec, Map<String, Object> context, Map<String, Object> result
8 : 
9 : // begin inline script
10 : import org.moqui.context.ExecutionContext
11 :             import org.moqui.entity.EntityCondition
12 :             ExecutionContext ec = context.ec
13 :             Calendar basisCal = ec.user.getCalendarSafe()
14 :             basisCal.add(Calendar.DAY_OF_YEAR, (int) -daysToKeep)
15 :             basisTimestamp = new Timestamp(basisCal.getTimeInMillis())
16 :             artifactHitsRemoved = ec.entity.find("moqui.server.ArtifactHit")
17 :                     .condition("startDateTime", EntityCondition.LESS_THAN, basisTimestamp)
18 :                     .disableAuthz().deleteAll()
19 :             artifactHitBinsRemoved = ec.entity.find("moqui.server.ArtifactHitBin")
20 :                     .condition("binEndDateTime", EntityCondition.LESS_THAN, basisTimestamp)
21 :                     .disableAuthz().deleteAll()
22 :             ec.logger.info("Removed ${artifactHitsRemoved} ArtifactHit records and ${artifactHitBinsRemoved} ArtifactHitBin records more than ${daysToKeep} days old")
23 : // end inline script
24 : // make sure the last statement is not considered the return value
25 : return;

java.lang.OutOfMemoryError: Java heap space
        at java.util.Arrays.copyOfRange(Arrays.java:4030) ~[?:?]
        at java.lang.StringCoding.decodeUTF8(StringCoding.java:732) ~[?:?]
        at java.lang.StringCoding.decode(StringCoding.java:257) ~[?:?]
        at java.lang.String.<init>(String.java:507) ~[?:?]
        at org.postgresql.core.Encoding.decode(Encoding.java:284) ~[postgresql-jdbc3.jar:42.3.3]
        at org.postgresql.core.Encoding.decode(Encoding.java:295) ~[postgresql-jdbc3.jar:42.3.3]
        at org.postgresql.jdbc.PgResultSet.getString(PgResultSet.java:2200) ~[postgresql-jdbc3.jar:42.3.3]
        at bitronix.tm.resource.jdbc.proxy.ResultSetJavassistProxy.getString(ResultSetJavassistProxy.java) ~[?:?]
        at org.moqui.impl.entity.FieldInfo.getResultSetValue(FieldInfo.java:326) ~[moqui-framework-3.1.0-rc1.jar:3.1.0-rc1]
        at org.moqui.impl.entity.EntityListIteratorImpl.currentEntityValueBase(EntityListIteratorImpl.java:155) ~[moqui-framework-3.1.0-rc1.jar:3.1.0-rc1]
        at org.moqui.impl.entity.EntityListIteratorImpl.next(EntityListIteratorImpl.java:218) ~[moqui-framework-3.1.0-rc1.jar:3.1.0-rc1]
        at org.moqui.impl.entity.EntityListIteratorImpl.getCompleteList(EntityListIteratorImpl.java:276) ~[moqui-framework-3.1.0-rc1.jar:3.1.0-rc1]
        at org.moqui.impl.entity.EntityFindBase.listInternal(EntityFindBase.groovy:1162) ~[moqui-framework-3.1.0-rc1.jar:3.1.0-rc1]
        at org.moqui.impl.entity.EntityFindBase.list(EntityFindBase.groovy:1003) ~[moqui-framework-3.1.0-rc1.jar:3.1.0-rc1]
        at org.moqui.impl.entity.EntityFindBase.deleteAllInternal(EntityFindBase.groovy:1489) ~[moqui-framework-3.1.0-rc1.jar:3.1.0-rc1]
        at org.moqui.impl.entity.EntityFindBase.deleteAll(EntityFindBase.groovy:1471) ~[moqui-framework-3.1.0-rc1.jar:3.1.0-rc1]
        at org_moqui_impl_ServerServices_clean_ArtifactData.run(org_moqui_impl_ServerServices_clean_ArtifactData:16) ~[?:?]
        at org.moqui.impl.actions.XmlAction.run(XmlAction.java:67) ~[moqui-framework-3.1.0-rc1.jar:3.1.0-rc1]
        at org.moqui.impl.service.runner.InlineServiceRunner.runService(InlineServiceRunner.java:59) ~[moqui-framework-3.1.0-rc1.jar:3.1.0-rc1]
        at org.moqui.impl.service.ServiceCallSyncImpl.callSingle(ServiceCallSyncImpl.java:322) ~[moqui-framework-3.1.0-rc1.jar:3.1.0-rc1]
        at org.moqui.impl.service.ServiceCallSyncImpl.call(ServiceCallSyncImpl.java:125) ~[moqui-framework-3.1.0-rc1.jar:3.1.0-rc1]
        at org.moqui.impl.service.ServiceCallJobImpl$ServiceJobCallable.call(ServiceCallJobImpl.groovy:244) ~[moqui-framework-3.1.0-rc1.jar:3.1.0-rc1]
        at java.util.concurrent.FutureTask.run(FutureTask.java:264) ~[?:?]
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128) ~[?:?]
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628) ~[?:?]
        at java.lang.Thread.run(Thread.java:829) ~[?:?]

moqui=> select count(*) from artifact_hit;
  count  
---------
 5380091
(1 row)

moqui=> select count(*) from artifact_hit_bin 
moqui-> ;
 count  
--------
 186515
(1 row)
2

Hi,
It simply means that the JVM ran out of memory. When this occurs, you basically have 2 choices:

  1. Allow the JVM to use more memory using the -Xmx VM argument. For instance, to allow the JVM to use 2 GB (2048 MB) of memory or more.
  2. Improve/Fix the application so that it uses less memory.

In many cases, like in the case of a memory leak, the second option is the only sound choice. A memory leak happens when the application keeps more and more references to objects and never releases them. The garbage collector will therefore never collect those objects and less and less free memory will be available until we reach the point where not enough free memory is available for the application to function normally. At this point, the JVM will throw an OOM.

3

I think @doogie already alluded to the fact that he’s running out of memory, suggesting that perhaps we need to improve the way we’re executing deleteAll.

There is even a comment in the delete all implementation which suggests that the current implementation might not be very efficient. But I’m not sure what’s the best approach here, is it to continue to use the for loop to go over all the values in memory, or perhaps change the design such that the whole thing is converted to a SQL statement that ends up doing something like DELETE FROM ... WHERE ID IN (SELECT...) which defers the whole thing to the database.

1 Like
4

This is not a memory leak. The deleteAll() loop explicitly loads all rows into memory, and there will always be a situation where the memory settings are too small for the number of rows being deleted.

If you read the comments in EntityFindBase, you’ll see that originally it used an iterator(), and then attempted to remove the row, but some JDBC drivers don’t support that. And it can not be passed directly to the database(DELETE FROM WHERE), as ECAs need to be run.

An appropriate fix would be do use the iterator version, but only fetch X items, then close the iterator, remove the items, then continue looping until the iterator returns no more rows. I just don’t have time right now to work on such a patch.

2 Likes
5

Ahh interesting. What if something goes wrong in a deleteAll like a foreign key constraint or something like that? does that mean we’ll have partial deletes and is that OK?

Or is that in the same transaction? if it is I don’t mind looking at the code and try to help out

2 Likes