From: simonbrandhof Date: Wed, 22 Dec 2010 22:43:49 +0000 (+0000) Subject: fix memory leak in ruby database migration X-Git-Tag: 2.6~239 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=2366419df66b6a46e9be173ee71debff338f2f03;p=sonarqube.git fix memory leak in ruby database migration --- diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/platform/ServerUpgradeStatus.java b/sonar-plugin-api/src/main/java/org/sonar/api/platform/ServerUpgradeStatus.java index 68f5c2bab6d..dad047afa70 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/platform/ServerUpgradeStatus.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/platform/ServerUpgradeStatus.java @@ -32,9 +32,9 @@ public interface ServerUpgradeStatus extends ServerComponent { boolean isUpgraded(); /** - * Has the database been created during the current startup ? + * Has the database been created from scratch during the current startup ? */ - boolean isInstalledFromScratch(); + boolean isFreshInstall(); /** * The database version before the server startup. Returns <=0 if db created from scratch. diff --git a/sonar-server/src/main/java/org/sonar/server/platform/DefaultServerUpgradeStatus.java b/sonar-server/src/main/java/org/sonar/server/platform/DefaultServerUpgradeStatus.java index 545f2c336f7..0230b53708e 100644 --- a/sonar-server/src/main/java/org/sonar/server/platform/DefaultServerUpgradeStatus.java +++ b/sonar-server/src/main/java/org/sonar/server/platform/DefaultServerUpgradeStatus.java @@ -42,10 +42,10 @@ public final class DefaultServerUpgradeStatus implements ServerUpgradeStatus { } public boolean isUpgraded() { - return !isInstalledFromScratch() &&(initialDbVersion < SchemaMigration.LAST_VERSION); + return !isFreshInstall() &&(initialDbVersion < SchemaMigration.LAST_VERSION); } - public boolean isInstalledFromScratch() { + public boolean isFreshInstall() { return initialDbVersion <= 0; } diff --git a/sonar-server/src/main/java/org/sonar/server/platform/Platform.java b/sonar-server/src/main/java/org/sonar/server/platform/Platform.java index 302ff2a2fc7..3850ae6d799 100644 --- a/sonar-server/src/main/java/org/sonar/server/platform/Platform.java +++ b/sonar-server/src/main/java/org/sonar/server/platform/Platform.java @@ -28,7 +28,6 @@ import org.sonar.api.Plugins; import org.sonar.api.database.configuration.DatabaseConfiguration; import org.sonar.api.platform.Environment; import org.sonar.api.platform.Server; -import org.sonar.api.platform.ServerUpgradeStatus; import org.sonar.api.profiles.AnnotationProfileParser; import org.sonar.api.profiles.XMLProfileParser; import org.sonar.api.profiles.XMLProfileSerializer; @@ -205,6 +204,7 @@ public final class Platform { startupContainer.as(Characteristics.CACHE).addComponent(JdbcDriverDeployer.class); startupContainer.as(Characteristics.CACHE).addComponent(ServerMetadataPersister.class); startupContainer.as(Characteristics.CACHE).addComponent(RegisterQualityModels.class); + startupContainer.as(Characteristics.CACHE).addComponent(DeleteDeprecatedMeasures.class); startupContainer.start(); startupContainer.getComponent(ServerLifecycleNotifier.class).notifyStart(); diff --git a/sonar-server/src/main/java/org/sonar/server/startup/DeleteDeprecatedMeasures.java b/sonar-server/src/main/java/org/sonar/server/startup/DeleteDeprecatedMeasures.java index 97c6a3f4912..42f23331663 100644 --- a/sonar-server/src/main/java/org/sonar/server/startup/DeleteDeprecatedMeasures.java +++ b/sonar-server/src/main/java/org/sonar/server/startup/DeleteDeprecatedMeasures.java @@ -19,10 +19,61 @@ */ package org.sonar.server.startup; +import org.slf4j.LoggerFactory; +import org.sonar.api.database.DatabaseSession; +import org.sonar.api.database.model.MeasureModel; +import org.sonar.api.platform.ServerUpgradeStatus; +import org.sonar.api.utils.Logs; import org.sonar.jpa.session.DatabaseSessionFactory; -public class DeleteDeprecatedMeasures { - public DeleteDeprecatedMeasures(DatabaseSessionFactory sessionFactory) { +import javax.persistence.Query; +import java.util.List; +/** + * This purge script can not be executed with ActiveRecord. It fails with an out of memory error. + * + * @since 2.5 this component could be removed after 4 or 5 releases. + */ +public final class DeleteDeprecatedMeasures { + + private ServerUpgradeStatus status; + private DatabaseSessionFactory sessionFactory; + private static final int MAX_IN_ELEMENTS = 500; + + public DeleteDeprecatedMeasures(DatabaseSessionFactory sessionFactory, ServerUpgradeStatus status) { + this.sessionFactory = sessionFactory; + this.status = status; + } + + public void start() { + if (mustDoPurge()) { + doPurge(); + } + } + + boolean mustDoPurge() { + return status.isUpgraded() && status.getInitialDbVersion()<=162; + } + + void doPurge() { + Logs.INFO.info("Deleting measures with deprecated ISO category"); + deleteRows("SELECT m.id FROM " + MeasureModel.class.getSimpleName() + " m WHERE m.ruleId IS NULL AND m.rulesCategoryId IS NOT NULL"); + + Logs.INFO.info("Deleting measures with deprecated priority"); + deleteRows("SELECT m.id FROM " + MeasureModel.class.getSimpleName() + " m WHERE m.ruleId IS NULL AND m.rulePriority IS NOT NULL"); + } + + private void deleteRows(String hql) { + DatabaseSession session = sessionFactory.getSession(); + List ids = session.getEntityManager().createQuery(hql).getResultList(); + int index = 0; + while (index < ids.size()) { + List paginedSids = ids.subList(index, Math.min(ids.size(), index + MAX_IN_ELEMENTS)); + Query query = session.createQuery("DELETE FROM " + MeasureModel.class.getSimpleName() + " WHERE id IN (:ids)"); + query.setParameter("ids", paginedSids); + query.executeUpdate(); + index += MAX_IN_ELEMENTS; + session.commit(); + } } } diff --git a/sonar-server/src/main/webapp/WEB-INF/db/migrate/161_add_snapshots_period_columns.rb b/sonar-server/src/main/webapp/WEB-INF/db/migrate/161_add_snapshots_period_columns.rb index a0d0945d22b..34783863e6c 100644 --- a/sonar-server/src/main/webapp/WEB-INF/db/migrate/161_add_snapshots_period_columns.rb +++ b/sonar-server/src/main/webapp/WEB-INF/db/migrate/161_add_snapshots_period_columns.rb @@ -24,25 +24,33 @@ class AddSnapshotsPeriodColumns < ActiveRecord::Migration def self.up - add_column :snapshots, :period1_mode, :string, :null => true, :limit => 100 - add_column :snapshots, :period1_param, :string, :null => true, :limit => 100 - add_column :snapshots, :period1_date, :datetime, :null => true + Snapshot.reset_column_information() + + add_period_column('period1_mode', :string, :null => true, :limit => 100) + add_period_column('period1_param', :string, :null => true, :limit => 100) + add_period_column('period1_date', :datetime, :null => true) - add_column :snapshots, :period2_mode, :string, :null => true, :limit => 100 - add_column :snapshots, :period2_param, :string, :null => true, :limit => 100 - add_column :snapshots, :period2_date, :datetime, :null => true + add_period_column('period2_mode', :string, :null => true, :limit => 100) + add_period_column('period2_param', :string, :null => true, :limit => 100) + add_period_column('period2_date', :datetime, :null => true) - add_column :snapshots, :period3_mode, :string, :null => true, :limit => 100 - add_column :snapshots, :period3_param, :string, :null => true, :limit => 100 - add_column :snapshots, :period3_date, :datetime, :null => true + add_period_column('period3_mode', :string, :null => true, :limit => 100) + add_period_column('period3_param', :string, :null => true, :limit => 100) + add_period_column('period3_date', :datetime, :null => true) - add_column :snapshots, :period4_mode, :string, :null => true, :limit => 100 - add_column :snapshots, :period4_param, :string, :null => true, :limit => 100 - add_column :snapshots, :period4_date, :datetime, :null => true + add_period_column('period4_mode', :string, :null => true, :limit => 100) + add_period_column('period4_param', :string, :null => true, :limit => 100) + add_period_column('period4_date', :datetime, :null => true) - add_column :snapshots, :period5_mode, :string, :null => true, :limit => 100 - add_column :snapshots, :period5_param, :string, :null => true, :limit => 100 - add_column :snapshots, :period5_date, :datetime, :null => true + add_period_column('period5_mode', :string, :null => true, :limit => 100) + add_period_column('period5_param', :string, :null => true, :limit => 100) + add_period_column('period5_date', :datetime, :null => true) end + private + def self.add_period_column(name, type, options={}) + unless Snapshot.column_names.include?(name) + add_column(:snapshots, name, type, options) + end + end end diff --git a/sonar-server/src/main/webapp/WEB-INF/db/migrate/162_delete_iso_rule_categories.rb b/sonar-server/src/main/webapp/WEB-INF/db/migrate/162_delete_iso_rule_categories.rb index 2b426cb6520..f9cebf73c1e 100644 --- a/sonar-server/src/main/webapp/WEB-INF/db/migrate/162_delete_iso_rule_categories.rb +++ b/sonar-server/src/main/webapp/WEB-INF/db/migrate/162_delete_iso_rule_categories.rb @@ -24,13 +24,6 @@ class DeleteIsoRuleCategories < ActiveRecord::Migration def self.up - remove_rule_categories - delete_measures_on_iso_category - end - - private - - def self.remove_rule_categories begin remove_column('rules', 'rules_category_id') rescue @@ -38,10 +31,4 @@ class DeleteIsoRuleCategories < ActiveRecord::Migration end end - def self.delete_measures_on_iso_category - ids=ProjectMeasure.connection.select_values("SELECT ID FROM PROJECT_MEASURES WHERE RULE_ID IS NULL AND RULES_CATEGORY_ID IS NOT NULL") - ids.in_groups_of(900, false) do |group| - ProjectMeasure.delete(group.map{|id| id.to_i}) unless group.empty? - end - end end diff --git a/sonar-server/src/main/webapp/WEB-INF/db/migrate/164_delete_measures_on_violations_and_priority.rb b/sonar-server/src/main/webapp/WEB-INF/db/migrate/164_delete_measures_on_violations_and_priority.rb deleted file mode 100644 index 56f2febf0e5..00000000000 --- a/sonar-server/src/main/webapp/WEB-INF/db/migrate/164_delete_measures_on_violations_and_priority.rb +++ /dev/null @@ -1,33 +0,0 @@ -# -# Sonar, entreprise quality control tool. -# Copyright (C) 2009 SonarSource SA -# mailto:contact AT sonarsource DOT com -# -# Sonar 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. -# -# Sonar 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 Sonar; if not, write to the Free Software -# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02 -# - -# -# Sonar 2.5 -# -class DeleteMeasuresOnViolationsAndPriority < ActiveRecord::Migration - - def self.up - ids=ProjectMeasure.connection.select_values("SELECT ID FROM PROJECT_MEASURES WHERE RULE_ID IS NULL AND RULE_PRIORITY IS NOT NULL") - ids.in_groups_of(900, false) do |group| - ProjectMeasure.delete(group.map{|id| id.to_i}) unless group.empty? - end - end - -end diff --git a/sonar-server/src/test/java/org/sonar/server/platform/DefaultServerUpgradeStatusTest.java b/sonar-server/src/test/java/org/sonar/server/platform/DefaultServerUpgradeStatusTest.java index be801d2ac83..ecfe58793c4 100644 --- a/sonar-server/src/test/java/org/sonar/server/platform/DefaultServerUpgradeStatusTest.java +++ b/sonar-server/src/test/java/org/sonar/server/platform/DefaultServerUpgradeStatusTest.java @@ -31,14 +31,14 @@ import static org.mockito.Mockito.when; public class DefaultServerUpgradeStatusTest { @Test - public void shouldBeInstalledFromScratch() { + public void shouldBeFreshInstallation() { DatabaseConnector connector = mock(DatabaseConnector.class); when(connector.getDatabaseVersion()).thenReturn(-1); DefaultServerUpgradeStatus status = new DefaultServerUpgradeStatus(connector); status.start(); - assertThat(status.isInstalledFromScratch(), is(true)); + assertThat(status.isFreshInstall(), is(true)); assertThat(status.isUpgraded(), is(false)); assertThat(status.getInitialDbVersion(), is(-1)); } @@ -51,7 +51,7 @@ public class DefaultServerUpgradeStatusTest { DefaultServerUpgradeStatus status = new DefaultServerUpgradeStatus(connector); status.start(); - assertThat(status.isInstalledFromScratch(), is(false)); + assertThat(status.isFreshInstall(), is(false)); assertThat(status.isUpgraded(), is(true)); assertThat(status.getInitialDbVersion(), is(50)); } @@ -64,7 +64,7 @@ public class DefaultServerUpgradeStatusTest { DefaultServerUpgradeStatus status = new DefaultServerUpgradeStatus(connector); status.start(); - assertThat(status.isInstalledFromScratch(), is(false)); + assertThat(status.isFreshInstall(), is(false)); assertThat(status.isUpgraded(), is(false)); assertThat(status.getInitialDbVersion(), is(SchemaMigration.LAST_VERSION)); } diff --git a/sonar-server/src/test/java/org/sonar/server/startup/DeleteDeprecatedMeasuresTest.java b/sonar-server/src/test/java/org/sonar/server/startup/DeleteDeprecatedMeasuresTest.java new file mode 100644 index 00000000000..438058322de --- /dev/null +++ b/sonar-server/src/test/java/org/sonar/server/startup/DeleteDeprecatedMeasuresTest.java @@ -0,0 +1,106 @@ +/* + * Sonar, open source software quality management tool. + * Copyright (C) 2009 SonarSource SA + * mailto:contact AT sonarsource DOT com + * + * Sonar 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. + * + * Sonar 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 Sonar; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02 + */ +package org.sonar.server.startup; + +import org.junit.Test; +import org.sonar.api.database.model.MeasureModel; +import org.sonar.api.platform.ServerUpgradeStatus; +import org.sonar.jpa.entity.SchemaMigration; +import org.sonar.jpa.test.AbstractDbUnitTestCase; + +import java.sql.SQLException; +import java.util.List; + +import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertThat; +import static org.junit.internal.matchers.IsCollectionContaining.hasItems; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class DeleteDeprecatedMeasuresTest extends AbstractDbUnitTestCase { + + @Test + public void shouldDeleteMeasuresWithCategory() throws SQLException { + setupData("sharedFixture"); + + ServerUpgradeStatus upgradeStatus = mock(ServerUpgradeStatus.class); + + final DeleteDeprecatedMeasures purge = new DeleteDeprecatedMeasures(getSessionFactory(), upgradeStatus); + purge.doPurge(); + + List rows = getSession().createQuery("from " + MeasureModel.class.getSimpleName() + " where ruleId is null and rulesCategoryId is not null").getResultList(); + assertThat(rows.size(), is(0)); + } + + @Test + public void shouldDeleteMeasuresWithPriority() throws SQLException { + setupData("sharedFixture"); + + ServerUpgradeStatus upgradeStatus = mock(ServerUpgradeStatus.class); + final DeleteDeprecatedMeasures purge = new DeleteDeprecatedMeasures(getSessionFactory(), upgradeStatus); + purge.doPurge(); + + List rowsToDelete = getSession().createQuery("from " + MeasureModel.class.getSimpleName() + " where ruleId is null and rulePriority is not null").getResultList(); + assertThat(rowsToDelete.size(), is(0)); + + List rowIdsToKeep = getSession().createQuery("select id from " + MeasureModel.class.getSimpleName()).getResultList(); + assertThat(rowIdsToKeep, hasItems(1L, 2L, 4L, 6L)); + } + + @Test + public void shouldDoPurgeOnUpgradeBefore162() { + ServerUpgradeStatus upgradeStatus = mock(ServerUpgradeStatus.class); + when(upgradeStatus.isUpgraded()).thenReturn(true); + when(upgradeStatus.getInitialDbVersion()).thenReturn(50); + + final DeleteDeprecatedMeasures purge = new DeleteDeprecatedMeasures(getSessionFactory(), upgradeStatus); + assertThat(purge.mustDoPurge(), is(true)); + } + + @Test + public void shouldNotDoPurgeOnUpgradeAfter162() { + ServerUpgradeStatus upgradeStatus = mock(ServerUpgradeStatus.class); + when(upgradeStatus.isUpgraded()).thenReturn(true); + when(upgradeStatus.getInitialDbVersion()).thenReturn(170); + + final DeleteDeprecatedMeasures purge = new DeleteDeprecatedMeasures(getSessionFactory(), upgradeStatus); + assertThat(purge.mustDoPurge(), is(false)); + } + + @Test + public void shouldNotDoPurgeOnFreshInstall() { + ServerUpgradeStatus upgradeStatus = mock(ServerUpgradeStatus.class); + when(upgradeStatus.isUpgraded()).thenReturn(false); + when(upgradeStatus.getInitialDbVersion()).thenReturn(-1); + + final DeleteDeprecatedMeasures purge = new DeleteDeprecatedMeasures(getSessionFactory(), upgradeStatus); + assertThat(purge.mustDoPurge(), is(false)); + } + + @Test + public void shouldNotDoPurgeOnStandardStartup() { + ServerUpgradeStatus upgradeStatus = mock(ServerUpgradeStatus.class); + when(upgradeStatus.isUpgraded()).thenReturn(false); + when(upgradeStatus.getInitialDbVersion()).thenReturn(SchemaMigration.LAST_VERSION); + + final DeleteDeprecatedMeasures purge = new DeleteDeprecatedMeasures(getSessionFactory(), upgradeStatus); + assertThat(purge.mustDoPurge(), is(false)); + } +} diff --git a/sonar-server/src/test/resources/org/sonar/server/startup/DeleteDeprecatedMeasuresTest/sharedFixture.xml b/sonar-server/src/test/resources/org/sonar/server/startup/DeleteDeprecatedMeasuresTest/sharedFixture.xml new file mode 100644 index 00000000000..870e3a48412 --- /dev/null +++ b/sonar-server/src/test/resources/org/sonar/server/startup/DeleteDeprecatedMeasuresTest/sharedFixture.xml @@ -0,0 +1,52 @@ + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file