Skip to content

Commit

Permalink
SONAR-21655 ensure issue indexing chunks are properly closed when ite…
Browse files Browse the repository at this point in the history
…rating over large batches.

(cherry picked from commit 95e6a254696765e8984a7a13f951fc7e6baf1736)
  • Loading branch information
steve-marion-sonarsource authored and sonartech committed Feb 23, 2024
1 parent d65a69b commit edc71bb
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -27,15 +28,19 @@
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;
Expand Down Expand Up @@ -64,6 +69,7 @@
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;
Expand All @@ -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<IssueDto> 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<DatabaseUtils> dbUtils = Mockito.mockStatic(DatabaseUtils.class)) {
dbUtils.when(() -> DatabaseUtils.toUniqueAndSortedPartitions(Mockito.any())).thenAnswer(invocation -> {
Collection<String> 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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,10 @@ public boolean hasNext() {

@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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,16 @@

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;
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;
Expand All @@ -48,19 +51,21 @@
* 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<IndexedIssueDto> indexCursor;
private final Iterator<IndexedIssueDto> iterator;

IssueIteratorForSingleChunk(DbClient dbClient, @Nullable String branchUuid, @Nullable Collection<String> issueKeys) {
checkArgument(issueKeys == null || issueKeys.size() <= DatabaseUtils.PARTITION_SIZE_FOR_ORACLE,
"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<IndexedIssueDto> indexCursor = dbClient.issueDao().scrollIssuesForIndexation(session, branchUuid, issueKeys);
indexCursor = dbClient.issueDao().scrollIssuesForIndexation(session, branchUuid, issueKeys);
iterator = indexCursor.iterator();
} catch (Exception e) {
session.close();
Expand All @@ -75,6 +80,7 @@ public boolean hasNext() {

@Override
public IssueDoc next() {

return toIssueDoc(iterator.next());
}

Expand Down Expand Up @@ -170,6 +176,11 @@ private static String extractFilePath(@Nullable String filePath, String scope) {

@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();
}
}

0 comments on commit edc71bb

Please sign in to comment.