SONAR-10462 Postgres deadlock when updating last used date of Q profile

6 files changed, 87 insertions, 44 deletions

diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/UpdateQualityProfilesLastUsedDateStep.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/UpdateQualityProfilesLastUsedDateStep.java
index 1a3e6a23490..d270aa777ed 100644
--- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/UpdateQualityProfilesLastUsedDateStep.java
+++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/UpdateQualityProfilesLastUsedDateStep.java
@@ -19,11 +19,8 @@
  */
 package org.sonar.ce.task.projectanalysis.step;
 
-import java.util.ArrayList;
-import java.util.List;
 import java.util.Optional;
 import java.util.Set;
-import java.util.stream.Collectors;
 import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolder;
 import org.sonar.ce.task.projectanalysis.component.Component;
 import org.sonar.ce.task.projectanalysis.component.TreeRootHolder;
@@ -50,7 +47,7 @@ public class UpdateQualityProfilesLastUsedDateStep implements ComputationStep {
   private final MeasureRepository measureRepository;
 
   public UpdateQualityProfilesLastUsedDateStep(DbClient dbClient, AnalysisMetadataHolder analysisMetadataHolder, TreeRootHolder treeRootHolder, MetricRepository metricRepository,
-                                               MeasureRepository measureRepository) {
+    MeasureRepository measureRepository) {
     this.dbClient = dbClient;
     this.analysisMetadataHolder = analysisMetadataHolder;
     this.treeRootHolder = treeRootHolder;
@@ -60,38 +57,33 @@ public class UpdateQualityProfilesLastUsedDateStep implements ComputationStep {
 
   @Override
   public void execute(ComputationStep.Context context) {
+    Component root = treeRootHolder.getRoot();
+    Metric metric = metricRepository.getByKey(QUALITY_PROFILES_KEY);
+    Set<QualityProfile> qualityProfiles = parseQualityProfiles(measureRepository.getRawMeasure(root, metric));
     try (DbSession dbSession = dbClient.openSession(true)) {
-      Component root = treeRootHolder.getRoot();
-      Metric metric = metricRepository.getByKey(QUALITY_PROFILES_KEY);
-      Set<QualityProfile> qualityProfiles = parseQualityProfiles(measureRepository.getRawMeasure(root, metric));
-      if (qualityProfiles.isEmpty()) {
-        return;
+      for (QualityProfile qualityProfile : qualityProfiles) {
+        updateDate(dbSession, qualityProfile.getQpKey(), analysisMetadataHolder.getAnalysisDate());
       }
-
-      List<QProfileDto> dtos = dbClient.qualityProfileDao().selectByUuids(dbSession, qualityProfiles.stream().map(QualityProfile::getQpKey).collect(Collectors.toList()));
-      dtos.addAll(getAncestors(dbSession, dtos));
-      long analysisDate = analysisMetadataHolder.getAnalysisDate();
-      dtos.forEach(dto -> {
-        dto.setLastUsed(analysisDate);
-        dbClient.qualityProfileDao().update(dbSession, dto);
-      });
-
       dbSession.commit();
     }
   }
 
-  private List<QProfileDto> getAncestors(DbSession dbSession, List<QProfileDto> dtos) {
-    List<QProfileDto> ancestors = new ArrayList<>();
-    dtos.forEach(dto -> incrementAncestors(dbSession, dto, ancestors));
-    return ancestors;
-  }
-
-  private void incrementAncestors(DbSession session, QProfileDto profile, List<QProfileDto> ancestors) {
-    String parentKey = profile.getParentKee();
-    if (parentKey != null) {
-      QProfileDto parentDto = dbClient.qualityProfileDao().selectOrFailByUuid(session, parentKey);
-      ancestors.add(parentDto);
-      incrementAncestors(session, parentDto, ancestors);
+  private void updateDate(DbSession dbSession, String qProfileUuid, long lastUsedDate) {
+    // Traverse profiles from bottom to up in order to avoid DB deadlocks between multiple transactions.
+    // Example of hierarchy of profiles:
+    // A
+    // |- B
+    //    |- C1
+    //    |- C2
+    // Transaction #1 updates C1 then B then A
+    // Transaction #2 updates C2 then B then A
+    // Transaction #3 updates B then A
+    // No cross-dependencies are possible.
+    QProfileDto dto = dbClient.qualityProfileDao().selectOrFailByUuid(dbSession, qProfileUuid);
+    dbClient.qualityProfileDao().updateLastUsedDate(dbSession, dto, lastUsedDate);
+    String parentUuid = dto.getParentKee();
+    if (parentUuid != null) {
+      updateDate(dbSession, parentUuid, lastUsedDate);
     }
   }
 
diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/UpdateQualityProfilesLastUsedDateStepTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/UpdateQualityProfilesLastUsedDateStepTest.java
index b1e63f7e21e..372d34ac9a1 100644
--- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/UpdateQualityProfilesLastUsedDateStepTest.java
+++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/UpdateQualityProfilesLastUsedDateStepTest.java
@@ -52,9 +52,9 @@ import static org.sonar.db.qualityprofile.QualityProfileTesting.newQualityProfil
 public class UpdateQualityProfilesLastUsedDateStepTest {
   static final long ANALYSIS_DATE = 1_123_456_789L;
   private static final Component PROJECT = ReportComponent.DUMB_PROJECT;
-  private QProfileDto sonarWayJava = newQualityProfileDto().setKee("sonar-way-java");
-  private QProfileDto sonarWayPhp = newQualityProfileDto().setKee("sonar-way-php");
-  private QProfileDto myQualityProfile = newQualityProfileDto().setKee("my-qp");
+  private QProfileDto sonarWayJava = newProfile("sonar-way-java");
+  private QProfileDto sonarWayPhp = newProfile("sonar-way-php");
+  private QProfileDto myQualityProfile = newProfile("my-qp");
 
   @Rule
   public ExpectedException expectedException = ExpectedException.none();
@@ -74,9 +74,9 @@ public class UpdateQualityProfilesLastUsedDateStepTest {
   @Rule
   public MeasureRepositoryRule measureRepository = MeasureRepositoryRule.create(treeRootHolder, metricRepository);
 
-  DbClient dbClient = db.getDbClient();
-  DbSession dbSession = db.getSession();
-  QualityProfileDbTester qualityProfileDb = new QualityProfileDbTester(db);
+  private DbClient dbClient = db.getDbClient();
+  private DbSession dbSession = db.getSession();
+  private QualityProfileDbTester qualityProfileDb = new QualityProfileDbTester(db);
 
   UpdateQualityProfilesLastUsedDateStep underTest = new UpdateQualityProfilesLastUsedDateStep(dbClient, analysisMetadataHolder, treeRootHolder, metricRepository,
     measureRepository);
@@ -109,12 +109,12 @@ public class UpdateQualityProfilesLastUsedDateStepTest {
   @Test
   public void ancestor_profiles_are_updated() {
     // Parent profiles should be updated
-    QProfileDto rootProfile = newQualityProfileDto().setKee("root");
-    QProfileDto parentProfile = newQualityProfileDto().setKee("parent").setParentKee(rootProfile.getKee());
+    QProfileDto rootProfile = newProfile("root");
+    QProfileDto parentProfile = newProfile("parent").setParentKee(rootProfile.getKee());
     // Current profile => should be updated
-    QProfileDto currentProfile = newQualityProfileDto().setKee("current").setParentKee(parentProfile.getKee());
+    QProfileDto currentProfile = newProfile("current").setParentKee(parentProfile.getKee());
     // Child of current profile => should not be updated
-    QProfileDto childProfile = newQualityProfileDto().setKee("child").setParentKee(currentProfile.getKee());
+    QProfileDto childProfile = newProfile("child").setParentKee(currentProfile.getKee());
     qualityProfileDb.insert(rootProfile, parentProfile, currentProfile, childProfile);
 
     measureRepository.addRawMeasure(1, QUALITY_PROFILES_KEY, Measure.newMeasureBuilder().create(toJson(currentProfile.getKee())));
@@ -129,7 +129,7 @@ public class UpdateQualityProfilesLastUsedDateStepTest {
 
   @Test
   public void fail_when_profile_is_linked_to_unknown_parent() {
-    QProfileDto currentProfile = newQualityProfileDto().setKee("current").setParentKee("unknown");
+    QProfileDto currentProfile = newProfile("current").setParentKee("unknown");
     qualityProfileDb.insert(currentProfile);
 
     measureRepository.addRawMeasure(1, QUALITY_PROFILES_KEY, Measure.newMeasureBuilder().create(toJson(currentProfile.getKee())));
@@ -143,6 +143,12 @@ public class UpdateQualityProfilesLastUsedDateStepTest {
     assertThat(underTest.getDescription()).isEqualTo("Update last usage date of quality profiles");
   }
 
+  private static QProfileDto newProfile(String key) {
+    return newQualityProfileDto().setKee(key)
+      // profile has been used before the analysis
+      .setLastUsed(ANALYSIS_DATE - 10_000);
+  }
+
   private void assertQualityProfileIsUpdated(QProfileDto qp) {
     assertThat(selectLastUser(qp.getKee())).withFailMessage("Quality profile '%s' hasn't been updated. Value: %d", qp.getKee(), qp.getLastUsed()).isEqualTo(ANALYSIS_DATE);
   }
diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/qualityprofile/QualityProfileDao.java b/server/sonar-db-dao/src/main/java/org/sonar/db/qualityprofile/QualityProfileDao.java
index 958e684b764..838f295d85f 100644
--- a/server/sonar-db-dao/src/main/java/org/sonar/db/qualityprofile/QualityProfileDao.java
+++ b/server/sonar-db-dao/src/main/java/org/sonar/db/qualityprofile/QualityProfileDao.java
@@ -121,6 +121,10 @@ public class QualityProfileDao implements Dao {
     }
   }
 
+  public int updateLastUsedDate(DbSession dbSession, QProfileDto profile, long lastUsedDate) {
+    return mapper(dbSession).updateLastUsedDate(profile.getKee(), lastUsedDate, system.now());
+  }
+
   public void update(DbSession dbSession, RulesProfileDto rulesProfile) {
     QualityProfileMapper mapper = mapper(dbSession);
     long now = system.now();
diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/qualityprofile/QualityProfileMapper.java b/server/sonar-db-dao/src/main/java/org/sonar/db/qualityprofile/QualityProfileMapper.java
index 668beff7054..b0185d50cd9 100644
--- a/server/sonar-db-dao/src/main/java/org/sonar/db/qualityprofile/QualityProfileMapper.java
+++ b/server/sonar-db-dao/src/main/java/org/sonar/db/qualityprofile/QualityProfileMapper.java
@@ -135,4 +135,9 @@ public interface QualityProfileMapper {
   void renameRuleProfiles(@Param("newName") String newName, @Param("updatedAt") Date updatedAt, @Param("uuids") Collection<String> uuids);
 
   List<QProfileDto> selectQProfilesByRuleProfileUuid(@Param("rulesProfileUuid") String rulesProfileUuid);
+
+  int updateLastUsedDate(
+    @Param("uuid") String uuid,
+    @Param("lastUsedDate") long lastUsedDate,
+    @Param("now") long now);
 }
diff --git a/server/sonar-db-dao/src/main/resources/org/sonar/db/qualityprofile/QualityProfileMapper.xml b/server/sonar-db-dao/src/main/resources/org/sonar/db/qualityprofile/QualityProfileMapper.xml
index 3da11e2c1b9..dcbdaefe4ea 100644
--- a/server/sonar-db-dao/src/main/resources/org/sonar/db/qualityprofile/QualityProfileMapper.xml
+++ b/server/sonar-db-dao/src/main/resources/org/sonar/db/qualityprofile/QualityProfileMapper.xml
@@ -91,6 +91,16 @@
       uuid = #{dto.uuid, jdbcType=VARCHAR}
   </update>
 
+  <update id="updateLastUsedDate" parameterType="map">
+    update org_qprofiles
+    set
+      last_used = #{lastUsedDate, jdbcType=BIGINT},
+      updated_at = #{now, jdbcType=BIGINT}
+    where
+      uuid = #{uuid, jdbcType=VARCHAR}
+      and last_used &lt; #{lastUsedDate, jdbcType=BIGINT}
+  </update>
+
   <delete id="deleteRuleProfilesByUuids" parameterType="String">
     delete from rules_profiles
     where kee in
@@ -120,7 +130,7 @@
     <include refid="qProfileColumns"/>
     from org_qprofiles oqp
     inner join rules_profiles rp on oqp.rules_profile_uuid = rp.kee
-    where 
+    where
       oqp.organization_uuid = #{organizationUuid,jdbcType=VARCHAR}
     order by rp.name, rp.language
   </select>
@@ -183,7 +193,7 @@
     <include refid="qProfileColumns"/>
     from org_qprofiles oqp
     inner join rules_profiles rp on oqp.rules_profile_uuid = rp.kee
-    where 
+    where
       rp.name = #{name, jdbcType=VARCHAR}
       and rp.language = #{language, jdbcType=VARCHAR}
       and oqp.organization_uuid = #{organizationUuid, jdbcType=VARCHAR}
@@ -198,7 +208,7 @@
     rp.kee = #{ruleProfileUuid, jdbcType=VARCHAR}
     and oqp.organization_uuid = #{organizationUuid, jdbcType=VARCHAR}
   </select>
-    
+
   <select id="selectByNameAndLanguages" parameterType="map" resultType="org.sonar.db.qualityprofile.QProfileDto">
     select
     <include refid="qProfileColumns"/>
diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/qualityprofile/QualityProfileDaoTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/qualityprofile/QualityProfileDaoTest.java
index 51b5e3e7046..590651831fe 100644
--- a/server/sonar-db-dao/src/test/java/org/sonar/db/qualityprofile/QualityProfileDaoTest.java
+++ b/server/sonar-db-dao/src/test/java/org/sonar/db/qualityprofile/QualityProfileDaoTest.java
@@ -149,6 +149,32 @@ public class QualityProfileDaoTest {
   }
 
   @Test
+  public void test_updateLastUsedDate() {
+    QProfileDto initial = QualityProfileTesting.newQualityProfileDto()
+      .setLastUsed(10_000L);
+    underTest.insert(dbSession, initial);
+
+    int count = underTest.updateLastUsedDate(dbSession, initial, 15_000L);
+
+    assertThat(count).isEqualTo(1);
+    QProfileDto reloaded = underTest.selectByUuid(dbSession, initial.getKee());
+    assertThat(reloaded.getLastUsed()).isEqualTo(15_000L);
+  }
+
+  @Test
+  public void updateLastUsedDate_does_not_touch_row_if_last_used_is_more_recent() {
+    QProfileDto initial = QualityProfileTesting.newQualityProfileDto()
+      .setLastUsed(10_000L);
+    underTest.insert(dbSession, initial);
+
+    int count = underTest.updateLastUsedDate(dbSession, initial, 8_000L);
+
+    assertThat(count).isZero();
+    QProfileDto reloaded = underTest.selectByUuid(dbSession, initial.getKee());
+    assertThat(reloaded.getLastUsed()).isEqualTo(10_000L);
+  }
+
+  @Test
   public void selectRuleProfile() {
     RulesProfileDto rp = insertRulesProfile();