From 1e8e115ae162d673fd7ed46dcf552db477772d86 Mon Sep 17 00:00:00 2001 From: Henning Andersen <33268011+henningandersen@users.noreply.github.com> Date: Tue, 9 Jun 2020 21:53:15 +0200 Subject: [PATCH] Rollover avoid heavy lifting in dry-run/validation (#57894) Fixed two newly introduced issues with rollover: 1. Using auto-expand replicas, rollover could result in unexpected log messages on future indexes. 2. It did a reroute and other heavy work on the network thread. Closes #57706 Supersedes #57865 Relates #53965 --- .../admin/indices/rollover/RolloverIT.java | 32 ++++++++ .../rollover/MetadataRolloverService.java | 17 +++-- .../rollover/TransportRolloverAction.java | 5 +- .../MetadataRolloverServiceTests.java | 75 ++++++++++++++++++- 4 files changed, 119 insertions(+), 10 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/action/admin/indices/rollover/RolloverIT.java b/server/src/internalClusterTest/java/org/elasticsearch/action/admin/indices/rollover/RolloverIT.java index 294c668427f..8f67d167bd2 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/action/admin/indices/rollover/RolloverIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/action/admin/indices/rollover/RolloverIT.java @@ -19,11 +19,18 @@ package org.elasticsearch.action.admin.indices.rollover; +import org.apache.logging.log4j.Level; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.elasticsearch.ResourceAlreadyExistsException; import org.elasticsearch.action.admin.indices.alias.Alias; import org.elasticsearch.action.admin.indices.settings.get.GetSettingsResponse; +import org.elasticsearch.action.admin.indices.template.put.PutIndexTemplateRequestBuilder; import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.metadata.AutoExpandReplicas; import org.elasticsearch.cluster.metadata.IndexMetadata; +import org.elasticsearch.cluster.routing.allocation.AllocationService; +import org.elasticsearch.common.logging.Loggers; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.time.DateFormatter; import org.elasticsearch.common.unit.ByteSizeUnit; @@ -32,6 +39,7 @@ import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.test.ESIntegTestCase; import org.elasticsearch.test.InternalSettingsPlugin; +import org.elasticsearch.test.MockLogAppender; import java.time.ZoneOffset; import java.time.ZonedDateTime; @@ -181,10 +189,34 @@ public class RolloverIT extends ESIntegTestCase { } public void testRolloverDryRun() throws Exception { + if (randomBoolean()) { + PutIndexTemplateRequestBuilder putTemplate = client().admin().indices() + .preparePutTemplate("test_index") + .setPatterns(Collections.singletonList("test_index-*")) + .setOrder(-1) + .setSettings(Settings.builder().put(AutoExpandReplicas.SETTING.getKey(), "0-all")); + assertAcked(putTemplate.get()); + } assertAcked(prepareCreate("test_index-1").addAlias(new Alias("test_alias")).get()); index("test_index-1", "type1", "1", "field", "value"); flush("test_index-1"); + ensureGreen(); + Logger allocationServiceLogger = LogManager.getLogger(AllocationService.class); + + MockLogAppender appender = new MockLogAppender(); + appender.start(); + appender.addExpectation( + new MockLogAppender.UnseenEventExpectation("no related message logged on dry run", + AllocationService.class.getName(), Level.INFO, "*test_index*") + ); + Loggers.addAppender(allocationServiceLogger, appender); + final RolloverResponse response = client().admin().indices().prepareRolloverIndex("test_alias").dryRun(true).get(); + + appender.assertAllExpectationsMatched(); + appender.stop(); + Loggers.removeAppender(allocationServiceLogger, appender); + assertThat(response.getOldIndex(), equalTo("test_index-1")); assertThat(response.getNewIndex(), equalTo("test_index-000002")); assertThat(response.isDryRun(), equalTo(true)); diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/rollover/MetadataRolloverService.java b/server/src/main/java/org/elasticsearch/action/admin/indices/rollover/MetadataRolloverService.java index aef05f4022f..9beab3b6ecf 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/rollover/MetadataRolloverService.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/rollover/MetadataRolloverService.java @@ -90,16 +90,16 @@ public class MetadataRolloverService { public RolloverResult rolloverClusterState(ClusterState currentState, String rolloverTarget, String newIndexName, CreateIndexRequest createIndexRequest, List> metConditions, - boolean silent) throws Exception { + boolean silent, boolean onlyValidate) throws Exception { validate(currentState.metadata(), rolloverTarget, newIndexName, createIndexRequest); final IndexAbstraction indexAbstraction = currentState.metadata().getIndicesLookup().get(rolloverTarget); switch (indexAbstraction.getType()) { case ALIAS: return rolloverAlias(currentState, (IndexAbstraction.Alias) indexAbstraction, rolloverTarget, newIndexName, - createIndexRequest, metConditions, silent); + createIndexRequest, metConditions, silent, onlyValidate); case DATA_STREAM: return rolloverDataStream(currentState, (IndexAbstraction.DataStream) indexAbstraction, rolloverTarget, - createIndexRequest, metConditions, silent); + createIndexRequest, metConditions, silent, onlyValidate); default: // the validate method above prevents this case throw new IllegalStateException("unable to roll over type [" + indexAbstraction.getType().getDisplayName() + "]"); @@ -108,7 +108,7 @@ public class MetadataRolloverService { private RolloverResult rolloverAlias(ClusterState currentState, IndexAbstraction.Alias alias, String aliasName, String newIndexName, CreateIndexRequest createIndexRequest, List> metConditions, - boolean silent) throws Exception { + boolean silent, boolean onlyValidate) throws Exception { final Metadata metadata = currentState.metadata(); final IndexMetadata writeIndex = alias.getWriteIndex(); final AliasMetadata aliasMetadata = writeIndex.getAliases().get(alias.getName()); @@ -124,6 +124,9 @@ public class MetadataRolloverService { IndexMetadata.INDEX_HIDDEN_SETTING.get(createIndexRequest.settings()) : null; createIndexService.validateIndexName(rolloverIndexName, currentState); // fails if the index already exists checkNoDuplicatedAliasInIndexTemplate(metadata, rolloverIndexName, aliasName, isHidden); + if (onlyValidate) { + return new RolloverResult(rolloverIndexName, sourceIndexName, currentState); + } CreateIndexClusterStateUpdateRequest createIndexClusterStateRequest = prepareCreateIndexRequest(unresolvedName, rolloverIndexName, createIndexRequest); @@ -142,12 +145,16 @@ public class MetadataRolloverService { private RolloverResult rolloverDataStream(ClusterState currentState, IndexAbstraction.DataStream dataStream, String dataStreamName, CreateIndexRequest createIndexRequest, List> metConditions, - boolean silent) throws Exception { + boolean silent, boolean onlyValidate) throws Exception { lookupTemplateForDataStream(dataStreamName, currentState.metadata()); final DataStream ds = dataStream.getDataStream(); final IndexMetadata originalWriteIndex = dataStream.getWriteIndex(); final String newWriteIndexName = DataStream.getDefaultBackingIndexName(ds.getName(), ds.getGeneration() + 1); + createIndexService.validateIndexName(newWriteIndexName, currentState); // fails if the index already exists + if (onlyValidate) { + return new RolloverResult(newWriteIndexName, originalWriteIndex.getIndex().getName(), currentState); + } CreateIndexClusterStateUpdateRequest createIndexClusterStateRequest = prepareDataStreamCreateIndexRequest(dataStreamName, newWriteIndexName, createIndexRequest); diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverAction.java b/server/src/main/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverAction.java index 04f850a6824..169c1f98ece 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverAction.java @@ -103,10 +103,11 @@ public class TransportRolloverAction extends TransportMasterNodeAction listener) throws Exception { + MetadataRolloverService.RolloverResult preResult = rolloverService.rolloverClusterState(state, rolloverRequest.getRolloverTarget(), rolloverRequest.getNewIndexName(), rolloverRequest.getCreateIndexRequest(), - Collections.emptyList(), true); + Collections.emptyList(), true, true); Metadata metadata = state.metadata(); String sourceIndexName = preResult.sourceIndexName; String rolloverIndexName = preResult.rolloverIndexName; @@ -136,7 +137,7 @@ public class TransportRolloverAction extends TransportMasterNodeAction rolloverService.rolloverClusterState(clusterState, rolloverTarget, null, new CreateIndexRequest("_na_"), null, + randomBoolean(), randomBoolean())); + + verify(createIndexService).validateIndexName(any(), same(clusterState)); + verifyZeroInteractions(createIndexService); + verifyZeroInteractions(metadataIndexAliasesService); + } + public void testRolloverClusterStateForDataStreamNoTemplate() throws Exception { final DataStream dataStream = DataStreamTests.randomInstance(); Metadata.Builder builder = Metadata.builder(); @@ -614,7 +683,7 @@ public class MetadataRolloverServiceTests extends ESTestCase { CreateIndexRequest createIndexRequest = new CreateIndexRequest("_na_"); Exception e = expectThrows(IllegalArgumentException.class, () -> rolloverService.rolloverClusterState(clusterState, - dataStream.getName(), null, createIndexRequest, metConditions, false)); + dataStream.getName(), null, createIndexRequest, metConditions, false, randomBoolean())); assertThat(e.getMessage(), equalTo("no matching index template found for data stream [" + dataStream.getName() + "]")); }