From 530bfa982e36d80b929f902aa5d34db7831a0947 Mon Sep 17 00:00:00 2001 From: Rob Bygrave Date: Thu, 5 Jan 2023 23:27:22 +1300 Subject: [PATCH] Fix memory leak with streaming queries with Id Embedded - The Id Embedded beans do not need the embeddedOwner to be set. They do not need to propagate dirty state back to the owner. --- .../server/deploy/BeanProperty.java | 2 +- .../server/deploy/BeanPropertyAssocOne.java | 8 +- .../tests/basic/TestPersistenceContext.java | 161 +++++++++++++++--- 3 files changed, 142 insertions(+), 29 deletions(-) diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanProperty.java b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanProperty.java index 8c1b208df..97d1b790e 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanProperty.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanProperty.java @@ -65,7 +65,7 @@ public class BeanProperty implements ElPropertyValue, Property, STreeProperty { /** * Flag to mark this is the id property. */ - private final boolean id; + protected final boolean id; private final boolean importedPrimaryKey; /** * Flag to make this as a dummy property for unidirecitonal relationships. diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocOne.java b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocOne.java index b97c0a75a..a6a706cc6 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocOne.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocOne.java @@ -655,7 +655,7 @@ public class BeanPropertyAssocOne extends BeanPropertyAssoc implements STr @Override public void setValue(EntityBean bean, Object value) { super.setValue(bean, value); - if (embedded && value instanceof EntityBean) { + if (!id && embedded && value instanceof EntityBean) { setEmbeddedOwner(bean, value); } } @@ -663,7 +663,7 @@ public class BeanPropertyAssocOne extends BeanPropertyAssoc implements STr @Override public void setValueIntercept(EntityBean bean, Object value) { super.setValueIntercept(bean, value); - if (embedded && value instanceof EntityBean) { + if (!id && embedded && value instanceof EntityBean) { setEmbeddedOwner(bean, value); } } @@ -675,8 +675,10 @@ public class BeanPropertyAssocOne extends BeanPropertyAssoc implements STr Object emb = getValue(owner); if (emb != null) { EntityBean embeddedBean = (EntityBean) emb; - embeddedBean._ebean_getIntercept().setEmbeddedOwner(owner, propertyIndex); targetDescriptor.setAllLoaded(embeddedBean); + if (!id) { + embeddedBean._ebean_getIntercept().setEmbeddedOwner(owner, propertyIndex); + } } } diff --git a/ebean-test/src/test/java/org/tests/basic/TestPersistenceContext.java b/ebean-test/src/test/java/org/tests/basic/TestPersistenceContext.java index 6312cfcf8..98b746240 100644 --- a/ebean-test/src/test/java/org/tests/basic/TestPersistenceContext.java +++ b/ebean-test/src/test/java/org/tests/basic/TestPersistenceContext.java @@ -1,8 +1,11 @@ package org.tests.basic; -import io.ebean.xtest.BaseTestCase; import io.ebean.DB; +import io.ebean.DatabaseFactory; +import io.ebean.QueryIterator; import io.ebean.Transaction; +import io.ebean.config.DatabaseConfig; +import io.ebean.xtest.BaseTestCase; import io.ebeaninternal.api.SpiPersistenceContext; import io.ebeaninternal.api.SpiTransaction; import org.junit.jupiter.api.Disabled; @@ -12,15 +15,17 @@ import org.tests.model.basic.Customer; import org.tests.model.basic.Order; import org.tests.model.basic.ResetBasicData; -import java.util.ArrayList; -import java.util.List; -import java.util.WeakHashMap; +import javax.persistence.Embeddable; +import javax.persistence.EmbeddedId; +import javax.persistence.Entity; +import javax.validation.constraints.Size; +import java.util.*; import java.util.concurrent.atomic.AtomicInteger; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.*; -class TestPersistenceContext extends BaseTestCase { +public class TestPersistenceContext extends BaseTestCase { @Test void testReload() { @@ -191,29 +196,135 @@ class TestPersistenceContext extends BaseTestCase { } } - @Disabled // run manually - @Test - void testPcScopes_with_findEachFindList() { - for (int i = 0; i < 5000; i++) { - Customer c = new Customer(); - c.setName("Customer #" + i); - DB.save(c); - Order o = new Order(); - o.setCustomer(c); - DB.save(o); + @Embeddable + public static class TmId { + private final UUID id1; + private final UUID id2; + + public TmId(UUID id1, UUID id2) { + this.id1 = id1; + this.id2 = id2; } - for (int i = 0; i < 1000; i++) { - try (Transaction txn = DB.beginTransaction()) { - List customers = new ArrayList<>(); - DB.find(Customer.class).select("id").findEach(customers::add); - SpiPersistenceContext pc = ((SpiTransaction) txn).getPersistenceContext(); - assertThat(pc.toString()).contains("Customer=size:5000 (5000 weak)"); - customers.clear(); - customers = DB.find(Customer.class).select("id").findList(); + public UUID getId1() { + return id1; + } - assertThat(pc.toString()).contains("Customer=size:5000"); // We expect ALWAYS 5000 entries in the PC - } + public UUID getId2() { + return id2; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + TmId tmId = (TmId) o; + return Objects.equals(id1, tmId.id1) && Objects.equals(id2, tmId.id2); + } + + @Override + public int hashCode() { + return Objects.hash(id1, id2); } } + + @Entity + // @Cache(enableQueryCache = true, enableBeanCache = false) + public static class TestModel2 { + + @EmbeddedId + private TmId id; + @Size(max = 255) + private String someData; + + public String getSomeData() { + return someData; + } + + public void setSomeData(String someData) { + this.someData = someData; + } + + public TmId getId() { + return id; + } + + public void setId(TmId id) { + this.id = id; + } + } + + @Test + @Disabled + void initDb() { + DatabaseConfig config = new DatabaseConfig(); + config.setName("h2-batch"); + config.loadFromProperties(); + config.setDdlExtra(false); + config.getDataSourceConfig().setUsername("sa"); + config.getDataSourceConfig().setPassword("sa"); + config.getDataSourceConfig().setUrl("jdbc:h2:file:./testsFile3;DB_CLOSE_ON_EXIT=FALSE;NON_KEYWORDS=KEY,VALUE"); + config.addClass(TestModel2.class); + config.addClass(TmId.class); + DatabaseFactory.create(config); + + String base = "x".repeat(240); + // 10 mio TestModel - each needs about 1/4 kbytes -> 2,5 GB in total + List batch = new ArrayList<>(); + for (int i = 0; i < 1_000_000; i++) { + TestModel2 m = new TestModel2(); + TmId id = new TmId(UUID.randomUUID(), UUID.randomUUID()); + m.setId(id); + m.setSomeData(base + i); // ensure we have not duplicates + batch.add(m); + if (i % 1000 == 0) { + DB.saveAll(batch); + batch.clear(); + } + if (i % 100000 == 0) { + System.out.println(i); + } + } + DB.saveAll(batch); + } + + @Test + @Disabled + void testFindEachFindList() { + DatabaseConfig config = new DatabaseConfig(); + config.setName("h2-batch"); + config.loadFromProperties(); + config.setDdlRun(false); + config.getDataSourceConfig().setUsername("sa"); + config.getDataSourceConfig().setPassword("sa"); + config.getDataSourceConfig().setUrl("jdbc:h2:file:./testsFile3;DB_CLOSE_ON_EXIT=FALSE;NON_KEYWORDS=KEY,VALUE"); + config.addClass(TestModel2.class); + config.addClass(TmId.class); + DatabaseFactory.create(config); + + AtomicInteger i = new AtomicInteger(); + System.out.println("Doing findEach"); + DB.find(TestModel2.class).select("*").findEach(c -> { + i.incrementAndGet(); + }); + System.out.println("Read " + i + " entries"); + + i.set(0); + System.out.println("Doing findStream"); + DB.find(TestModel2.class).select("*").findStream().forEach(c -> i.incrementAndGet()); + System.out.println("Read " + i + " entries"); + + i.set(0); + System.out.println("Doing findIterate"); + QueryIterator iter = DB.find(TestModel2.class).select("*").findIterate(); + while (iter.hasNext()) { + iter.next(); + i.incrementAndGet(); + } + System.out.println("Read " + i + " entries"); + + System.out.println("Doing FindList, will hold all entries in memory. Expect OOM with -Xmx100m."); + List lst = DB.find(TestModel2.class).select("*").findList(); + System.out.println("Read " + lst.size() + " entries"); + } }