]> source.dussan.org Git - sonarqube.git/commitdiff
Fix performance issue in CE when many files have being deleted/moved
authorSimon Brandhof <simon.brandhof@sonarsource.com>
Thu, 21 Jul 2016 14:49:23 +0000 (16:49 +0200)
committerSimon Brandhof <simon.brandhof@sonarsource.com>
Fri, 22 Jul 2016 07:25:13 +0000 (09:25 +0200)
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.

server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/step/PersistComponentsStep.java
sonar-db/src/main/java/org/sonar/db/DatabaseUtils.java
sonar-db/src/main/java/org/sonar/db/component/ComponentDao.java
sonar-db/src/main/java/org/sonar/db/component/ComponentMapper.java
sonar-db/src/main/resources/org/sonar/db/component/ComponentMapper.xml
sonar-db/src/test/java/org/sonar/db/DatabaseUtilsTest.java
sonar-db/src/test/java/org/sonar/db/component/ComponentDaoTest.java

index 424021b4e3431e5416a96272477bf5d6ccf48d20..71d9ac9affb5ebeaf37d01c8dff5f0f1f6bb4c03 100644 (file)
@@ -24,14 +24,15 @@ 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;
 import org.sonar.api.resources.Qualifiers;
 import org.sonar.api.resources.Scopes;
 import org.sonar.api.utils.System2;
+import org.sonar.core.util.stream.Collectors;
 import org.sonar.db.DbClient;
 import org.sonar.db.DbSession;
 import org.sonar.db.component.ComponentDto;
@@ -48,7 +49,6 @@ import org.sonar.server.computation.task.projectanalysis.component.TreeRootHolde
 import org.sonar.server.computation.task.step.ComputationStep;
 
 import static com.google.common.collect.FluentIterable.from;
-import static org.sonar.core.util.stream.Collectors.toList;
 import static org.sonar.db.component.ComponentDto.UUID_PATH_SEPARATOR;
 import static org.sonar.db.component.ComponentDto.formatUuidPathFromParent;
 import static org.sonar.server.computation.task.projectanalysis.component.ComponentVisitor.Order.PRE_ORDER;
@@ -103,15 +103,12 @@ public class PersistComponentsStep implements ComputationStep {
   }
 
   private void disableRemainingComponents(DbSession dbSession, Collection<ComponentDto> dtos) {
-    dtos.stream()
+    Set<String> 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(toList(dtos.size())));
+      .map(ComponentDto::uuid)
+      .collect(Collectors.toSet(dtos.size()));
+    dbClient.componentDao().updateBEnabledToFalse(dbSession, uuids);
+    disabledComponentsHolder.setUuids(uuids);
   }
 
   /**
@@ -121,7 +118,7 @@ public class PersistComponentsStep implements ComputationStep {
   private Map<String, ComponentDto> 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<ComponentDtoHolder> {
index 5977fd7364224408e6238aeb3971bab335e02a4a..3e45946888067ea69a5585f4fbe5c7c65360d9d0 100644 (file)
@@ -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 <INPUT extends Comparable<INPUT>> void executeLargeUpdates(Collection<INPUT> inputs, Consumer<List<INPUT>> consumer) {
+    Iterable<List<INPUT>> partitions = toUniqueAndSortedPartitions(inputs);
+    for (List<INPUT> partition : partitions) {
+      consumer.accept(partition);
+    }
+  }
+
   /**
    * Ensure values {@code inputs} are unique (which avoids useless arguments) and sorted before creating the partition.
    */
index af24457c602371f42df43a7ff43862d853ba1639..fd5fe9dc6524e699b317eef930ec9f4798d92710 100644 (file)
@@ -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<String> uuids) {
+    executeLargeUpdates(uuids, mapper(session)::updateBEnabledToFalse);
+  }
+
   public void applyBChangesForRootComponentUuid(DbSession session, String projectUuid) {
     mapper(session).applyBChangesForRootComponentUuid(projectUuid);
   }
index 74d4b1c92b033eddb05ff6cbda7afa2d7a78153c..027a67fe5a10e3e1b206e895ba88317e13e4b8ae 100644 (file)
@@ -143,10 +143,11 @@ public interface ComponentMapper {
 
   void update(ComponentUpdateDto component);
 
+  void updateBEnabledToFalse(@Param("uuids") List<String> uuids);
+
   void applyBChangesForRootComponentUuid(@Param("projectUuid") String projectUuid);
 
   void resetBChangedForRootComponentUuid(@Param("projectUuid") String projectUuid);
 
   void delete(long componentId);
-
 }
index 919dffbf62b68c851b145ee496766a8ab2724f6e..c1dece9ad819b0a1d400107efba68e7347955649 100644 (file)
     uuid = #{uuid}
   </update>
 
+  <update id="updateBEnabledToFalse" parameterType="org.sonar.db.component.ComponentUpdateDto" useGeneratedKeys="false">
+    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 <foreach collection="uuids" open="(" close=")" item="uuid" separator=",">#{uuid}</foreach>
+  </update>
+
+
   <update id="applyBChangesForRootComponentUuid" parameterType="string" useGeneratedKeys="false">
     update projects set
     copy_component_uuid = b_copy_component_uuid,
index b529c18ebd69f4fe315cce12b02583885e237418..1855803dcc328115d398e502ab93de29f1f7e78b 100644 (file)
@@ -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<Integer> inputs = newArrayList();
     List<String> expectedOutputs = newArrayList();
     for (int i = 0; i < 2010; i++) {
@@ -264,26 +263,18 @@ public class DatabaseUtilsTest {
       expectedOutputs.add(Integer.toString(i));
     }
 
-    List<String> outputs = DatabaseUtils.executeLargeInputs(inputs, new Function<List<Integer>, List<String>>() {
-      @Override
-      public List<String> apply(List<Integer> input) {
-        // Check that each partition is only done on 1000 elements max
-        assertThat(input.size()).isLessThanOrEqualTo(1000);
-        return newArrayList(Iterables.transform(input, new Function<Integer, String>() {
-          @Override
-          public String apply(Integer input) {
-            return Integer.toString(input);
-          }
-        }));
-      }
+    List<String> 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<String> outputs = DatabaseUtils.executeLargeInputs(Collections.<Integer>emptyList(), new Function<List<Integer>, List<String>>() {
+  public void executeLargeInputs_on_empty_list() {
+    List<String> outputs = DatabaseUtils.executeLargeInputs(Collections.emptyList(), new Function<List<Integer>, List<String>>() {
       @Override
       public List<String> apply(List<Integer> 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<Integer> inputs = newArrayList();
+    for (int i = 0; i < 2010; i++) {
+      inputs.add(i);
+    }
+
+    List<Integer> 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.<Integer>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");
index 9623de547546784dbf6e0bf11e8025ced0fa8130..f54b6165162ec06eb95085e006f71ac22a3eb99a 100644 (file)
@@ -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<String, Object> 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<String, Object> 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<String, Object> 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<String, Object> row3 = selectBColumnsForUuid("U3");
+    assertThat(row3.get("bChanged")).isIn(false, /* for Oracle */ 0L, 0);
+  }
+
+  private Map<String, Object> 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 {