From 5997adeac3ac0df15dca2080ba216ed111e89c49 Mon Sep 17 00:00:00 2001 From: Eric Hartmann Date: Thu, 21 Sep 2017 15:06:55 +0200 Subject: [PATCH] GOV-285 Make DeleteAction resilient --- .../sonar/server/view/index/ViewIndex.java | 10 --- .../sonar/server/view/index/ViewIndexer.java | 56 +++++++++++++-- .../server/view/index/ViewIndexTest.java | 15 ---- .../server/view/index/ViewIndexerTest.java | 72 +++++++++++++++++++ 4 files changed, 122 insertions(+), 31 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/view/index/ViewIndex.java b/server/sonar-server/src/main/java/org/sonar/server/view/index/ViewIndex.java index 4b7dc2f4936..d27f925df34 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/view/index/ViewIndex.java +++ b/server/sonar-server/src/main/java/org/sonar/server/view/index/ViewIndex.java @@ -19,7 +19,6 @@ */ package org.sonar.server.view.index; -import java.util.Collection; import java.util.List; import org.elasticsearch.action.search.SearchRequestBuilder; import org.elasticsearch.action.search.SearchResponse; @@ -28,13 +27,10 @@ import org.elasticsearch.search.SearchHit; import org.elasticsearch.search.sort.SortOrder; import org.sonar.api.ce.ComputeEngineSide; import org.sonar.api.server.ServerSide; -import org.sonar.server.es.BulkIndexer; import org.sonar.server.es.EsClient; import static com.google.common.collect.Lists.newArrayList; -import static org.elasticsearch.index.query.QueryBuilders.boolQuery; import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery; -import static org.elasticsearch.index.query.QueryBuilders.termsQuery; @ServerSide @ComputeEngineSide @@ -75,10 +71,4 @@ public class ViewIndex { } return result; } - - public void delete(Collection viewUuids) { - SearchRequestBuilder searchRequest = esClient.prepareSearch(ViewIndexDefinition.INDEX_TYPE_VIEW) - .setQuery(boolQuery().must(matchAllQuery()).filter(termsQuery(ViewIndexDefinition.FIELD_UUID, viewUuids))); - BulkIndexer.delete(esClient, ViewIndexDefinition.INDEX_TYPE_VIEW, searchRequest); - } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/view/index/ViewIndexer.java b/server/sonar-server/src/main/java/org/sonar/server/view/index/ViewIndexer.java index 313599b80ea..a679232e9d1 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/view/index/ViewIndexer.java +++ b/server/sonar-server/src/main/java/org/sonar/server/view/index/ViewIndexer.java @@ -20,24 +20,31 @@ package org.sonar.server.view.index; import com.google.common.collect.ImmutableSet; +import java.util.Collection; import java.util.List; import java.util.Map; import java.util.Set; import org.elasticsearch.action.index.IndexRequest; +import org.sonar.core.util.stream.MoreCollectors; import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.component.ComponentDto; import org.sonar.db.component.UuidWithProjectUuidDto; +import org.sonar.db.es.EsQueueDto; import org.sonar.server.es.BulkIndexer; import org.sonar.server.es.BulkIndexer.Size; import org.sonar.server.es.EsClient; import org.sonar.server.es.IndexType; -import org.sonar.server.es.StartupIndexer; +import org.sonar.server.es.IndexingListener; +import org.sonar.server.es.IndexingResult; +import org.sonar.server.es.OneToOneResilientIndexingListener; +import org.sonar.server.es.ResilientIndexer; import static com.google.common.collect.Maps.newHashMap; +import static org.sonar.core.util.stream.MoreCollectors.toHashSet; import static org.sonar.server.view.index.ViewIndexDefinition.INDEX_TYPE_VIEW; -public class ViewIndexer implements StartupIndexer { +public class ViewIndexer implements ResilientIndexer { private final DbClient dbClient; private final EsClient esClient; @@ -70,15 +77,12 @@ public class ViewIndexer implements StartupIndexer { * The views lookup cache will be cleared */ public void index(String rootViewUuid) { - DbSession dbSession = dbClient.openSession(false); - try { + try (DbSession dbSession = dbClient.openSession(false)) { Map viewAndProjectViewUuidMap = newHashMap(); for (ComponentDto viewOrSubView : dbClient.componentDao().selectEnabledDescendantModules(dbSession, rootViewUuid)) { viewAndProjectViewUuidMap.put(viewOrSubView.uuid(), viewOrSubView.projectUuid()); } index(dbSession, viewAndProjectViewUuidMap, true, Size.REGULAR); - } finally { - dbSession.close(); } } @@ -129,4 +133,44 @@ public class ViewIndexer implements StartupIndexer { } } + /** + * This is based on the fact that a WebService is only calling {@link ViewIndexer#delete(DbSession, Collection)} + * So the resiliency is only taking in account a deletion of view component + * A safety check is done by not deleting any component that still exist in database. + * + * This should not occur but prevent any misuse on this resiliency + */ + @Override + public IndexingResult index(DbSession dbSession, Collection items) { + if (items.isEmpty()) { + return new IndexingResult(); + } + + Set views = items + .stream() + .map(EsQueueDto::getDocId) + .collect(toHashSet(items.size())); + + BulkIndexer bulkIndexer = newBulkIndexer(Size.REGULAR, new OneToOneResilientIndexingListener(dbClient, dbSession, items)); + bulkIndexer.start(); + + // Safety check to remove all views that may not have been deleted + views.removeAll(dbClient.componentDao().selectExistingUuids(dbSession, views)); + views.forEach(v -> bulkIndexer.addDeletion(INDEX_TYPE_VIEW, v)); + return bulkIndexer.stop(); + } + + public void delete(DbSession dbSession, Collection viewUuids) { + List items = viewUuids.stream() + .map(l -> EsQueueDto.create(INDEX_TYPE_VIEW.format(), l)) + .collect(MoreCollectors.toArrayList()); + + dbClient.esQueueDao().insert(dbSession, items); + dbSession.commit(); + index(dbSession, items); + } + + private BulkIndexer newBulkIndexer(Size bulkSize, IndexingListener listener) { + return new BulkIndexer(esClient, INDEX_TYPE_VIEW, bulkSize, listener); + } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/view/index/ViewIndexTest.java b/server/sonar-server/src/test/java/org/sonar/server/view/index/ViewIndexTest.java index dbe796ee666..a7a219de011 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/view/index/ViewIndexTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/view/index/ViewIndexTest.java @@ -55,19 +55,4 @@ public class ViewIndexTest { assertThat(result).isEmpty(); } - - @Test - public void delete_views() throws Exception { - ViewDoc view1 = new ViewDoc().setUuid("UUID1").setProjects(asList("P1")); - ViewDoc view2 = new ViewDoc().setUuid("UUID2").setProjects(asList("P2", "P3", "P4")); - ViewDoc view3 = new ViewDoc().setUuid("UUID3").setProjects(asList("P2", "P3", "P4")); - esTester.putDocuments(INDEX_TYPE_VIEW, view1); - esTester.putDocuments(INDEX_TYPE_VIEW, view2); - esTester.putDocuments(INDEX_TYPE_VIEW, view3); - - index.delete(asList(view1.uuid(), view2.uuid())); - - assertThat(esTester.getDocumentFieldValues(INDEX_TYPE_VIEW, ViewIndexDefinition.FIELD_UUID)).containsOnly(view3.uuid()); - } - } diff --git a/server/sonar-server/src/test/java/org/sonar/server/view/index/ViewIndexerTest.java b/server/sonar-server/src/test/java/org/sonar/server/view/index/ViewIndexerTest.java index 024605c79cf..1d2db1aa1f2 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/view/index/ViewIndexerTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/view/index/ViewIndexerTest.java @@ -22,9 +22,13 @@ package org.sonar.server.view.index; import com.google.common.collect.Maps; import java.util.List; import java.util.Map; +import java.util.function.BooleanSupplier; import org.elasticsearch.action.search.SearchResponse; import org.junit.Rule; import org.junit.Test; +import org.junit.rules.DisableOnDebug; +import org.junit.rules.TestRule; +import org.junit.rules.Timeout; import org.sonar.api.config.internal.MapSettings; import org.sonar.api.utils.System2; import org.sonar.db.DbClient; @@ -38,6 +42,7 @@ import org.sonar.db.organization.OrganizationDto; import org.sonar.db.rule.RuleDto; import org.sonar.db.rule.RuleTesting; import org.sonar.server.es.EsTester; +import org.sonar.server.es.RecoveryIndexer; import org.sonar.server.es.SearchOptions; import org.sonar.server.issue.IssueQuery; import org.sonar.server.issue.index.IssueIndex; @@ -49,14 +54,19 @@ import org.sonar.server.permission.index.PermissionIndexer; import org.sonar.server.tester.UserSessionRule; import static com.google.common.collect.Lists.newArrayList; +import static java.util.Arrays.asList; import static java.util.Collections.emptySet; import static org.assertj.core.api.Assertions.assertThat; import static org.sonar.db.component.ComponentTesting.newProjectCopy; +import static org.sonar.server.view.index.ViewIndexDefinition.INDEX_TYPE_VIEW; public class ViewIndexerTest { private System2 system2 = System2.INSTANCE; + @Rule + public TestRule safeguardTimeout = new DisableOnDebug(Timeout.seconds(60)); + @Rule public DbTester dbTester = DbTester.create(system2); @@ -192,6 +202,52 @@ public class ViewIndexerTest { assertThat(issueIndex.search(IssueQuery.builder().viewUuids(newArrayList(viewUuid)).build(), new SearchOptions()).getHits()).hasSize(2); } + @Test + public void delete_should_delete_the_view() { + ViewDoc view1 = new ViewDoc().setUuid("UUID1").setProjects(asList("P1")); + ViewDoc view2 = new ViewDoc().setUuid("UUID2").setProjects(asList("P2", "P3", "P4")); + ViewDoc view3 = new ViewDoc().setUuid("UUID3").setProjects(asList("P2", "P3", "P4")); + esTester.putDocuments(INDEX_TYPE_VIEW, view1); + esTester.putDocuments(INDEX_TYPE_VIEW, view2); + esTester.putDocuments(INDEX_TYPE_VIEW, view3); + + assertThat(esTester.getDocumentFieldValues(INDEX_TYPE_VIEW, ViewIndexDefinition.FIELD_UUID)) + .containsOnly(view1.uuid(), view2.uuid(), view3.uuid()); + + underTest.delete(dbSession, asList(view1.uuid(), view2.uuid())); + + assertThat(esTester.getDocumentFieldValues(INDEX_TYPE_VIEW, ViewIndexDefinition.FIELD_UUID)) + .containsOnly(view3.uuid()); + } + + @Test + public void delete_should_be_resilient() throws InterruptedException { + ViewDoc view1 = new ViewDoc().setUuid("UUID1").setProjects(asList("P1")); + ViewDoc view2 = new ViewDoc().setUuid("UUID2").setProjects(asList("P2", "P3", "P4")); + ViewDoc view3 = new ViewDoc().setUuid("UUID3").setProjects(asList("P2", "P3", "P4")); + esTester.putDocuments(INDEX_TYPE_VIEW, view1); + esTester.putDocuments(INDEX_TYPE_VIEW, view2); + esTester.putDocuments(INDEX_TYPE_VIEW, view3); + + assertThat(esTester.getDocumentFieldValues(INDEX_TYPE_VIEW, ViewIndexDefinition.FIELD_UUID)) + .containsOnly(view1.uuid(), view2.uuid(), view3.uuid()); + + // Lock writes + esTester.lockWrites(INDEX_TYPE_VIEW); + underTest.delete(dbSession, asList(view1.uuid(), view2.uuid())); + + assertThat(esTester.getDocumentFieldValues(INDEX_TYPE_VIEW, ViewIndexDefinition.FIELD_UUID)) + .containsOnly(view1.uuid(), view2.uuid(), view3.uuid()); + + // Unlock writes + esTester.unlockWrites(INDEX_TYPE_VIEW); + + doRecover(() -> esTester.getDocumentFieldValues(INDEX_TYPE_VIEW, ViewIndexDefinition.FIELD_UUID).size() == 3); + + assertThat(esTester.getDocumentFieldValues(INDEX_TYPE_VIEW, ViewIndexDefinition.FIELD_UUID)) + .containsOnly(view3.uuid()); + } + private ComponentDto addProjectWithIssue(RuleDto rule, OrganizationDto org) { ComponentDto project = ComponentTesting.newPublicProjectDto(org); ComponentDto file = ComponentTesting.newFileDto(project, null); @@ -204,4 +260,20 @@ public class ViewIndexerTest { return project; } + private void doRecover(BooleanSupplier condition) throws InterruptedException { + MapSettings settings = new MapSettings() + .setProperty("sonar.search.recovery.initialDelayInMs", "0") + .setProperty("sonar.search.recovery.minAgeInMs", "1") + .setProperty("sonar.search.recovery.delayInMs", "1"); + + RecoveryIndexer recoveryIndexer = new RecoveryIndexer(System2.INSTANCE, settings.asConfig(), dbClient, underTest); + recoveryIndexer.start(); + + // Wait for recovery + while (condition.getAsBoolean()) { + Thread.sleep(1_000); + } + + recoveryIndexer.stop(); + } } -- 2.39.5