From 43081095702af0eb3d12ebf1815fac884ca474bb Mon Sep 17 00:00:00 2001 From: Julien HENRY Date: Thu, 31 Jul 2014 15:14:56 +0200 Subject: [PATCH] Don't do batch insert for duplication in order to avoid OOM error --- .../DuplicationGroupValueCoder.java | 4 +- .../batch/index/DuplicationPersister.java | 107 ++++++++++++++++++ .../sonar/batch/index/MeasurePersister.java | 53 +-------- .../batch/scan/ProjectScanContainer.java | 2 + .../batch/index/DuplicationPersisterTest.java | 93 +++++++++++++++ .../batch/index/MeasurePersisterTest.java | 9 +- .../index/DuplicationPersisterTest/empty.xml | 2 + .../shouldInsertDuplication-result.xml | 13 +++ 8 files changed, 223 insertions(+), 60 deletions(-) create mode 100644 sonar-batch/src/main/java/org/sonar/batch/index/DuplicationPersister.java create mode 100644 sonar-batch/src/test/java/org/sonar/batch/index/DuplicationPersisterTest.java create mode 100644 sonar-batch/src/test/resources/org/sonar/batch/index/DuplicationPersisterTest/empty.xml create mode 100644 sonar-batch/src/test/resources/org/sonar/batch/index/DuplicationPersisterTest/shouldInsertDuplication-result.xml diff --git a/sonar-batch/src/main/java/org/sonar/batch/duplication/DuplicationGroupValueCoder.java b/sonar-batch/src/main/java/org/sonar/batch/duplication/DuplicationGroupValueCoder.java index 244cffccd5f..b813d728354 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/duplication/DuplicationGroupValueCoder.java +++ b/sonar-batch/src/main/java/org/sonar/batch/duplication/DuplicationGroupValueCoder.java @@ -33,7 +33,7 @@ class DuplicationGroupValueCoder implements ValueCoder { @Override public void put(Value value, Object object, CoderContext context) { DuplicationGroup c = (DuplicationGroup) object; - value.put(c.originBlock()); + blockCoder.put(value, c.originBlock(), context); value.put(c.duplicates().size()); for (DuplicationGroup.Block block : c.duplicates()) { blockCoder.put(value, block, context); @@ -42,7 +42,7 @@ class DuplicationGroupValueCoder implements ValueCoder { @Override public Object get(Value value, Class clazz, CoderContext context) { - DuplicationGroup g = new DuplicationGroup((DuplicationGroup.Block) value.get()); + DuplicationGroup g = new DuplicationGroup((Block) blockCoder.get(value, DuplicationGroup.Block.class, context)); int count = value.getInt(); ArrayList blocks = new ArrayList(count); for (int i = 0; i < count; i++) { diff --git a/sonar-batch/src/main/java/org/sonar/batch/index/DuplicationPersister.java b/sonar-batch/src/main/java/org/sonar/batch/index/DuplicationPersister.java new file mode 100644 index 00000000000..9ac3085880c --- /dev/null +++ b/sonar-batch/src/main/java/org/sonar/batch/index/DuplicationPersister.java @@ -0,0 +1,107 @@ +/* + * SonarQube, open source software quality management tool. + * Copyright (C) 2008-2014 SonarSource + * mailto:contact AT sonarsource DOT com + * + * SonarQube is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * SonarQube is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.batch.index; + +import org.apache.commons.lang.StringEscapeUtils; +import org.sonar.api.database.model.MeasureMapper; +import org.sonar.api.database.model.MeasureModel; +import org.sonar.api.database.model.Snapshot; +import org.sonar.api.measures.CoreMetrics; +import org.sonar.api.measures.Measure; +import org.sonar.api.measures.PersistenceMode; +import org.sonar.api.resources.Resource; +import org.sonar.api.rules.RuleFinder; +import org.sonar.batch.duplication.DuplicationCache; +import org.sonar.batch.duplication.DuplicationGroup; +import org.sonar.batch.index.Cache.Entry; +import org.sonar.core.persistence.DbSession; +import org.sonar.core.persistence.MyBatis; + +import java.util.ArrayList; + +public final class DuplicationPersister implements ScanPersister { + private final MyBatis mybatis; + private final RuleFinder ruleFinder; + private final SnapshotCache snapshotCache; + private final ResourceCache resourceCache; + private final DuplicationCache duplicationCache; + private final org.sonar.api.measures.MetricFinder metricFinder; + + public DuplicationPersister(MyBatis mybatis, RuleFinder ruleFinder, + SnapshotCache snapshotCache, ResourceCache resourceCache, + DuplicationCache duplicationCache, org.sonar.api.measures.MetricFinder metricFinder) { + this.mybatis = mybatis; + this.ruleFinder = ruleFinder; + this.snapshotCache = snapshotCache; + this.resourceCache = resourceCache; + this.duplicationCache = duplicationCache; + this.metricFinder = metricFinder; + } + + @Override + public void persist() { + // Don't use batch insert for duplications since keeping all data in memory can produce OOM + DbSession session = mybatis.openSession(false); + try { + MeasureMapper mapper = session.getMapper(MeasureMapper.class); + org.sonar.api.measures.Metric duplicationMetricWithId = metricFinder.findByKey(CoreMetrics.DUPLICATIONS_DATA_KEY); + for (Entry> entry : duplicationCache.entries()) { + String effectiveKey = entry.key()[0].toString(); + Measure measure = new Measure(duplicationMetricWithId, toXml(entry.value())).setPersistenceMode(PersistenceMode.DATABASE); + Resource resource = resourceCache.get(effectiveKey); + + if (MeasurePersister.shouldPersistMeasure(resource, measure)) { + Snapshot snapshot = snapshotCache.get(effectiveKey); + MeasureModel measureModel = MeasurePersister.model(measure, ruleFinder).setSnapshotId(snapshot.getId()); + mapper.insert(measureModel); + } + } + + session.commit(); + } catch (Exception e) { + throw new IllegalStateException("Unable to save some measures", e); + } finally { + MyBatis.closeQuietly(session); + } + } + + private static String toXml(Iterable duplications) { + StringBuilder xml = new StringBuilder(); + xml.append(""); + for (DuplicationGroup duplication : duplications) { + xml.append(""); + toXml(xml, duplication.originBlock()); + for (DuplicationGroup.Block part : duplication.duplicates()) { + toXml(xml, part); + } + xml.append(""); + } + xml.append(""); + return xml.toString(); + } + + private static void toXml(StringBuilder xml, DuplicationGroup.Block part) { + xml.append(""); + } + +} diff --git a/sonar-batch/src/main/java/org/sonar/batch/index/MeasurePersister.java b/sonar-batch/src/main/java/org/sonar/batch/index/MeasurePersister.java index 0f7691728a1..6c382abfa1a 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/index/MeasurePersister.java +++ b/sonar-batch/src/main/java/org/sonar/batch/index/MeasurePersister.java @@ -20,13 +20,10 @@ package org.sonar.batch.index; import com.google.common.annotations.VisibleForTesting; -import org.apache.commons.lang.StringEscapeUtils; import org.sonar.api.database.model.MeasureMapper; import org.sonar.api.database.model.MeasureModel; import org.sonar.api.database.model.Snapshot; -import org.sonar.api.measures.CoreMetrics; import org.sonar.api.measures.Measure; -import org.sonar.api.measures.PersistenceMode; import org.sonar.api.measures.RuleMeasure; import org.sonar.api.resources.Resource; import org.sonar.api.resources.ResourceUtils; @@ -34,8 +31,6 @@ import org.sonar.api.rule.RuleKey; import org.sonar.api.rules.Rule; import org.sonar.api.rules.RuleFinder; import org.sonar.api.technicaldebt.batch.Characteristic; -import org.sonar.batch.duplication.DuplicationCache; -import org.sonar.batch.duplication.DuplicationGroup; import org.sonar.batch.index.Cache.Entry; import org.sonar.batch.scan.measure.MeasureCache; import org.sonar.core.persistence.DbSession; @@ -43,27 +38,20 @@ import org.sonar.core.persistence.MyBatis; import javax.annotation.Nullable; -import java.util.ArrayList; - public final class MeasurePersister implements ScanPersister { private final MyBatis mybatis; private final RuleFinder ruleFinder; private final MeasureCache measureCache; private final SnapshotCache snapshotCache; private final ResourceCache resourceCache; - private final DuplicationCache duplicationCache; - private final org.sonar.api.measures.MetricFinder metricFinder; public MeasurePersister(MyBatis mybatis, RuleFinder ruleFinder, - MeasureCache measureCache, SnapshotCache snapshotCache, ResourceCache resourceCache, - DuplicationCache duplicationCache, org.sonar.api.measures.MetricFinder metricFinder) { + MeasureCache measureCache, SnapshotCache snapshotCache, ResourceCache resourceCache) { this.mybatis = mybatis; this.ruleFinder = ruleFinder; this.measureCache = measureCache; this.snapshotCache = snapshotCache; this.resourceCache = resourceCache; - this.duplicationCache = duplicationCache; - this.metricFinder = metricFinder; } @Override @@ -79,20 +67,7 @@ public final class MeasurePersister implements ScanPersister { if (shouldPersistMeasure(resource, measure)) { Snapshot snapshot = snapshotCache.get(effectiveKey); - MeasureModel measureModel = model(measure).setSnapshotId(snapshot.getId()); - mapper.insert(measureModel); - } - } - - org.sonar.api.measures.Metric duplicationMetricWithId = metricFinder.findByKey(CoreMetrics.DUPLICATIONS_DATA_KEY); - for (Entry> entry : duplicationCache.entries()) { - String effectiveKey = entry.key()[0].toString(); - Measure measure = new Measure(duplicationMetricWithId, toXml(entry.value())).setPersistenceMode(PersistenceMode.DATABASE); - Resource resource = resourceCache.get(effectiveKey); - - if (shouldPersistMeasure(resource, measure)) { - Snapshot snapshot = snapshotCache.get(effectiveKey); - MeasureModel measureModel = model(measure).setSnapshotId(snapshot.getId()); + MeasureModel measureModel = model(measure, ruleFinder).setSnapshotId(snapshot.getId()); mapper.insert(measureModel); } } @@ -105,28 +80,6 @@ public final class MeasurePersister implements ScanPersister { } } - private static String toXml(Iterable duplications) { - StringBuilder xml = new StringBuilder(); - xml.append(""); - for (DuplicationGroup duplication : duplications) { - xml.append(""); - toXml(xml, duplication.originBlock()); - for (DuplicationGroup.Block part : duplication.duplicates()) { - toXml(xml, part); - } - xml.append(""); - } - xml.append(""); - return xml.toString(); - } - - private static void toXml(StringBuilder xml, DuplicationGroup.Block part) { - xml.append(""); - } - @VisibleForTesting static boolean shouldPersistMeasure(@Nullable Resource resource, @Nullable Measure measure) { if (resource == null || measure == null) { @@ -145,7 +98,7 @@ public final class MeasurePersister implements ScanPersister { || isNotEmpty; } - private MeasureModel model(Measure measure) { + static MeasureModel model(Measure measure, RuleFinder ruleFinder) { MeasureModel model = new MeasureModel(); // we assume that the index has updated the metric model.setMetricId(measure.getMetric().getId()); diff --git a/sonar-batch/src/main/java/org/sonar/batch/scan/ProjectScanContainer.java b/sonar-batch/src/main/java/org/sonar/batch/scan/ProjectScanContainer.java index 3a209b91961..6a115108a0b 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/scan/ProjectScanContainer.java +++ b/sonar-batch/src/main/java/org/sonar/batch/scan/ProjectScanContainer.java @@ -51,6 +51,7 @@ import org.sonar.batch.index.DefaultIndex; import org.sonar.batch.index.DefaultPersistenceManager; import org.sonar.batch.index.DefaultResourcePersister; import org.sonar.batch.index.DependencyPersister; +import org.sonar.batch.index.DuplicationPersister; import org.sonar.batch.index.EventPersister; import org.sonar.batch.index.LinkPersister; import org.sonar.batch.index.MeasurePersister; @@ -136,6 +137,7 @@ public class ProjectScanContainer extends ComponentContainer { EventPersister.class, LinkPersister.class, MeasurePersister.class, + DuplicationPersister.class, DefaultResourcePersister.class, SourcePersister.class, DefaultNotificationManager.class, diff --git a/sonar-batch/src/test/java/org/sonar/batch/index/DuplicationPersisterTest.java b/sonar-batch/src/test/java/org/sonar/batch/index/DuplicationPersisterTest.java new file mode 100644 index 00000000000..1860732e80d --- /dev/null +++ b/sonar-batch/src/test/java/org/sonar/batch/index/DuplicationPersisterTest.java @@ -0,0 +1,93 @@ +/* + * SonarQube, open source software quality management tool. + * Copyright (C) 2008-2014 SonarSource + * mailto:contact AT sonarsource DOT com + * + * SonarQube is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * SonarQube is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.batch.index; + +import org.junit.Before; +import org.junit.Test; +import org.junit.rules.ExpectedException; +import org.sonar.api.database.model.Snapshot; +import org.sonar.api.measures.CoreMetrics; +import org.sonar.api.measures.MetricFinder; +import org.sonar.api.resources.File; +import org.sonar.api.rules.RuleFinder; +import org.sonar.batch.duplication.DuplicationCache; +import org.sonar.batch.duplication.DuplicationGroup; +import org.sonar.core.persistence.AbstractDaoTestCase; + +import java.util.ArrayList; +import java.util.Arrays; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class DuplicationPersisterTest extends AbstractDaoTestCase { + + @org.junit.Rule + public ExpectedException thrown = ExpectedException.none(); + + public static final int FILE_SNAPSHOT_ID = 3003; + + DuplicationPersister duplicationPersister; + RuleFinder ruleFinder = mock(RuleFinder.class); + File aFile = new File("org/foo/Bar.java"); + Snapshot fileSnapshot = snapshot(FILE_SNAPSHOT_ID); + SnapshotCache snapshotCache; + + private DuplicationCache duplicationCache; + + @Before + public void mockResourcePersister() { + duplicationCache = mock(DuplicationCache.class); + + snapshotCache = mock(SnapshotCache.class); + ResourceCache resourceCache = mock(ResourceCache.class); + when(snapshotCache.get("foo:org/foo/Bar.java")).thenReturn(fileSnapshot); + when(resourceCache.get("foo:org/foo/Bar.java")).thenReturn(aFile); + + MetricFinder metricFinder = mock(MetricFinder.class); + when(metricFinder.findByKey(CoreMetrics.DUPLICATIONS_DATA_KEY)).thenReturn(CoreMetrics.DUPLICATIONS_DATA.setId(2)); + + duplicationPersister = new DuplicationPersister(getMyBatis(), ruleFinder, snapshotCache, resourceCache, duplicationCache, metricFinder); + } + + @Test + public void should_insert_duplications() { + setupData("empty"); + + DuplicationGroup.Block originBlock = new DuplicationGroup.Block("foo:org/foo/Bar.java", 1, 4); + + DuplicationGroup group = new DuplicationGroup(originBlock) + .addDuplicate(new DuplicationGroup.Block("foo:org/foo/Foo.java", 5, 9)); + + when(duplicationCache.entries()).thenReturn( + Arrays.>>asList(new Cache.Entry(new String[] {"foo:org/foo/Bar.java"}, Arrays.asList(group)))); + + duplicationPersister.persist(); + + checkTables("shouldInsertDuplication", "project_measures"); + } + + private static Snapshot snapshot(int id) { + Snapshot snapshot = mock(Snapshot.class); + when(snapshot.getId()).thenReturn(id); + return snapshot; + } + +} diff --git a/sonar-batch/src/test/java/org/sonar/batch/index/MeasurePersisterTest.java b/sonar-batch/src/test/java/org/sonar/batch/index/MeasurePersisterTest.java index dfed349e696..fe8a1a45a65 100644 --- a/sonar-batch/src/test/java/org/sonar/batch/index/MeasurePersisterTest.java +++ b/sonar-batch/src/test/java/org/sonar/batch/index/MeasurePersisterTest.java @@ -27,7 +27,6 @@ import org.sonar.api.database.model.Snapshot; import org.sonar.api.measures.CoreMetrics; import org.sonar.api.measures.Measure; import org.sonar.api.measures.Metric; -import org.sonar.api.measures.MetricFinder; import org.sonar.api.measures.PersistenceMode; import org.sonar.api.measures.RuleMeasure; import org.sonar.api.resources.Directory; @@ -36,14 +35,10 @@ import org.sonar.api.resources.Project; import org.sonar.api.rules.Rule; import org.sonar.api.rules.RuleFinder; import org.sonar.api.rules.RulePriority; -import org.sonar.batch.duplication.DuplicationCache; -import org.sonar.batch.duplication.DuplicationGroup; import org.sonar.batch.scan.measure.MeasureCache; import org.sonar.core.persistence.AbstractDaoTestCase; -import java.util.ArrayList; import java.util.Arrays; -import java.util.Collections; import static org.fest.assertions.Assertions.assertThat; import static org.mockito.Mockito.mock; @@ -75,8 +70,6 @@ public class MeasurePersisterTest extends AbstractDaoTestCase { public void mockResourcePersister() { snapshotCache = mock(SnapshotCache.class); measureCache = mock(MeasureCache.class); - DuplicationCache duplicationCache = mock(DuplicationCache.class); - when(duplicationCache.entries()).thenReturn(Collections.>>emptyList()); ResourceCache resourceCache = mock(ResourceCache.class); when(snapshotCache.get("foo")).thenReturn(projectSnapshot); when(snapshotCache.get("foo:org/foo")).thenReturn(packageSnapshot); @@ -84,7 +77,7 @@ public class MeasurePersisterTest extends AbstractDaoTestCase { when(resourceCache.get("foo:org/foo/Bar.java")).thenReturn(aFile); when(resourceCache.get("foo:org/foo")).thenReturn(aDirectory); - measurePersister = new MeasurePersister(getMyBatis(), ruleFinder, measureCache, snapshotCache, resourceCache, duplicationCache, mock(MetricFinder.class)); + measurePersister = new MeasurePersister(getMyBatis(), ruleFinder, measureCache, snapshotCache, resourceCache); } @Test diff --git a/sonar-batch/src/test/resources/org/sonar/batch/index/DuplicationPersisterTest/empty.xml b/sonar-batch/src/test/resources/org/sonar/batch/index/DuplicationPersisterTest/empty.xml new file mode 100644 index 00000000000..fb0854fccbe --- /dev/null +++ b/sonar-batch/src/test/resources/org/sonar/batch/index/DuplicationPersisterTest/empty.xml @@ -0,0 +1,2 @@ + + diff --git a/sonar-batch/src/test/resources/org/sonar/batch/index/DuplicationPersisterTest/shouldInsertDuplication-result.xml b/sonar-batch/src/test/resources/org/sonar/batch/index/DuplicationPersisterTest/shouldInsertDuplication-result.xml new file mode 100644 index 00000000000..f6c6e26ce4e --- /dev/null +++ b/sonar-batch/src/test/resources/org/sonar/batch/index/DuplicationPersisterTest/shouldInsertDuplication-result.xml @@ -0,0 +1,13 @@ + + + + + + -- 2.39.5