From edc71bbf870662c4c93623b51e7d098aeaf971b6 Mon Sep 17 00:00:00 2001 From: Steve Marion Date: Wed, 21 Feb 2024 17:56:56 +0100 Subject: [PATCH] SONAR-21655 ensure issue indexing chunks are properly closed when iterating over large batches. (cherry picked from commit 95e6a254696765e8984a7a13f951fc7e6baf1736) --- .../server/issue/index/IssueIndexerIT.java | 51 +++++++++++++++++++ .../index/IssueIteratorForMultipleChunks.java | 5 +- .../index/IssueIteratorForSingleChunk.java | 13 ++++- 3 files changed, 67 insertions(+), 2 deletions(-) diff --git a/server/sonar-server-common/src/it/java/org/sonar/server/issue/index/IssueIndexerIT.java b/server/sonar-server-common/src/it/java/org/sonar/server/issue/index/IssueIndexerIT.java index da7fa07acc7..f4e20951b5e 100644 --- a/server/sonar-server-common/src/it/java/org/sonar/server/issue/index/IssueIndexerIT.java +++ b/server/sonar-server-common/src/it/java/org/sonar/server/issue/index/IssueIndexerIT.java @@ -19,6 +19,7 @@ */ package org.sonar.server.issue.index; +import com.google.common.collect.Iterables; import java.util.Arrays; import java.util.Collection; import java.util.Date; @@ -27,15 +28,19 @@ import java.util.List; import java.util.Map; import java.util.Set; import java.util.function.Predicate; +import java.util.stream.Stream; import org.assertj.core.api.Assertions; import org.elasticsearch.search.SearchHit; import org.junit.Rule; import org.junit.Test; +import org.mockito.MockedStatic; +import org.mockito.Mockito; import org.slf4j.event.Level; import org.sonar.api.issue.impact.Severity; import org.sonar.api.issue.impact.SoftwareQuality; import org.sonar.api.resources.Qualifiers; import org.sonar.api.testfixtures.log.LogTester; +import org.sonar.db.DatabaseUtils; import org.sonar.db.DbSession; import org.sonar.db.DbTester; import org.sonar.db.component.BranchDto; @@ -64,6 +69,7 @@ import static java.util.Collections.emptyList; import static java.util.Collections.emptySet; import static java.util.Collections.singletonList; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.assertj.core.groups.Tuple.tuple; import static org.sonar.db.component.ComponentTesting.newFileDto; @@ -88,6 +94,51 @@ public class IssueIndexerIT { private final IssueIndexer underTest = new IssueIndexer(es.client(), db.getDbClient(), new IssueIteratorFactory(db.getDbClient()), null); + @Test + public void whenMultipleChunks_allShouldBeClosed() { + RuleDto rule = db.rules().insert(); + ProjectData projectData = db.components().insertPrivateProject(); + ComponentDto project = projectData.getMainBranchComponent(); + ComponentDto file = db.components().insertComponent(newFileDto(project) + .setPath("src/main/java/Action.java")); + + List issuesToIndex = Stream.generate(() -> + db.issues().insert(rule, project, file, + t -> t.setResolution("FIXED") + .setStatus("RESOLVED") + .setSeverity("BLOCKER") + .setManualSeverity(false) + .setAssigneeUuid("uuid-of-guy1") + .setAuthorLogin("guy2") + .setChecksum("FFFFF") + .setGap(2D) + .setEffort(10L) + .setMessage(null) + .setLine(444) + .setRuleUuid(rule.getUuid()) + .setTags(List.of("tag1", "tag2", "tag3")) + .setCreatedAt(1400000000000L) + .setUpdatedAt(1400000000000L) + .setIssueCreationDate(new Date(1115848800000L)) + .setIssueUpdateDate(new Date(1356994800000L)) + .setIssueCloseDate(null) + .setType(2) + .setCodeVariants(List.of("variant1", "variant2")))) + .limit(200) + .toList(); + + + try (MockedStatic dbUtils = Mockito.mockStatic(DatabaseUtils.class)) { + dbUtils.when(() -> DatabaseUtils.toUniqueAndSortedPartitions(Mockito.any())).thenAnswer(invocation -> { + Collection input = invocation.getArgument(0); + return Iterables.partition(List.copyOf(input), 10); + }); + // should have no exception. if session aren't released properly, a timeout on acquiring a session should be thrown. + assertThatCode(() -> underTest.commitAndIndexIssues(db.getSession(), issuesToIndex)) + .doesNotThrowAnyException(); + } + } + @Test public void getIndexTypes_shouldReturnTypeIssue() { assertThat(underTest.getIndexTypes()).containsExactly(TYPE_ISSUE); diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/issue/index/IssueIteratorForMultipleChunks.java b/server/sonar-server-common/src/main/java/org/sonar/server/issue/index/IssueIteratorForMultipleChunks.java index d84534e9676..6b4f7812aef 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/issue/index/IssueIteratorForMultipleChunks.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/issue/index/IssueIteratorForMultipleChunks.java @@ -48,7 +48,10 @@ public class IssueIteratorForMultipleChunks implements IssueIterator { @Override public IssueDoc next() { - if (currentChunk == null || !currentChunk.hasNext()) { + if (currentChunk == null) { + currentChunk = nextChunk(); + } else if (!currentChunk.hasNext()) { + currentChunk.close(); currentChunk = nextChunk(); } return currentChunk.next(); diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/issue/index/IssueIteratorForSingleChunk.java b/server/sonar-server-common/src/main/java/org/sonar/server/issue/index/IssueIteratorForSingleChunk.java index 8c4ee30f5a4..fad352719d5 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/issue/index/IssueIteratorForSingleChunk.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/issue/index/IssueIteratorForSingleChunk.java @@ -21,6 +21,7 @@ package org.sonar.server.issue.index; import com.google.common.base.CharMatcher; import com.google.common.base.Splitter; +import java.io.IOException; import java.util.Collection; import java.util.HashMap; import java.util.Iterator; @@ -28,6 +29,8 @@ import java.util.Optional; import javax.annotation.CheckForNull; import javax.annotation.Nullable; import org.apache.ibatis.cursor.Cursor; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.sonar.api.resources.Qualifiers; import org.sonar.api.resources.Scopes; import org.sonar.api.rules.CleanCodeAttribute; @@ -48,11 +51,13 @@ import static org.sonar.server.security.SecurityStandards.fromSecurityStandards; * the issues index */ class IssueIteratorForSingleChunk implements IssueIterator { + private static final Logger LOG = LoggerFactory.getLogger(IssueIteratorForSingleChunk.class); static final Splitter STRING_LIST_SPLITTER = Splitter.on(',').trimResults().omitEmptyStrings(); private final DbSession session; + private final Cursor indexCursor; private final Iterator iterator; IssueIteratorForSingleChunk(DbClient dbClient, @Nullable String branchUuid, @Nullable Collection issueKeys) { @@ -60,7 +65,7 @@ class IssueIteratorForSingleChunk implements IssueIterator { "Cannot search for more than " + DatabaseUtils.PARTITION_SIZE_FOR_ORACLE + " issue keys at once. Please provide the keys in smaller chunks."); this.session = dbClient.openSession(false); try { - Cursor indexCursor = dbClient.issueDao().scrollIssuesForIndexation(session, branchUuid, issueKeys); + indexCursor = dbClient.issueDao().scrollIssuesForIndexation(session, branchUuid, issueKeys); iterator = indexCursor.iterator(); } catch (Exception e) { session.close(); @@ -75,6 +80,7 @@ class IssueIteratorForSingleChunk implements IssueIterator { @Override public IssueDoc next() { + return toIssueDoc(iterator.next()); } @@ -170,6 +176,11 @@ class IssueIteratorForSingleChunk implements IssueIterator { @Override public void close() { + try { + indexCursor.close(); + } catch (IOException e) { + LOG.atWarn().setMessage("unable to close the cursor, this may lead to database connexion leak. error is : {}").addArgument(e).log(); + } session.close(); } } -- 2.39.5