From dd7ca50b28422f8728cf26b096d22dced32863cf Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Thu, 21 Jul 2016 16:49:23 +0200 Subject: [PATCH] Fix performance issue in CE when many files have being deleted/moved When updating columns PROJECTS.B_ENABLED to false, as many SQL UPDATE requests as components to be disabled are executed, whereas only a single request with "UUID IN ?" could be executed. --- .../step/PersistComponentsStep.java | 17 ++- .../main/java/org/sonar/db/DatabaseUtils.java | 14 +++ .../org/sonar/db/component/ComponentDao.java | 5 + .../sonar/db/component/ComponentMapper.java | 3 +- .../sonar/db/component/ComponentMapper.xml | 18 +++ .../java/org/sonar/db/DatabaseUtilsTest.java | 51 +++++--- .../sonar/db/component/ComponentDaoTest.java | 115 +++++++++++++----- 7 files changed, 163 insertions(+), 60 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/step/PersistComponentsStep.java b/server/sonar-server/src/main/java/org/sonar/server/computation/step/PersistComponentsStep.java index e096849ab81..7acfeb52768 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/step/PersistComponentsStep.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/step/PersistComponentsStep.java @@ -24,8 +24,8 @@ import java.util.Collection; import java.util.Date; import java.util.Map; import java.util.Optional; +import java.util.Set; import java.util.function.Function; -import java.util.stream.Collectors; import javax.annotation.Nonnull; import org.apache.commons.io.FilenameUtils; import org.apache.commons.lang.StringUtils; @@ -102,15 +102,12 @@ public class PersistComponentsStep implements ComputationStep { } private void disableRemainingComponents(DbSession dbSession, Collection dtos) { - dtos.stream() + Set uuids = dtos.stream() .filter(ComponentDto::isEnabled) - .forEach(c -> { - ComponentUpdateDto update = ComponentUpdateDto.copyFrom(c) - .setBChanged(true) - .setBEnabled(false); - dbClient.componentDao().update(dbSession, update); - }); - disabledComponentsHolder.setUuids(dtos.stream().map(ComponentDto::uuid).collect(GuavaCollectors.toList(dtos.size()))); + .map(ComponentDto::uuid) + .collect(GuavaCollectors.toSet(dtos.size())); + dbClient.componentDao().updateBEnabledToFalse(dbSession, uuids); + disabledComponentsHolder.setUuids(uuids); } /** @@ -120,7 +117,7 @@ public class PersistComponentsStep implements ComputationStep { private Map indexExistingDtosByKey(DbSession session) { return dbClient.componentDao().selectAllComponentsFromProjectKey(session, treeRootHolder.getRoot().getKey()) .stream() - .collect(Collectors.toMap(ComponentDto::key, Function.identity())); + .collect(java.util.stream.Collectors.toMap(ComponentDto::key, Function.identity())); } private class PersistComponentStepsVisitor extends PathAwareVisitorAdapter { diff --git a/sonar-db/src/main/java/org/sonar/db/DatabaseUtils.java b/sonar-db/src/main/java/org/sonar/db/DatabaseUtils.java index 5977fd73642..3e459468880 100644 --- a/sonar-db/src/main/java/org/sonar/db/DatabaseUtils.java +++ b/sonar-db/src/main/java/org/sonar/db/DatabaseUtils.java @@ -35,6 +35,7 @@ import java.util.Collections; import java.util.Date; import java.util.List; import java.util.Set; +import java.util.function.Consumer; import javax.annotation.CheckForNull; import javax.annotation.Nullable; import org.sonar.api.utils.log.Logger; @@ -135,6 +136,19 @@ public class DatabaseUtils { return results; } + /** + * Partition by 1000 elements a list of input and execute a consumer on each part. + * + * The goal is to prevent issue with ORACLE when there's more than 1000 elements in a 'in ('X', 'Y', ...)' + * and with MsSQL when there's more than 2000 parameters in a query + */ + public static > void executeLargeUpdates(Collection inputs, Consumer> consumer) { + Iterable> partitions = toUniqueAndSortedPartitions(inputs); + for (List partition : partitions) { + consumer.accept(partition); + } + } + /** * Ensure values {@code inputs} are unique (which avoids useless arguments) and sorted before creating the partition. */ diff --git a/sonar-db/src/main/java/org/sonar/db/component/ComponentDao.java b/sonar-db/src/main/java/org/sonar/db/component/ComponentDao.java index af24457c602..fd5fe9dc652 100644 --- a/sonar-db/src/main/java/org/sonar/db/component/ComponentDao.java +++ b/sonar-db/src/main/java/org/sonar/db/component/ComponentDao.java @@ -44,6 +44,7 @@ import static com.google.common.collect.Maps.newHashMapWithExpectedSize; import static java.util.Collections.emptyList; import static org.sonar.api.utils.Paging.offset; import static org.sonar.db.DatabaseUtils.executeLargeInputs; +import static org.sonar.db.DatabaseUtils.executeLargeUpdates; public class ComponentDao implements Dao { @@ -349,6 +350,10 @@ public class ComponentDao implements Dao { mapper(session).update(component); } + public void updateBEnabledToFalse(DbSession session, Collection uuids) { + executeLargeUpdates(uuids, mapper(session)::updateBEnabledToFalse); + } + public void applyBChangesForRootComponentUuid(DbSession session, String projectUuid) { mapper(session).applyBChangesForRootComponentUuid(projectUuid); } diff --git a/sonar-db/src/main/java/org/sonar/db/component/ComponentMapper.java b/sonar-db/src/main/java/org/sonar/db/component/ComponentMapper.java index 74d4b1c92b0..027a67fe5a1 100644 --- a/sonar-db/src/main/java/org/sonar/db/component/ComponentMapper.java +++ b/sonar-db/src/main/java/org/sonar/db/component/ComponentMapper.java @@ -143,10 +143,11 @@ public interface ComponentMapper { void update(ComponentUpdateDto component); + void updateBEnabledToFalse(@Param("uuids") List uuids); + void applyBChangesForRootComponentUuid(@Param("projectUuid") String projectUuid); void resetBChangedForRootComponentUuid(@Param("projectUuid") String projectUuid); void delete(long componentId); - } diff --git a/sonar-db/src/main/resources/org/sonar/db/component/ComponentMapper.xml b/sonar-db/src/main/resources/org/sonar/db/component/ComponentMapper.xml index 919dffbf62b..c1dece9ad81 100644 --- a/sonar-db/src/main/resources/org/sonar/db/component/ComponentMapper.xml +++ b/sonar-db/src/main/resources/org/sonar/db/component/ComponentMapper.xml @@ -608,6 +608,24 @@ uuid = #{uuid} + + update projects set + b_changed = ${_true}, + b_copy_component_uuid = copy_component_uuid, + b_description = description, + b_enabled = ${_false}, + b_language = language, + b_long_name = long_name, + b_module_uuid = module_uuid, + b_module_uuid_path = module_uuid_path, + b_name = name, + b_path = path, + b_qualifier = qualifier + where + uuid in #{uuid} + + + update projects set copy_component_uuid = b_copy_component_uuid, diff --git a/sonar-db/src/test/java/org/sonar/db/DatabaseUtilsTest.java b/sonar-db/src/test/java/org/sonar/db/DatabaseUtilsTest.java index b529c18ebd6..1855803dcc3 100644 --- a/sonar-db/src/test/java/org/sonar/db/DatabaseUtilsTest.java +++ b/sonar-db/src/test/java/org/sonar/db/DatabaseUtilsTest.java @@ -20,7 +20,6 @@ package org.sonar.db; import com.google.common.base.Function; -import com.google.common.collect.Iterables; import java.sql.Connection; import java.sql.PreparedStatement; import java.sql.ResultSet; @@ -38,6 +37,7 @@ import org.sonar.api.utils.System2; import org.sonar.api.utils.log.LogTester; import org.sonar.api.utils.log.LoggerLevel; import org.sonar.api.utils.log.Loggers; +import org.sonar.core.util.stream.Collectors; import org.sonar.db.dialect.Oracle; import static com.google.common.collect.Lists.newArrayList; @@ -53,7 +53,6 @@ import static org.sonar.db.WildcardPosition.AFTER; import static org.sonar.db.WildcardPosition.BEFORE; import static org.sonar.db.WildcardPosition.BEFORE_AND_AFTER; - public class DatabaseUtilsTest { @Rule @@ -167,8 +166,8 @@ public class DatabaseUtilsTest { public void toUniqueAndSortedList_removes_duplicates_and_apply_natural_order_of_any_Comparable() { assertThat( toUniqueAndSortedList(asList(myComparable(2), myComparable(5), myComparable(2), myComparable(4), myComparable(-1), myComparable(10)))) - .containsExactly( - myComparable(-1), myComparable(2), myComparable(4), myComparable(5), myComparable(10)); + .containsExactly( + myComparable(-1), myComparable(2), myComparable(4), myComparable(5), myComparable(10)); } private static DatabaseUtilsTest.MyComparable myComparable(int ordinal) { @@ -256,7 +255,7 @@ public class DatabaseUtilsTest { } @Test - public void execute_large_inputs() { + public void executeLargeInputs() { List inputs = newArrayList(); List expectedOutputs = newArrayList(); for (int i = 0; i < 2010; i++) { @@ -264,26 +263,18 @@ public class DatabaseUtilsTest { expectedOutputs.add(Integer.toString(i)); } - List outputs = DatabaseUtils.executeLargeInputs(inputs, new Function, List>() { - @Override - public List apply(List input) { - // Check that each partition is only done on 1000 elements max - assertThat(input.size()).isLessThanOrEqualTo(1000); - return newArrayList(Iterables.transform(input, new Function() { - @Override - public String apply(Integer input) { - return Integer.toString(input); - } - })); - } + List outputs = DatabaseUtils.executeLargeInputs(inputs, input -> { + // Check that each partition is only done on 1000 elements max + assertThat(input.size()).isLessThanOrEqualTo(1000); + return input.stream().map(String::valueOf).collect(Collectors.toList()); }); assertThat(outputs).isEqualTo(expectedOutputs); } @Test - public void execute_large_inputs_on_empty_list() { - List outputs = DatabaseUtils.executeLargeInputs(Collections.emptyList(), new Function, List>() { + public void executeLargeInputs_on_empty_list() { + List outputs = DatabaseUtils.executeLargeInputs(Collections.emptyList(), new Function, List>() { @Override public List apply(List input) { fail("No partition should be made on empty list"); @@ -294,6 +285,28 @@ public class DatabaseUtilsTest { assertThat(outputs).isEmpty(); } + @Test + public void executeLargeUpdates() { + List inputs = newArrayList(); + for (int i = 0; i < 2010; i++) { + inputs.add(i); + } + + List processed = newArrayList(); + DatabaseUtils.executeLargeUpdates(inputs, input -> { + assertThat(input.size()).isLessThanOrEqualTo(1000); + processed.addAll(input); + }); + assertThat(processed).containsExactlyElementsOf(inputs); + } + + @Test + public void executeLargeUpdates_on_empty_list() { + DatabaseUtils.executeLargeUpdates(Collections.emptyList(), input -> { + fail("No partition should be made on empty list"); + }); + } + @Test public void log_all_sql_exceptions() { SQLException root = new SQLException("this is root", "123"); diff --git a/sonar-db/src/test/java/org/sonar/db/component/ComponentDaoTest.java b/sonar-db/src/test/java/org/sonar/db/component/ComponentDaoTest.java index 9623de54754..f54b6165162 100644 --- a/sonar-db/src/test/java/org/sonar/db/component/ComponentDaoTest.java +++ b/sonar-db/src/test/java/org/sonar/db/component/ComponentDaoTest.java @@ -22,6 +22,7 @@ package org.sonar.db.component; import com.google.common.base.Optional; import java.util.Collections; import java.util.List; +import java.util.Map; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; @@ -657,36 +658,90 @@ public class ComponentDaoTest { db.assertDbUnit(getClass(), "insert_disabled_component-result.xml", "projects"); } - // FIXME -// @Test -// public void update() { -// db.prepareDbUnit(getClass(), "update.xml"); -// -// ComponentDto componentDto = new ComponentDto() -// .setUuid("GHIJ") -// .setProjectUuid("DCBA") -// .setModuleUuid("HGFE") -// .setModuleUuidPath(".DCBA.HGFE.") -// .setKey("org.struts:struts-core:src/org/struts/RequestContext2.java") -// .setDeprecatedKey("org.struts:struts-core:src/org/struts/RequestContext2.java") -// .setName("RequestContext2.java") -// .setLongName("org.struts.RequestContext2") -// .setQualifier("LIF") -// .setScope("LIF") -// .setLanguage("java2") -// .setDescription("description2") -// .setPath("src/org/struts/RequestContext2.java") -// .setRootUuid("uuid_4") -// .setCopyComponentUuid("uuid_6") -// .setDeveloperUuid("uuid_9") -// .setEnabled(false) -// .setAuthorizationUpdatedAt(12345678910L); -// -// underTest.update(dbSession, componentDto); -// dbSession.commit(); -// -// db.assertDbUnit(getClass(), "update-result.xml", "projects"); -// } + @Test + public void update() { + ComponentDto dto = ComponentTesting.newProjectDto("U1"); + underTest.insert(dbSession, dto); + dbSession.commit(); + + underTest.update(dbSession, new ComponentUpdateDto() + .setUuid("U1") + .setBCopyComponentUuid("copy") + .setBChanged(true) + .setBDescription("desc") + .setBEnabled(true) + .setBLanguage("lang") + .setBLongName("longName") + .setBModuleUuid("moduleUuid") + .setBModuleUuidPath("moduleUuidPath") + .setBName("name") + .setBPath("path") + .setBQualifier("qualifier") + ); + dbSession.commit(); + + Map row = selectBColumnsForUuid("U1"); + assertThat(row.get("bChanged")).isIn(true, /* for Oracle */ 1L, 1); + assertThat(row.get("bCopyComponentUuid")).isEqualTo("copy"); + assertThat(row.get("bDescription")).isEqualTo("desc"); + assertThat(row.get("bEnabled")).isIn(true, /* for Oracle */ 1L, 1); + assertThat(row.get("bLanguage")).isEqualTo("lang"); + assertThat(row.get("bLongName")).isEqualTo("longName"); + assertThat(row.get("bModuleUuid")).isEqualTo("moduleUuid"); + assertThat(row.get("bModuleUuidPath")).isEqualTo("moduleUuidPath"); + assertThat(row.get("bName")).isEqualTo("name"); + assertThat(row.get("bPath")).isEqualTo("path"); + assertThat(row.get("bQualifier")).isEqualTo("qualifier"); + } + + @Test + public void updateBEnabledToFalse() { + ComponentDto dto1 = ComponentTesting.newProjectDto("U1"); + ComponentDto dto2 = ComponentTesting.newProjectDto("U2"); + ComponentDto dto3 = ComponentTesting.newProjectDto("U3"); + underTest.insert(dbSession, dto1, dto2, dto3); + + underTest.updateBEnabledToFalse(dbSession, asList("U1", "U2")); + dbSession.commit(); + + Map row1 = selectBColumnsForUuid("U1"); + assertThat(row1.get("bChanged")).isIn(true, /* for Oracle */ 1L, 1); + assertThat(row1.get("bCopyComponentUuid")).isEqualTo(dto1.getCopyResourceUuid()); + assertThat(row1.get("bDescription")).isEqualTo(dto1.description()); + assertThat(row1.get("bEnabled")).isIn(false, /* for Oracle */ 0L, 0); + assertThat(row1.get("bLanguage")).isEqualTo(dto1.language()); + assertThat(row1.get("bLongName")).isEqualTo(dto1.longName()); + assertThat(row1.get("bModuleUuid")).isEqualTo(dto1.moduleUuid()); + assertThat(row1.get("bModuleUuidPath")).isEqualTo(dto1.moduleUuidPath()); + assertThat(row1.get("bName")).isEqualTo(dto1.name()); + assertThat(row1.get("bPath")).isEqualTo(dto1.path()); + assertThat(row1.get("bQualifier")).isEqualTo(dto1.qualifier()); + + Map row2 = selectBColumnsForUuid("U2"); + assertThat(row2.get("bChanged")).isIn(true, /* for Oracle */ 1L, 1); + assertThat(row2.get("bCopyComponentUuid")).isEqualTo(dto2.getCopyResourceUuid()); + assertThat(row2.get("bDescription")).isEqualTo(dto2.description()); + assertThat(row2.get("bEnabled")).isIn(false, /* for Oracle */ 0L, 0); + assertThat(row2.get("bLanguage")).isEqualTo(dto2.language()); + assertThat(row2.get("bLongName")).isEqualTo(dto2.longName()); + assertThat(row2.get("bModuleUuid")).isEqualTo(dto2.moduleUuid()); + assertThat(row2.get("bModuleUuidPath")).isEqualTo(dto2.moduleUuidPath()); + assertThat(row2.get("bName")).isEqualTo(dto2.name()); + assertThat(row2.get("bPath")).isEqualTo(dto2.path()); + assertThat(row2.get("bQualifier")).isEqualTo(dto2.qualifier()); + + Map row3 = selectBColumnsForUuid("U3"); + assertThat(row3.get("bChanged")).isIn(false, /* for Oracle */ 0L, 0); + } + + private Map selectBColumnsForUuid(String uuid) { + return db.selectFirst( + "select b_changed as \"bChanged\", b_copy_component_uuid as \"bCopyComponentUuid\", b_description as \"bDescription\", " + + "b_enabled as \"bEnabled\", b_language as \"bLanguage\", b_long_name as \"bLongName\"," + + "b_module_uuid as \"bModuleUuid\", b_module_uuid_path as \"bModuleUuidPath\", b_name as \"bName\", " + + "b_path as \"bPath\", b_qualifier as \"bQualifier\" " + + "from projects where uuid='" + uuid + "'"); + } @Test public void delete() throws Exception { -- 2.39.5