From bec5515eab682f114931d1badfc726d021fc5dbd Mon Sep 17 00:00:00 2001 From: Antoine Vinot Date: Fri, 24 Feb 2023 16:50:16 +0100 Subject: [PATCH] SONAR-18565 - Improve IssueIndexer performance to use markAsUnchanged flag --- .../component/FileStatuses.java | 5 + .../component/FileStatusesImpl.java | 7 ++ .../component/NopFileStatuses.java | 45 ++++++++ .../step/IndexAnalysisStep.java | 27 ++++- .../component/FileStatusesImplTest.java | 28 +++++ .../component/NopFileStatusesTest.java | 47 ++++++++ .../step/IndexAnalysisStepTest.java | 61 +++++++++-- .../org/sonar/db/component/BranchDao.java | 6 ++ .../org/sonar/db/component/BranchDaoTest.java | 23 ++++ .../component/index/ComponentIndexer.java | 5 + .../org/sonar/server/es/ProjectIndexer.java | 3 + .../server/issue/index/IssueIndexer.java | 35 ++++-- .../measure/index/ProjectMeasuresIndexer.java | 5 + .../sonar/server/es/ProjectIndexersTest.java | 10 +- .../server/issue/index/IssueIndexerTest.java | 102 ++++++++++++------ .../permission/index/PermissionIndexer.java | 5 + .../server/permission/index/FooIndexer.java | 5 + 17 files changed, 363 insertions(+), 56 deletions(-) create mode 100644 server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/component/NopFileStatuses.java create mode 100644 server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/component/NopFileStatusesTest.java diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/component/FileStatuses.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/component/FileStatuses.java index 36fe3e7c1aa..ffd76c8d30f 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/component/FileStatuses.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/component/FileStatuses.java @@ -19,6 +19,8 @@ */ package org.sonar.ce.task.projectanalysis.component; +import java.util.Set; + public interface FileStatuses { /** * A file is unchanged compared to the last analysis if it was detected as unchanged by the scanner and @@ -27,4 +29,7 @@ public interface FileStatuses { boolean isUnchanged(Component component); boolean isDataUnchanged(Component component); + + Set getFileUuidsMarkedAsUnchanged(); + } diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/component/FileStatusesImpl.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/component/FileStatusesImpl.java index a36268bb8db..487dcb049ff 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/component/FileStatusesImpl.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/component/FileStatusesImpl.java @@ -27,6 +27,7 @@ import org.sonar.ce.task.projectanalysis.source.SourceHashRepository; import org.sonar.db.source.FileHashesDto; import static com.google.common.base.Preconditions.checkState; +import static java.util.Collections.unmodifiableSet; import static org.sonar.ce.task.projectanalysis.component.ComponentVisitor.Order.PRE_ORDER; public class FileStatusesImpl implements FileStatuses { @@ -87,6 +88,12 @@ public class FileStatusesImpl implements FileStatuses { return fileUuidsMarkedAsUnchanged.contains(component.getUuid()); } + @Override + public Set getFileUuidsMarkedAsUnchanged() { + failIfNotInitialized(); + return unmodifiableSet(fileUuidsMarkedAsUnchanged); + } + private boolean hashEquals(Component component) { Optional dbHash = previousSourceHashRepository.getDbFile(component).map(FileHashesDto::getSrcHash); return dbHash.map(hash -> hash.equals(sourceHashRepository.getRawSourceHash(component))).orElse(false); diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/component/NopFileStatuses.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/component/NopFileStatuses.java new file mode 100644 index 00000000000..8ef493f92ed --- /dev/null +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/component/NopFileStatuses.java @@ -0,0 +1,45 @@ +/* + * SonarQube + * Copyright (C) 2009-2023 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.ce.task.projectanalysis.component; + + +import java.util.Set; + +/** + * No operation implementation of {@link FileStatuses} + */ +public final class NopFileStatuses implements FileStatuses { + + + @Override + public boolean isUnchanged(Component component) { + return false; + } + + @Override + public boolean isDataUnchanged(Component component) { + return false; + } + + @Override + public Set getFileUuidsMarkedAsUnchanged() { + return Set.of(); + } +} diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/IndexAnalysisStep.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/IndexAnalysisStep.java index bfde5e93641..739c51716cb 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/IndexAnalysisStep.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/IndexAnalysisStep.java @@ -19,10 +19,15 @@ */ package org.sonar.ce.task.projectanalysis.step; +import java.util.Set; +import java.util.function.Consumer; import org.sonar.api.utils.log.Logger; import org.sonar.api.utils.log.Loggers; +import org.sonar.ce.task.projectanalysis.component.FileStatuses; import org.sonar.ce.task.projectanalysis.component.TreeRootHolder; import org.sonar.ce.task.step.ComputationStep; +import org.sonar.db.DbClient; +import org.sonar.db.DbSession; import org.sonar.server.es.ProjectIndexer; public class IndexAnalysisStep implements ComputationStep { @@ -30,19 +35,37 @@ public class IndexAnalysisStep implements ComputationStep { private static final Logger LOGGER = Loggers.get(IndexAnalysisStep.class); private final TreeRootHolder treeRootHolder; + private final FileStatuses fileStatuses; private final ProjectIndexer[] indexers; + private final DbClient dbClient; - public IndexAnalysisStep(TreeRootHolder treeRootHolder, ProjectIndexer... indexers) { + public IndexAnalysisStep(TreeRootHolder treeRootHolder, FileStatuses fileStatuses, DbClient dbClient,ProjectIndexer... indexers) { this.treeRootHolder = treeRootHolder; + this.fileStatuses = fileStatuses; this.indexers = indexers; + this.dbClient = dbClient; } @Override public void execute(ComputationStep.Context context) { String branchUuid = treeRootHolder.getRoot().getUuid(); + Consumer projectIndexerConsumer = getProjectIndexerConsumer(branchUuid); for (ProjectIndexer indexer : indexers) { LOGGER.debug("Call {}", indexer); - indexer.indexOnAnalysis(branchUuid); + projectIndexerConsumer.accept(indexer); + } + } + + private Consumer getProjectIndexerConsumer(String branchUuid) { + Set fileUuidsMarkedAsUnchanged = fileStatuses.getFileUuidsMarkedAsUnchanged(); + return isBranchNeedIssueSync(branchUuid) + ? (indexer -> indexer.indexOnAnalysis(branchUuid)) + : (indexer -> indexer.indexOnAnalysis(branchUuid, fileUuidsMarkedAsUnchanged)); + } + + private boolean isBranchNeedIssueSync(String branchUuid) { + try (DbSession dbSession = dbClient.openSession(false)) { + return dbClient.branchDao().isBranchNeedIssueSync(dbSession, branchUuid); } } diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/component/FileStatusesImplTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/component/FileStatusesImplTest.java index dc8eb4f5096..b472f2eab84 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/component/FileStatusesImplTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/component/FileStatusesImplTest.java @@ -29,6 +29,7 @@ import org.sonar.ce.task.projectanalysis.source.SourceHashRepository; import org.sonar.db.source.FileHashesDto; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -173,6 +174,33 @@ public class FileStatusesImplTest { verify(previousSourceHashRepository).getDbFile(file1); } + @Test + public void getFileUuidsMarkedAsUnchanged_whenNotInitialized_shouldFail() { + assertThatThrownBy(fileStatuses::getFileUuidsMarkedAsUnchanged) + .isInstanceOf(IllegalStateException.class) + .hasMessage("Not initialized"); + } + + @Test + public void getFileUuidsMarkedAsUnchanged_shouldReturnMarkAsUnchangedFileUuids() { + Component file1 = ReportComponent.builder(Component.Type.FILE, 2, "FILE1_KEY").setStatus(Component.Status.SAME) + .setFileAttributes(new FileAttributes(false, null, 10, true, null)).build(); + Component file2 = ReportComponent.builder(Component.Type.FILE, 3, "FILE2_KEY").setStatus(Component.Status.SAME).build(); + addDbFileHash(file1, "hash1"); + addDbFileHash(file2, "hash2"); + addReportFileHash(file1, "hash1"); + addReportFileHash(file2, "hash2"); + Component project = ReportComponent.builder(Component.Type.PROJECT, 1) + .setUuid(PROJECT_UUID) + .setKey(PROJECT_KEY) + .addChildren(file1, file2) + .build(); + treeRootHolder.setRoot(project); + fileStatuses.initialize(); + + assertThat(fileStatuses.getFileUuidsMarkedAsUnchanged()).contains(file1.getUuid()); + } + private void addDbFileHash(Component file, String hash) { FileHashesDto fileHashesDto = new FileHashesDto().setSrcHash(hash); when(previousSourceHashRepository.getDbFile(file)).thenReturn(Optional.of(fileHashesDto)); diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/component/NopFileStatusesTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/component/NopFileStatusesTest.java new file mode 100644 index 00000000000..39a25fef6e4 --- /dev/null +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/component/NopFileStatusesTest.java @@ -0,0 +1,47 @@ +/* + * SonarQube + * Copyright (C) 2009-2023 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.ce.task.projectanalysis.component; + +import org.junit.Test; +import org.mockito.Mockito; + +import static org.assertj.core.api.Assertions.assertThat; + +public class NopFileStatusesTest { + + private final NopFileStatuses nopFileStatuses = new NopFileStatuses(); + + private final Component component = Mockito.mock(Component.class); + + @Test + public void isUnchanged() { + assertThat(nopFileStatuses.isUnchanged(component)).isFalse(); + } + + @Test + public void isDataUnchanged() { + assertThat(nopFileStatuses.isDataUnchanged(component)).isFalse(); + } + + @Test + public void getFileUuidsMarkedAsUnchanged() { + assertThat(nopFileStatuses.getFileUuidsMarkedAsUnchanged()).isEmpty(); + } +} diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/IndexAnalysisStepTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/IndexAnalysisStepTest.java index 31bb4c3eb5d..24dfe9fa583 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/IndexAnalysisStepTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/IndexAnalysisStepTest.java @@ -19,19 +19,26 @@ */ package org.sonar.ce.task.projectanalysis.step; +import java.util.Set; +import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.sonar.ce.task.projectanalysis.batch.BatchReportReaderRule; import org.sonar.ce.task.projectanalysis.component.Component; +import org.sonar.ce.task.projectanalysis.component.FileStatuses; import org.sonar.ce.task.projectanalysis.component.ReportComponent; import org.sonar.ce.task.projectanalysis.component.TreeRootHolderRule; import org.sonar.ce.task.projectanalysis.component.ViewsComponent; import org.sonar.ce.task.step.ComputationStep; import org.sonar.ce.task.step.TestComputationStepContext; +import org.sonar.db.DbClient; +import org.sonar.db.DbSession; +import org.sonar.db.component.BranchDao; import org.sonar.server.es.ProjectIndexer; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import static org.sonar.ce.task.projectanalysis.component.Component.Type.PROJECT; import static org.sonar.ce.task.projectanalysis.component.Component.Type.VIEW; @@ -45,17 +52,36 @@ public class IndexAnalysisStepTest extends BaseStepTest { @Rule public BatchReportReaderRule reportReader = new BatchReportReaderRule(); - private ProjectIndexer componentIndexer = mock(ProjectIndexer.class); - private IndexAnalysisStep underTest = new IndexAnalysisStep(treeRootHolder, componentIndexer); + private final DbClient dbClient = mock(DbClient.class); + + private final FileStatuses fileStatuses = mock(FileStatuses.class); + + private final ProjectIndexer projectIndexer = mock(ProjectIndexer.class); + + private final DbSession dbSession = mock(DbSession.class); + + private final BranchDao branchDao = mock(BranchDao.class); + + private final IndexAnalysisStep underTest = new IndexAnalysisStep(treeRootHolder, fileStatuses, dbClient, projectIndexer); + + private TestComputationStepContext testComputationStepContext; + + @Before + public void init() { + testComputationStepContext = new TestComputationStepContext(); + + when(dbClient.openSession(false)).thenReturn(dbSession); + when(dbClient.branchDao()).thenReturn(branchDao); + } @Test public void call_indexByProjectUuid_of_indexer_for_project() { Component project = ReportComponent.builder(PROJECT, 1).setUuid(PROJECT_UUID).setKey(PROJECT_KEY).build(); treeRootHolder.setRoot(project); - underTest.execute(new TestComputationStepContext()); + underTest.execute(testComputationStepContext); - verify(componentIndexer).indexOnAnalysis(PROJECT_UUID); + verify(projectIndexer).indexOnAnalysis(PROJECT_UUID, Set.of()); } @Test @@ -63,9 +89,32 @@ public class IndexAnalysisStepTest extends BaseStepTest { Component view = ViewsComponent.builder(VIEW, PROJECT_KEY).setUuid(PROJECT_UUID).build(); treeRootHolder.setRoot(view); - underTest.execute(new TestComputationStepContext()); + underTest.execute(testComputationStepContext); + + verify(projectIndexer).indexOnAnalysis(PROJECT_UUID, Set.of()); + } + + @Test + public void execute_whenMarkAsUnchangedFlagActivated_shouldCallIndexOnAnalysisWithChangedComponents() { + Component project = ReportComponent.builder(PROJECT, 1).setUuid(PROJECT_UUID).setKey(PROJECT_KEY).build(); + treeRootHolder.setRoot(project); + Set anyUuids = Set.of("any-uuid"); + when(fileStatuses.getFileUuidsMarkedAsUnchanged()).thenReturn(anyUuids); + + underTest.execute(testComputationStepContext); + + verify(projectIndexer).indexOnAnalysis(PROJECT_UUID, anyUuids); + } + + @Test + public void execute_whenBranchIsNeedIssueSync_shouldReindexEverything() { + Component project = ReportComponent.builder(PROJECT, 1).setUuid(PROJECT_UUID).setKey(PROJECT_KEY).build(); + treeRootHolder.setRoot(project); + when(branchDao.isBranchNeedIssueSync(dbSession, PROJECT_UUID)).thenReturn(true); + + underTest.execute(testComputationStepContext); - verify(componentIndexer).indexOnAnalysis(PROJECT_UUID); + verify(projectIndexer).indexOnAnalysis(PROJECT_UUID); } @Override diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/component/BranchDao.java b/server/sonar-db-dao/src/main/java/org/sonar/db/component/BranchDao.java index a21fd54cd65..47f340a63ca 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/component/BranchDao.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/component/BranchDao.java @@ -177,4 +177,10 @@ public class BranchDao implements Dao { } return false; } + + public boolean isBranchNeedIssueSync(DbSession session, String branchUuid) { + return selectByUuid(session, branchUuid) + .map(BranchDto::isNeedIssueSync) + .orElse(false); + } } diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/component/BranchDaoTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/component/BranchDaoTest.java index 0740e9c2552..3156edc6e91 100644 --- a/server/sonar-db-dao/src/test/java/org/sonar/db/component/BranchDaoTest.java +++ b/server/sonar-db-dao/src/test/java/org/sonar/db/component/BranchDaoTest.java @@ -22,6 +22,7 @@ package org.sonar.db.component; import com.google.common.collect.Sets; import com.tngtech.java.junit.dataprovider.DataProvider; import com.tngtech.java.junit.dataprovider.DataProviderRunner; +import com.tngtech.java.junit.dataprovider.UseDataProvider; import java.util.Collection; import java.util.Collections; import java.util.HashMap; @@ -776,4 +777,26 @@ public class BranchDaoTest { assertThat(underTest.doAnyOfComponentsNeedIssueSync(dbSession, componentKeys)).isTrue(); } + + @DataProvider + public static Object[][] booleanValues() { + return new Object[][] { + {true}, + {false} + }; + } + + @Test + @UseDataProvider("booleanValues") + public void isBranchNeedIssueSync_shouldReturnCorrectValue(boolean needIssueSync) { + ComponentDto project = db.components().insertPrivateProject(); + String branchUuid = db.components().insertProjectBranch(project, branch -> branch.setBranchType(BranchType.BRANCH).setNeedIssueSync(needIssueSync)).uuid(); + + assertThat(underTest.isBranchNeedIssueSync(dbSession, branchUuid)).isEqualTo(needIssueSync); + } + + @Test + public void isBranchNeedIssueSync_whenNoBranch_shouldReturnFalse() { + assertThat(underTest.isBranchNeedIssueSync(dbSession, "unknown")).isFalse(); + } } diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/component/index/ComponentIndexer.java b/server/sonar-server-common/src/main/java/org/sonar/server/component/index/ComponentIndexer.java index 202f45328a5..2890a3809ec 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/component/index/ComponentIndexer.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/component/index/ComponentIndexer.java @@ -79,6 +79,11 @@ public class ComponentIndexer implements ProjectIndexer, NeedAuthorizationIndexe @Override public void indexOnAnalysis(String branchUuid) { + indexOnAnalysis(branchUuid, Set.of()); + } + + @Override + public void indexOnAnalysis(String branchUuid, Set unchangedComponentUuids) { doIndexByProjectUuid(branchUuid, Size.REGULAR); } diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/es/ProjectIndexer.java b/server/sonar-server-common/src/main/java/org/sonar/server/es/ProjectIndexer.java index b8b613a9a77..bc50f2c0d93 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/es/ProjectIndexer.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/es/ProjectIndexer.java @@ -20,6 +20,7 @@ package org.sonar.server.es; import java.util.Collection; +import java.util.Set; import org.sonar.db.DbSession; import org.sonar.db.es.EsQueueDto; @@ -52,5 +53,7 @@ public interface ProjectIndexer extends ResilientIndexer { */ void indexOnAnalysis(String branchUuid); + void indexOnAnalysis(String branchUuid, Set unchangedComponentUuids); + Collection prepareForRecovery(DbSession dbSession, Collection projectUuids, ProjectIndexer.Cause cause); } diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/issue/index/IssueIndexer.java b/server/sonar-server-common/src/main/java/org/sonar/server/issue/index/IssueIndexer.java index 1ccdba0c6c3..c8b477e955d 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/issue/index/IssueIndexer.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/issue/index/IssueIndexer.java @@ -104,14 +104,21 @@ public class IssueIndexer implements ProjectIndexer, NeedAuthorizationIndexer { public void indexAllIssues() { try (IssueIterator issues = issueIteratorFactory.createForAll()) { - doIndex(issues, Size.REGULAR, IndexingListener.FAIL_ON_ERROR); + doIndex(issues, Set.of()); } } @Override public void indexOnAnalysis(String branchUuid) { try (IssueIterator issues = issueIteratorFactory.createForBranch(branchUuid)) { - doIndex(issues, Size.REGULAR, IndexingListener.FAIL_ON_ERROR); + doIndex(issues, Set.of()); + } + } + + @Override + public void indexOnAnalysis(String branchUuid, Set unchangedComponentUuids) { + try (IssueIterator issues = issueIteratorFactory.createForBranch(branchUuid)) { + doIndex(issues, unchangedComponentUuids); } } @@ -185,7 +192,7 @@ public class IssueIndexer implements ProjectIndexer, NeedAuthorizationIndexer { return new IndexingResult(); } IndexingListener listener = new OneToOneResilientIndexingListener(dbClient, dbSession, itemsByIssueKey.values()); - BulkIndexer bulkIndexer = createBulkIndexer(Size.REGULAR, listener); + BulkIndexer bulkIndexer = createBulkIndexer(listener); bulkIndexer.start(); try (IssueIterator issues = issueIteratorFactory.createForIssueKeys(itemsByIssueKey.keySet())) { @@ -211,7 +218,7 @@ public class IssueIndexer implements ProjectIndexer, NeedAuthorizationIndexer { // one project, referenced by es_queue.doc_id = many issues IndexingListener listener = new OneToManyResilientIndexingListener(dbClient, dbSession, itemsByProjectUuid.values()); - BulkIndexer bulkIndexer = createBulkIndexer(Size.REGULAR, listener); + BulkIndexer bulkIndexer = createBulkIndexer(listener); bulkIndexer.start(); for (String projectUuid : itemsByProjectUuid.keySet()) { @@ -239,7 +246,7 @@ public class IssueIndexer implements ProjectIndexer, NeedAuthorizationIndexer { return; } - BulkIndexer bulkIndexer = createBulkIndexer(Size.REGULAR, IndexingListener.FAIL_ON_ERROR); + BulkIndexer bulkIndexer = createBulkIndexer(IndexingListener.FAIL_ON_ERROR); bulkIndexer.start(); issueKeys.forEach(issueKey -> bulkIndexer.addDeletion(TYPE_ISSUE.getMainType(), issueKey, AuthorizationDoc.idOf(projectUuid))); bulkIndexer.stop(); @@ -247,19 +254,25 @@ public class IssueIndexer implements ProjectIndexer, NeedAuthorizationIndexer { @VisibleForTesting protected void index(Iterator issues) { - doIndex(issues, Size.REGULAR, IndexingListener.FAIL_ON_ERROR); + doIndex(issues, Set.of()); } - private void doIndex(Iterator issues, Size size, IndexingListener listener) { - BulkIndexer bulk = createBulkIndexer(size, listener); + private void doIndex(Iterator issues, Set unchangedComponentUuids) { + BulkIndexer bulk = createBulkIndexer(IndexingListener.FAIL_ON_ERROR); bulk.start(); while (issues.hasNext()) { IssueDoc issue = issues.next(); - bulk.add(newIndexRequest(issue)); + if (shouldReindexIssue(issue, unchangedComponentUuids)) { + bulk.add(newIndexRequest(issue)); + } } bulk.stop(); } + private static boolean shouldReindexIssue(IssueDoc issue, Set unchangedComponentUuids) { + return issue.getFields().get(IssueIndexDefinition.FIELD_ISSUE_COMPONENT_UUID) == null || !unchangedComponentUuids.contains(issue.componentUuid()); + } + private static IndexRequest newIndexRequest(IssueDoc issue) { return new IndexRequest(TYPE_ISSUE.getMainType().getIndex().getName()) .id(issue.getId()) @@ -279,7 +292,7 @@ public class IssueIndexer implements ProjectIndexer, NeedAuthorizationIndexer { return EsQueueDto.create(TYPE_ISSUE.format(), docId, docIdType, AuthorizationDoc.idOf(projectUuid)); } - private BulkIndexer createBulkIndexer(Size size, IndexingListener listener) { - return new BulkIndexer(esClient, TYPE_ISSUE, size, listener); + private BulkIndexer createBulkIndexer(IndexingListener listener) { + return new BulkIndexer(esClient, TYPE_ISSUE, Size.REGULAR, listener); } } diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/measure/index/ProjectMeasuresIndexer.java b/server/sonar-server-common/src/main/java/org/sonar/server/measure/index/ProjectMeasuresIndexer.java index 636e2cb5f9b..4ffeaca7bbc 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/measure/index/ProjectMeasuresIndexer.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/measure/index/ProjectMeasuresIndexer.java @@ -83,6 +83,11 @@ public class ProjectMeasuresIndexer implements ProjectIndexer, NeedAuthorization @Override public void indexOnAnalysis(String projectUuid) { + indexOnAnalysis(projectUuid, Set.of()); + } + + @Override + public void indexOnAnalysis(String projectUuid, Set unchangedComponentUuids) { doIndex(Size.REGULAR, projectUuid); } diff --git a/server/sonar-server-common/src/test/java/org/sonar/server/es/ProjectIndexersTest.java b/server/sonar-server-common/src/test/java/org/sonar/server/es/ProjectIndexersTest.java index 223f229a583..bfecb51c4a6 100644 --- a/server/sonar-server-common/src/test/java/org/sonar/server/es/ProjectIndexersTest.java +++ b/server/sonar-server-common/src/test/java/org/sonar/server/es/ProjectIndexersTest.java @@ -49,11 +49,6 @@ public class ProjectIndexersTest { assertThat(indexer2.calledItems).containsExactlyInAnyOrder(item2); } - @Test - public void commitAndIndex_restricts_indexing_to_projects() { - // TODO - } - private static class FakeIndexer implements ProjectIndexer { private final List items; @@ -88,5 +83,10 @@ public class ProjectIndexersTest { public void indexOnAnalysis(String branchUuid) { throw new UnsupportedOperationException(); } + + @Override + public void indexOnAnalysis(String branchUuid, Set unchangedComponentUuids) { + throw new UnsupportedOperationException(); + } } } diff --git a/server/sonar-server-common/src/test/java/org/sonar/server/issue/index/IssueIndexerTest.java b/server/sonar-server-common/src/test/java/org/sonar/server/issue/index/IssueIndexerTest.java index c7dfe304216..d005db3e99d 100644 --- a/server/sonar-server-common/src/test/java/org/sonar/server/issue/index/IssueIndexerTest.java +++ b/server/sonar-server-common/src/test/java/org/sonar/server/issue/index/IssueIndexerTest.java @@ -24,6 +24,7 @@ import java.util.Collection; import java.util.Date; import java.util.HashSet; import java.util.List; +import java.util.Set; import java.util.function.Predicate; import java.util.stream.Collectors; import org.assertj.core.api.Assertions; @@ -42,6 +43,7 @@ import org.sonar.db.issue.IssueDto; import org.sonar.db.issue.IssueTesting; import org.sonar.db.rule.RuleDto; import org.sonar.server.es.EsTester; +import org.sonar.server.es.IndexType; import org.sonar.server.es.IndexingResult; import org.sonar.server.es.ProjectIndexer; import org.sonar.server.es.StartupIndexer; @@ -138,11 +140,11 @@ public class IssueIndexerTest { @Test public void verify_security_standards_indexation() { - RuleDto rule = db.rules().insert(r -> r.setSecurityStandards(new HashSet<>(Arrays.asList("cwe:123", "owaspTop10:a3", "cwe:863","owaspAsvs-4.0:2.1.1")))); + RuleDto rule = db.rules().insert(r -> r.setSecurityStandards(new HashSet<>(Arrays.asList("cwe:123", "owaspTop10:a3", "cwe:863", "owaspAsvs-4.0:2.1.1")))); ComponentDto project = db.components().insertPrivateProject(); ComponentDto dir = db.components().insertComponent(ComponentTesting.newDirectory(project, "src/main/java/foo")); ComponentDto file = db.components().insertComponent(newFileDto(project, dir, "F1")); - IssueDto issue = db.issues().insert(rule, project, file); + db.issues().insert(rule, project, file); underTest.indexAllIssues(); @@ -158,15 +160,13 @@ public class IssueIndexerTest { es.lockWrites(TYPE_ISSUE); db.issues().insert(); - try { - // FIXME : test also message - assertThatThrownBy(() -> underTest.indexOnStartup(emptySet())) - .isInstanceOf(IllegalStateException.class); - } finally { - assertThatIndexHasSize(0); - assertThatEsQueueTableHasSize(0); - es.unlockWrites(TYPE_ISSUE); - } + Set uninitializedIndexTypes = emptySet(); + assertThatThrownBy(() -> underTest.indexOnStartup(uninitializedIndexTypes)) + .isInstanceOf(IllegalStateException.class) + .hasMessage("SYNCHRONE StartupIndexer must implement indexOnStartup"); + assertThatIndexHasSize(0); + assertThatEsQueueTableHasSize(0); + es.unlockWrites(TYPE_ISSUE); } @Test @@ -176,7 +176,7 @@ public class IssueIndexerTest { ComponentDto file = db.components().insertComponent(newFileDto(project)); IssueDto issue = db.issues().insert(rule, project, file); ComponentDto otherProject = db.components().insertPrivateProject(); - ComponentDto fileOnOtherProject = db.components().insertComponent(newFileDto(otherProject)); + db.components().insertComponent(newFileDto(otherProject)); underTest.indexOnAnalysis(project.uuid()); @@ -209,15 +209,13 @@ public class IssueIndexerTest { es.lockWrites(TYPE_ISSUE); IssueDto issue = db.issues().insert(); - try { - // FIXME : test also message - assertThatThrownBy(() -> underTest.indexOnAnalysis(issue.getProjectUuid())) - .isInstanceOf(IllegalStateException.class); - } finally { - assertThatIndexHasSize(0); - assertThatEsQueueTableHasSize(0); - es.unlockWrites(TYPE_ISSUE); - } + String projectUuid = issue.getProjectUuid(); + assertThatThrownBy(() -> underTest.indexOnAnalysis(projectUuid)) + .isInstanceOf(IllegalStateException.class) + .hasMessage("Unrecoverable indexation failures: 1 errors among 1 requests. Check Elasticsearch logs for further details."); + assertThatIndexHasSize(0); + assertThatEsQueueTableHasSize(0); + es.unlockWrites(TYPE_ISSUE); } @Test @@ -394,8 +392,8 @@ public class IssueIndexerTest { RuleDto rule = db.rules().insert(); ComponentDto project = db.components().insertPrivateProject(); ComponentDto file = db.components().insertComponent(newFileDto(project)); - IssueDto issue1 = db.issues().insert(rule, project, file); - IssueDto issue2 = db.issues().insert(rule, project, file); + db.issues().insert(rule, project, file); + db.issues().insert(rule, project, file); es.lockWrites(TYPE_ISSUE); @@ -443,15 +441,13 @@ public class IssueIndexerTest { addIssueToIndex("P1", "Issue1"); es.lockWrites(TYPE_ISSUE); - try { - // FIXME : test also message - assertThatThrownBy(() -> underTest.deleteByKeys("P1", asList("Issue1"))) - .isInstanceOf(IllegalStateException.class); - } finally { - assertThatIndexHasOnly("Issue1"); - assertThatEsQueueTableHasSize(0); - es.unlockWrites(TYPE_ISSUE); - } + List issues = List.of("Issue1"); + assertThatThrownBy(() -> underTest.deleteByKeys("P1", issues)) + .isInstanceOf(IllegalStateException.class) + .hasMessage("Unrecoverable indexation failures: 1 errors among 1 requests. Check Elasticsearch logs for further details."); + assertThatIndexHasOnly("Issue1"); + assertThatEsQueueTableHasSize(0); + es.unlockWrites(TYPE_ISSUE); } @Test @@ -549,6 +545,48 @@ public class IssueIndexerTest { Assertions.assertThat(underTest.getType()).isEqualTo(StartupIndexer.Type.ASYNCHRONOUS); } + @Test + public void indexOnAnalysis_whenChangedComponents_shouldReindexOnlyChangedComponents() { + RuleDto rule = db.rules().insert(); + ComponentDto project = db.components().insertPrivateProject(); + ComponentDto changedComponent1 = db.components().insertComponent(newFileDto(project)); + ComponentDto unchangedComponent = db.components().insertComponent(newFileDto(project)); + ComponentDto ChangedComponent2 = db.components().insertComponent(newFileDto(project)); + IssueDto changedIssue1 = db.issues().insert(rule, project, changedComponent1); + IssueDto changedIssue2 = db.issues().insert(rule, project, changedComponent1); + IssueDto changedIssue3 = db.issues().insert(rule, project, ChangedComponent2); + db.issues().insert(rule, project, unchangedComponent); + db.issues().insert(rule, project, unchangedComponent); + + underTest.indexOnAnalysis(project.uuid(), Set.of(unchangedComponent.uuid())); + + assertThatIndexHasOnly(changedIssue1, changedIssue2, changedIssue3); + } + + @Test + public void indexOnAnalysis_whenEmptyUnchangedComponents_shouldReindexEverything() { + RuleDto rule = db.rules().insert(); + ComponentDto project = db.components().insertPrivateProject(); + ComponentDto changedComponent = db.components().insertComponent(newFileDto(project)); + IssueDto changedIssue1 = db.issues().insert(rule, project, changedComponent); + IssueDto changedIssue2 = db.issues().insert(rule, project, changedComponent); + + underTest.indexOnAnalysis(project.uuid(), Set.of()); + + assertThatIndexHasOnly(changedIssue1, changedIssue2); + } + + @Test + public void indexOnAnalysis_whenChangedComponentWithoutIssue_shouldReindexNothing() { + db.rules().insert(); + ComponentDto project = db.components().insertPrivateProject(); + db.components().insertComponent(newFileDto(project)); + + underTest.indexOnAnalysis(project.uuid(), Set.of()); + + assertThat(es.getDocuments(TYPE_ISSUE)).isEmpty(); + } + private void addIssueToIndex(String projectUuid, String issueKey) { es.putDocuments(TYPE_ISSUE, newDoc().setKey(issueKey).setProjectUuid(projectUuid)); diff --git a/server/sonar-webserver-es/src/main/java/org/sonar/server/permission/index/PermissionIndexer.java b/server/sonar-webserver-es/src/main/java/org/sonar/server/permission/index/PermissionIndexer.java index d2a7fcb2e86..32eab8af7f2 100644 --- a/server/sonar-webserver-es/src/main/java/org/sonar/server/permission/index/PermissionIndexer.java +++ b/server/sonar-webserver-es/src/main/java/org/sonar/server/permission/index/PermissionIndexer.java @@ -103,6 +103,11 @@ public class PermissionIndexer implements ProjectIndexer { // nothing to do, permissions don't change during an analysis } + @Override + public void indexOnAnalysis(String branchUuid, Set unchangedComponentUuids) { + // nothing to do, permissions don't change during an analysis + } + @Override public Collection prepareForRecovery(DbSession dbSession, Collection projectUuids, ProjectIndexer.Cause cause) { return switch (cause) { diff --git a/server/sonar-webserver-es/src/test/java/org/sonar/server/permission/index/FooIndexer.java b/server/sonar-webserver-es/src/test/java/org/sonar/server/permission/index/FooIndexer.java index 87236fb98b7..b25b742318b 100644 --- a/server/sonar-webserver-es/src/test/java/org/sonar/server/permission/index/FooIndexer.java +++ b/server/sonar-webserver-es/src/test/java/org/sonar/server/permission/index/FooIndexer.java @@ -50,6 +50,11 @@ public class FooIndexer implements ProjectIndexer, NeedAuthorizationIndexer { @Override public void indexOnAnalysis(String branchUuid) { + indexOnAnalysis(branchUuid, Set.of()); + } + + @Override + public void indexOnAnalysis(String branchUuid, Set unchangedComponentUuids) { addToIndex(branchUuid, "bar"); addToIndex(branchUuid, "baz"); } -- 2.39.5