From 899f67cdb2937a8d209be318ef7caa2a2fdc0cf6 Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Wed, 21 Mar 2018 16:46:29 +0100 Subject: [PATCH] SONAR-10504 commit after each delete of analysis to prevent accumulating too many rows in the transaction. By default 250*(nb of file/developer measures per analysis) are part of the transaction. That is too much, for example for size of Oracle rollback segments. --- .../db/migration/step/MassUpdate.java | 7 +- .../platform/db/migration/step/Upsert.java | 13 +++- .../db/migration/step/UpsertImpl.java | 21 +++++- .../version/v56/PopulateInitialSchema.java | 41 ++++++----- ...IsBuiltInToTrueForDefaultOrganization.java | 4 +- ...eOrgQProfilesToPointToBuiltInProfiles.java | 5 +- .../v70/DeletePersonAndFileMeasures.java | 2 +- .../db/migration/step/DataChangeTest.java | 42 ++++++++++- .../db/migration/step/UpsertImplTest.java | 73 +++++++++++++++++++ 9 files changed, 174 insertions(+), 34 deletions(-) create mode 100644 server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/step/UpsertImplTest.java diff --git a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/step/MassUpdate.java b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/step/MassUpdate.java index 28b77f05c3c..892580efc4b 100644 --- a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/step/MassUpdate.java +++ b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/step/MassUpdate.java @@ -72,9 +72,10 @@ public class MassUpdate { return this.select; } - public MassUpdate update(String sql) throws SQLException { - this.updates.add(UpsertImpl.create(writeConnection, sql)); - return this; + public Upsert update(String sql) throws SQLException { + UpsertImpl upsert = UpsertImpl.create(writeConnection, sql); + this.updates.add(upsert); + return upsert; } public MassUpdate rowPluralName(String s) { diff --git a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/step/Upsert.java b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/step/Upsert.java index 280884537e1..dfb7243b011 100644 --- a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/step/Upsert.java +++ b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/step/Upsert.java @@ -25,9 +25,20 @@ import java.sql.SQLException; * INSERT, UPDATE or DELETE */ public interface Upsert extends SqlStatement { - Upsert addBatch() throws SQLException; + /** + * Prepare for next statement. + * @return {@code true} if the buffer of batched requests has been sent and transaction + * has been committed, else {@code false}. + */ + boolean addBatch() throws SQLException; Upsert execute() throws SQLException; Upsert commit() throws SQLException; + + /** + * Number of requests required before sending group of batched + * requests and before committing. Default value is {@link UpsertImpl#MAX_BATCH_SIZE} + */ + Upsert setBatchSize(int i); } diff --git a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/step/UpsertImpl.java b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/step/UpsertImpl.java index 004ae033db8..a0aaae719d9 100644 --- a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/step/UpsertImpl.java +++ b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/step/UpsertImpl.java @@ -23,10 +23,13 @@ import java.sql.Connection; import java.sql.PreparedStatement; import java.sql.SQLException; +import static com.google.common.base.Preconditions.checkArgument; + public class UpsertImpl extends BaseSqlStatement implements Upsert { private static final int MAX_BATCH_SIZE = 250; + private int maxBatchSize = MAX_BATCH_SIZE; private long batchCount = 0L; private UpsertImpl(PreparedStatement pstmt) { @@ -34,15 +37,27 @@ public class UpsertImpl extends BaseSqlStatement implements Upsert { } @Override - public Upsert addBatch() throws SQLException { + public Upsert setBatchSize(int i) { + checkArgument(i >= 0, "size must be positive. Got %s", i); + this.maxBatchSize = i; + return this; + } + + public int getMaxBatchSize() { + return maxBatchSize; + } + + @Override + public boolean addBatch() throws SQLException { pstmt.addBatch(); pstmt.clearParameters(); batchCount++; - if (batchCount % MAX_BATCH_SIZE == 0L) { + if (batchCount % maxBatchSize == 0L) { pstmt.executeBatch(); pstmt.getConnection().commit(); + return true; } - return this; + return false; } @Override diff --git a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v56/PopulateInitialSchema.java b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v56/PopulateInitialSchema.java index 4a3eeec7e60..7a5a149425a 100644 --- a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v56/PopulateInitialSchema.java +++ b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v56/PopulateInitialSchema.java @@ -24,6 +24,7 @@ import java.util.Date; import org.sonar.api.utils.System2; import org.sonar.db.Database; import org.sonar.server.platform.db.migration.step.DataChange; +import org.sonar.server.platform.db.migration.step.Upsert; public class PopulateInitialSchema extends DataChange { @@ -51,17 +52,18 @@ public class PopulateInitialSchema extends DataChange { truncateTable(context, "groups"); Date now = new Date(system2.now()); - context.prepareUpsert("insert into groups (name, description, created_at, updated_at) values (?, ?, ?, ?)") - .setString(1, ADMINS_GROUP) + Upsert upsert = context.prepareUpsert("insert into groups (name, description, created_at, updated_at) values (?, ?, ?, ?)"); + upsert.setString(1, ADMINS_GROUP) .setString(2, "System administrators") .setDate(3, now) .setDate(4, now) - .addBatch() - .setString(1, USERS_GROUP) + .addBatch(); + upsert.setString(1, USERS_GROUP) .setString(2, "Any new users created will automatically join this group") .setDate(3, now) .setDate(4, now) - .addBatch() + .addBatch(); + upsert .execute() .commit(); } @@ -70,19 +72,21 @@ public class PopulateInitialSchema extends DataChange { truncateTable(context, "group_roles"); // admin group - context.prepareUpsert("insert into group_roles (group_id, resource_id, role) values ((select id from groups where name='" + ADMINS_GROUP + "'), null, ?)") - .setString(1, "admin").addBatch() - .setString(1, "profileadmin").addBatch() - .setString(1, "gateadmin").addBatch() - .setString(1, "shareDashboard").addBatch() - .setString(1, "provisioning").addBatch() + Upsert upsert = context.prepareUpsert("insert into group_roles (group_id, resource_id, role) values ((select id from groups where name='" + ADMINS_GROUP + "'), null, ?)"); + upsert.setString(1, "admin").addBatch(); + upsert.setString(1, "profileadmin").addBatch(); + upsert.setString(1, "gateadmin").addBatch(); + upsert.setString(1, "shareDashboard").addBatch(); + upsert.setString(1, "provisioning").addBatch(); + upsert .execute() .commit(); // anyone - context.prepareUpsert("insert into group_roles (group_id, resource_id, role) values (null, null, ?)") - .setString(1, "scan").addBatch() - .setString(1, "provisioning").addBatch() + upsert = context.prepareUpsert("insert into group_roles (group_id, resource_id, role) values (null, null, ?)"); + upsert.setString(1, "scan").addBatch(); + upsert.setString(1, "provisioning").addBatch(); + upsert .execute() .commit(); } @@ -106,10 +110,11 @@ public class PopulateInitialSchema extends DataChange { private static void insertGroupMemberships(Context context) throws SQLException { truncateTable(context, "groups_users"); - context.prepareUpsert("insert into groups_users (user_id, group_id) values " + - "((select id from users where login='" + ADMIN_USER + "'), (select id from groups where name=?))") - .setString(1, ADMINS_GROUP).addBatch() - .setString(1, USERS_GROUP).addBatch() + Upsert upsert = context.prepareUpsert("insert into groups_users (user_id, group_id) values " + + "((select id from users where login='" + ADMIN_USER + "'), (select id from groups where name=?))"); + upsert.setString(1, ADMINS_GROUP).addBatch(); + upsert.setString(1, USERS_GROUP).addBatch(); + upsert .execute() .commit(); } diff --git a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v65/SetRulesProfilesIsBuiltInToTrueForDefaultOrganization.java b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v65/SetRulesProfilesIsBuiltInToTrueForDefaultOrganization.java index 23c7b1e70cb..fa1ae01d0ca 100644 --- a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v65/SetRulesProfilesIsBuiltInToTrueForDefaultOrganization.java +++ b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v65/SetRulesProfilesIsBuiltInToTrueForDefaultOrganization.java @@ -50,10 +50,10 @@ public class SetRulesProfilesIsBuiltInToTrueForDefaultOrganization extends DataC "inner join org_qprofiles oqp on rp.kee = oqp.rules_profile_uuid " + "where oqp.organization_uuid = ? ") .setString(1, defaultOrganizationUuid); - massUpdate.update("update rules_profiles " + "set is_built_in=? " + - "where kee=?").execute((row, update) -> { + "where kee=?"); + massUpdate.execute((row, update) -> { String rulesProfilesUuid = row.getString(1); update.setBoolean(1, true); update.setString(2, rulesProfilesUuid); diff --git a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v65/UpdateOrgQProfilesToPointToBuiltInProfiles.java b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v65/UpdateOrgQProfilesToPointToBuiltInProfiles.java index b342ab6fabd..28bc6bb049c 100644 --- a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v65/UpdateOrgQProfilesToPointToBuiltInProfiles.java +++ b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v65/UpdateOrgQProfilesToPointToBuiltInProfiles.java @@ -59,8 +59,9 @@ public class UpdateOrgQProfilesToPointToBuiltInProfiles extends DataChange { massUpdate.update("update org_qprofiles " + "set rules_profile_uuid = ? " + - "where uuid=?") - .execute((row, update) -> { + "where uuid=?"); + + massUpdate.execute((row, update) -> { String orgQProfileUuid = row.getString(1); String language = row.getString(2); String name = row.getString(3); diff --git a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v70/DeletePersonAndFileMeasures.java b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v70/DeletePersonAndFileMeasures.java index 67db190fab0..a9de415eb35 100644 --- a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v70/DeletePersonAndFileMeasures.java +++ b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v70/DeletePersonAndFileMeasures.java @@ -49,7 +49,7 @@ public class DeletePersonAndFileMeasures extends DataChange { MassUpdate massUpdate = context.prepareMassUpdate(); massUpdate.select("select uuid from snapshots"); massUpdate.rowPluralName("snapshots"); - massUpdate.update(getDeleteSql()); + massUpdate.update(getDeleteSql()).setBatchSize(1); massUpdate.execute((row, update) -> { String analysisUuid = row.getString(1); diff --git a/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/step/DataChangeTest.java b/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/step/DataChangeTest.java index cad65f9c290..b754d233377 100644 --- a/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/step/DataChangeTest.java +++ b/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/step/DataChangeTest.java @@ -34,7 +34,6 @@ import org.sonar.server.platform.db.migration.step.Select.RowReader; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.fail; - public class DataChangeTest { private static final int MAX_BATCH_SIZE = 250; @@ -205,27 +204,31 @@ public class DataChangeTest { } @Test - public void batch_insert() throws Exception { + public void batch_inserts() throws Exception { db.prepareDbUnit(getClass(), "persons.xml"); new DataChange(db.database()) { @Override public void execute(Context context) throws SQLException { Upsert upsert = context.prepareUpsert("insert into persons(id,login,age,enabled,coeff) values (?,?,?,?,?)"); - upsert + boolean committed = upsert .setLong(1, 10L) .setString(2, "kurt") .setInt(3, 27) .setBoolean(4, true) .setDouble(5, 2.2) .addBatch(); - upsert + assertThat(committed).isFalse(); + + committed = upsert .setLong(1, 11L) .setString(2, "courtney") .setInt(3, 25) .setBoolean(4, false) .setDouble(5, 2.3) .addBatch(); + assertThat(committed).isFalse(); + upsert.execute().commit().close(); } }.execute(); @@ -233,6 +236,37 @@ public class DataChangeTest { db.assertDbUnit(getClass(), "batch-insert-result.xml", "persons"); } + @Test + public void override_size_of_batch_inserts() throws Exception { + new DataChange(db.database()) { + @Override + public void execute(Context context) throws SQLException { + Upsert upsert = context.prepareUpsert("insert into persons(id,login,age,enabled,coeff) values (?,?,?,?,?)") + .setBatchSize(3); + long id = 100L; + assertThat(addBatchInsert(upsert, id++)).isFalse(); + assertThat(addBatchInsert(upsert, id++)).isFalse(); + assertThat(addBatchInsert(upsert, id++)).isTrue(); + assertThat(addBatchInsert(upsert, id++)).isFalse(); + assertThat(addBatchInsert(upsert, id++)).isFalse(); + assertThat(addBatchInsert(upsert, id++)).isTrue(); + assertThat(addBatchInsert(upsert, id)).isFalse(); + upsert.execute().commit().close(); + } + }.execute(); + assertThat(db.countRowsOfTable("persons")).isEqualTo(7); + } + + private boolean addBatchInsert(Upsert upsert, long id) throws SQLException { + return upsert + .setLong(1, id) + .setString(2, "kurt") + .setInt(3, 27) + .setBoolean(4, true) + .setDouble(5, 2.2) + .addBatch(); + } + @Test public void update_null() throws Exception { db.prepareDbUnit(getClass(), "persons.xml"); diff --git a/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/step/UpsertImplTest.java b/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/step/UpsertImplTest.java new file mode 100644 index 00000000000..e6eb78b5d42 --- /dev/null +++ b/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/step/UpsertImplTest.java @@ -0,0 +1,73 @@ +/* + * SonarQube + * Copyright (C) 2009-2018 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program 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. + * + * This program 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.server.platform.db.migration.step; + +import java.sql.Connection; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; + +import static org.assertj.core.api.AssertionsForInterfaceTypes.assertThat; +import static org.mockito.Mockito.mock; + +public class UpsertImplTest { + + @Rule + public ExpectedException expectedException = ExpectedException.none(); + + @Test + public void setBatchSize_throws_IAE_if_value_is_negative() throws Exception { + UpsertImpl underTest = create(); + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("size must be positive. Got -1"); + + underTest.setBatchSize(-1); + } + + @Test + public void setBatchSize_accepts_zero() throws Exception { + UpsertImpl underTest = create(); + + underTest.setBatchSize(0); + + assertThat(underTest.getMaxBatchSize()).isEqualTo(0); + } + + @Test + public void setBatchSize_accepts_strictly_positive_value() throws Exception { + UpsertImpl underTest = create(); + + underTest.setBatchSize(42); + + assertThat(underTest.getMaxBatchSize()).isEqualTo(42); + } + + @Test + public void maxBatchSize_is_250_by_default() throws Exception { + UpsertImpl underTest = create(); + + assertThat(underTest.getMaxBatchSize()).isEqualTo(250); + } + + private UpsertImpl create() throws Exception { + return UpsertImpl.create(mock(Connection.class), "sql"); + } +} -- 2.39.5