From 218cf020139885bf9797b9a6a2413ad3e3517cde Mon Sep 17 00:00:00 2001 From: Gordon Brown Date: Thu, 3 Oct 2019 15:26:50 -0600 Subject: [PATCH 1/2] Fix Rollover error when alias has closed indices (#47148) Rollover previously requested index stats for all indices in the provided alias, which causes an exception when there is a closed index with that alias. This commit adjusts the IndicesOptions used on the index stats request so that closed indices are ignored, rather than throwing an exception. --- .../rollover/TransportRolloverAction.java | 5 +- .../admin/indices/rollover/RolloverIT.java | 51 +++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) 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 e0b8753de1b04..9e90ec7f0fdeb 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 @@ -124,7 +124,10 @@ protected void masterOperation(final RolloverRequest rolloverRequest, final Clus final String rolloverIndexName = indexNameExpressionResolver.resolveDateMathExpression(unresolvedName); MetaDataCreateIndexService.validateIndexName(rolloverIndexName, state); // will fail if the index already exists checkNoDuplicatedAliasInIndexTemplate(metaData, rolloverIndexName, rolloverRequest.getAlias()); - client.admin().indices().prepareStats(rolloverRequest.getAlias()).clear().setDocs(true).execute( + client.admin().indices().prepareStats(rolloverRequest.getAlias()) + .clear() + .setIndicesOptions(IndicesOptions.fromOptions(true, false, true, true)) + .setDocs(true).execute( new ActionListener() { @Override public void onResponse(IndicesStatsResponse statsResponse) { diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/rollover/RolloverIT.java b/server/src/test/java/org/elasticsearch/action/admin/indices/rollover/RolloverIT.java index 4926c90c0a691..1cabb0020fe7b 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/rollover/RolloverIT.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/rollover/RolloverIT.java @@ -40,6 +40,7 @@ import java.util.List; import java.util.Set; +import static org.elasticsearch.index.mapper.MapperService.SINGLE_MAPPING_NAME; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.equalTo; @@ -381,4 +382,54 @@ public void testRejectIfAliasFoundInTemplate() throws Exception { assertThat(error.getMessage(), equalTo( "Rollover alias [logs-write] can point to multiple indices, found duplicated alias [[logs-write]] in index template [logs]")); } + + public void testRolloverWithClosedIndexInAlias() throws Exception { + final String aliasName = "alias"; + final String openNonwriteIndex = "open-index-nonwrite"; + final String closedIndex = "closed-index-nonwrite"; + final String writeIndexPrefix = "write-index-"; + assertAcked(prepareCreate(openNonwriteIndex).addAlias(new Alias(aliasName)).get()); + assertAcked(prepareCreate(closedIndex).addAlias(new Alias(aliasName)).get()); + assertAcked(prepareCreate(writeIndexPrefix + "000001").addAlias(new Alias(aliasName).writeIndex(true)).get()); + + index(closedIndex, SINGLE_MAPPING_NAME, null, "{\"foo\": \"bar\"}"); + index(aliasName, SINGLE_MAPPING_NAME, null, "{\"foo\": \"bar\"}"); + index(aliasName, SINGLE_MAPPING_NAME, null, "{\"foo\": \"bar\"}"); + refresh(aliasName); + + assertAcked(client().admin().indices().prepareClose(closedIndex).get()); + + RolloverResponse rolloverResponse = client().admin().indices().prepareRolloverIndex(aliasName) + .addMaxIndexDocsCondition(1) + .get(); + assertTrue(rolloverResponse.isRolledOver()); + assertEquals(writeIndexPrefix + "000001", rolloverResponse.getOldIndex()); + assertEquals(writeIndexPrefix + "000002", rolloverResponse.getNewIndex()); + } + + public void testRolloverWithClosedWriteIndex() throws Exception { + final String aliasName = "alias"; + final String openNonwriteIndex = "open-index-nonwrite"; + final String closedIndex = "closed-index-nonwrite"; + final String writeIndexPrefix = "write-index-"; + assertAcked(prepareCreate(openNonwriteIndex).addAlias(new Alias(aliasName)).get()); + assertAcked(prepareCreate(closedIndex).addAlias(new Alias(aliasName)).get()); + assertAcked(prepareCreate(writeIndexPrefix + "000001").addAlias(new Alias(aliasName).writeIndex(true)).get()); + + index(closedIndex, SINGLE_MAPPING_NAME, null, "{\"foo\": \"bar\"}"); + index(aliasName, SINGLE_MAPPING_NAME, null, "{\"foo\": \"bar\"}"); + index(aliasName, SINGLE_MAPPING_NAME, null, "{\"foo\": \"bar\"}"); + refresh(aliasName); + + assertAcked(client().admin().indices().prepareClose(closedIndex).get()); + assertAcked(client().admin().indices().prepareClose(writeIndexPrefix + "000001").get()); + ensureGreen(aliasName); + + RolloverResponse rolloverResponse = client().admin().indices().prepareRolloverIndex(aliasName) + .addMaxIndexDocsCondition(1) + .get(); + assertTrue(rolloverResponse.isRolledOver()); + assertEquals(writeIndexPrefix + "000001", rolloverResponse.getOldIndex()); + assertEquals(writeIndexPrefix + "000002", rolloverResponse.getNewIndex()); + } } From 0fb9ad1cdeba1fa6338ba4d1544ae5d957938614 Mon Sep 17 00:00:00 2001 From: Gordon Brown Date: Fri, 4 Oct 2019 11:23:30 -0600 Subject: [PATCH 2/2] Fix testConditionEvaluationWhenAliasToWriteAndReadIndicesConsidersOnlyPrimariesFromWriteIndex --- .../rollover/TransportRolloverAction.java | 21 ++++++++--- .../TransportRolloverActionTests.java | 37 +++++++++---------- 2 files changed, 33 insertions(+), 25 deletions(-) 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 9e90ec7f0fdeb..ee01584f664b1 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 @@ -23,6 +23,8 @@ import org.elasticsearch.action.admin.indices.alias.IndicesAliasesClusterStateUpdateRequest; import org.elasticsearch.action.admin.indices.create.CreateIndexClusterStateUpdateRequest; import org.elasticsearch.action.admin.indices.create.CreateIndexRequest; +import org.elasticsearch.action.admin.indices.stats.IndicesStatsAction; +import org.elasticsearch.action.admin.indices.stats.IndicesStatsRequest; import org.elasticsearch.action.admin.indices.stats.IndicesStatsResponse; import org.elasticsearch.action.support.ActionFilters; import org.elasticsearch.action.support.ActiveShardCount; @@ -49,11 +51,12 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.unit.ByteSizeValue; import org.elasticsearch.index.shard.DocsStats; +import org.elasticsearch.tasks.Task; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportService; -import java.util.Arrays; import java.io.IOException; +import java.util.Arrays; import java.util.Collection; import java.util.List; import java.util.Locale; @@ -108,7 +111,13 @@ protected ClusterBlockException checkBlock(RolloverRequest request, ClusterState } @Override - protected void masterOperation(final RolloverRequest rolloverRequest, final ClusterState state, + protected void masterOperation(RolloverRequest request, ClusterState state, + ActionListener listener) throws Exception { + throw new UnsupportedOperationException("The task parameter is required"); + } + + @Override + protected void masterOperation(Task task, final RolloverRequest rolloverRequest, final ClusterState state, final ActionListener listener) { final MetaData metaData = state.metaData(); validate(metaData, rolloverRequest); @@ -124,10 +133,12 @@ protected void masterOperation(final RolloverRequest rolloverRequest, final Clus final String rolloverIndexName = indexNameExpressionResolver.resolveDateMathExpression(unresolvedName); MetaDataCreateIndexService.validateIndexName(rolloverIndexName, state); // will fail if the index already exists checkNoDuplicatedAliasInIndexTemplate(metaData, rolloverIndexName, rolloverRequest.getAlias()); - client.admin().indices().prepareStats(rolloverRequest.getAlias()) + IndicesStatsRequest statsRequest = new IndicesStatsRequest().indices(rolloverRequest.getAlias()) .clear() - .setIndicesOptions(IndicesOptions.fromOptions(true, false, true, true)) - .setDocs(true).execute( + .indicesOptions(IndicesOptions.fromOptions(true, false, true, true)) + .docs(true); + statsRequest.setParentTask(clusterService.localNode().getId(), task.getId()); + client.execute(IndicesStatsAction.INSTANCE, statsRequest, new ActionListener() { @Override public void onResponse(IndicesStatsResponse statsResponse) { diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverActionTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverActionTests.java index c24e0c65e00ce..ae3dcc819e335 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverActionTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverActionTests.java @@ -21,20 +21,19 @@ import org.elasticsearch.Version; import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.ActionRequest; import org.elasticsearch.action.admin.indices.alias.IndicesAliasesClusterStateUpdateRequest; import org.elasticsearch.action.admin.indices.create.CreateIndexClusterStateUpdateRequest; import org.elasticsearch.action.admin.indices.stats.CommonStats; import org.elasticsearch.action.admin.indices.stats.IndexStats; -import org.elasticsearch.action.admin.indices.stats.IndicesStatsRequestBuilder; +import org.elasticsearch.action.admin.indices.stats.IndicesStatsAction; import org.elasticsearch.action.admin.indices.stats.IndicesStatsResponse; import org.elasticsearch.action.admin.indices.stats.IndicesStatsTests; import org.elasticsearch.action.admin.indices.stats.ShardStats; import org.elasticsearch.action.support.ActionFilters; import org.elasticsearch.action.support.ActiveShardCount; import org.elasticsearch.action.support.PlainActionFuture; -import org.elasticsearch.client.AdminClient; import org.elasticsearch.client.Client; -import org.elasticsearch.client.IndicesAdminClient; import org.elasticsearch.cluster.ClusterName; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.metadata.AliasAction; @@ -45,6 +44,7 @@ import org.elasticsearch.cluster.metadata.MetaData; import org.elasticsearch.cluster.metadata.MetaDataCreateIndexService; import org.elasticsearch.cluster.metadata.MetaDataIndexAliasesService; +import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.cluster.routing.RecoverySource; import org.elasticsearch.cluster.routing.ShardRouting; import org.elasticsearch.cluster.routing.UnassignedInfo; @@ -71,6 +71,7 @@ import org.elasticsearch.index.store.StoreStats; import org.elasticsearch.index.warmer.WarmerStats; import org.elasticsearch.search.suggest.completion.CompletionStats; +import org.elasticsearch.tasks.Task; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportService; @@ -92,6 +93,7 @@ import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.is; import static org.mockito.Matchers.any; +import static org.mockito.Matchers.eq; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; @@ -366,9 +368,12 @@ public void testRejectDuplicateAlias() { assertThat(ex.getMessage(), containsString("index template [test-template]")); } - public void testConditionEvaluationWhenAliasToWriteAndReadIndicesConsidersOnlyPrimariesFromWriteIndex() { + public void testConditionEvaluationWhenAliasToWriteAndReadIndicesConsidersOnlyPrimariesFromWriteIndex() throws Exception { final TransportService mockTransportService = mock(TransportService.class); final ClusterService mockClusterService = mock(ClusterService.class); + final DiscoveryNode mockNode = mock(DiscoveryNode.class); + when(mockNode.getId()).thenReturn("mocknode"); + when(mockClusterService.localNode()).thenReturn(mockNode); final ThreadPool mockThreadPool = mock(ThreadPool.class); final MetaDataCreateIndexService mockCreateIndexService = mock(MetaDataCreateIndexService.class); final IndexNameExpressionResolver mockIndexNameExpressionResolver = mock(IndexNameExpressionResolver.class); @@ -377,31 +382,23 @@ public void testConditionEvaluationWhenAliasToWriteAndReadIndicesConsidersOnlyPr final MetaDataIndexAliasesService mdIndexAliasesService = mock(MetaDataIndexAliasesService.class); final Client mockClient = mock(Client.class); - final AdminClient mockAdminClient = mock(AdminClient.class); - final IndicesAdminClient mockIndicesAdminClient = mock(IndicesAdminClient.class); - when(mockClient.admin()).thenReturn(mockAdminClient); - when(mockAdminClient.indices()).thenReturn(mockIndicesAdminClient); - final IndicesStatsRequestBuilder mockIndicesStatsBuilder = mock(IndicesStatsRequestBuilder.class); - when(mockIndicesAdminClient.prepareStats(any())).thenReturn(mockIndicesStatsBuilder); final Map indexStats = new HashMap<>(); int total = randomIntBetween(500, 1000); indexStats.put("logs-index-000001", createIndexStats(200L, total)); indexStats.put("logs-index-000002", createIndexStats(300L, total)); final IndicesStatsResponse statsResponse = createAliasToMultipleIndicesStatsResponse(indexStats); - when(mockIndicesStatsBuilder.clear()).thenReturn(mockIndicesStatsBuilder); - when(mockIndicesStatsBuilder.setDocs(true)).thenReturn(mockIndicesStatsBuilder); - - assert statsResponse.getPrimaries().getDocs().getCount() == 500L; - assert statsResponse.getTotal().getDocs().getCount() == (total + total); doAnswer(invocation -> { Object[] args = invocation.getArguments(); - assert args.length == 1; - ActionListener listener = (ActionListener) args[0]; + assert args.length == 3; + ActionListener listener = (ActionListener) args[2]; listener.onResponse(statsResponse); return null; - }).when(mockIndicesStatsBuilder).execute(any(ActionListener.class)); + }).when(mockClient).execute(eq(IndicesStatsAction.INSTANCE), any(ActionRequest.class), any(ActionListener.class)); + + assert statsResponse.getPrimaries().getDocs().getCount() == 500L; + assert statsResponse.getTotal().getDocs().getCount() == (total + total); final IndexMetaData.Builder indexMetaData = IndexMetaData.builder("logs-index-000001") .putAlias(AliasMetaData.builder("logs-alias").writeIndex(false).build()).settings(settings(Version.CURRENT)) @@ -422,7 +419,7 @@ public void testConditionEvaluationWhenAliasToWriteAndReadIndicesConsidersOnlyPr RolloverRequest rolloverRequest = new RolloverRequest("logs-alias", "logs-index-000003"); rolloverRequest.addMaxIndexDocsCondition(500L); rolloverRequest.dryRun(true); - transportRolloverAction.masterOperation(rolloverRequest, stateBefore, future); + transportRolloverAction.masterOperation(mock(Task.class), rolloverRequest, stateBefore, future); RolloverResponse response = future.actionGet(); assertThat(response.getOldIndex(), equalTo("logs-index-000002")); @@ -438,7 +435,7 @@ public void testConditionEvaluationWhenAliasToWriteAndReadIndicesConsidersOnlyPr rolloverRequest = new RolloverRequest("logs-alias", "logs-index-000003"); rolloverRequest.addMaxIndexDocsCondition(300L); rolloverRequest.dryRun(true); - transportRolloverAction.masterOperation(rolloverRequest, stateBefore, future); + transportRolloverAction.masterOperation(mock(Task.class), rolloverRequest, stateBefore, future); response = future.actionGet(); assertThat(response.getOldIndex(), equalTo("logs-index-000002"));