From 2e33171d6696e4ae3e3e6082f21b7e186149a926 Mon Sep 17 00:00:00 2001 From: Gaurav Chandani Date: Thu, 28 May 2020 14:13:00 +0530 Subject: [PATCH 1/4] Normalized prefix for rollover API It fixes the issue https://github.com/elastic/elasticsearch/issues/53388 by normalizing prefix at index creation request itself --- .../action/admin/indices/rollover/RolloverIT.java | 2 +- .../indices/rollover/MetadataRolloverService.java | 1 - .../metadata/MetadataCreateIndexService.java | 13 +++++++++---- .../rollover/MetadataRolloverServiceTests.java | 5 +++-- 4 files changed, 13 insertions(+), 8 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 56e00e0575105..dc4bb07560bbe 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 @@ -154,7 +154,7 @@ public void testRolloverWithIndexSettings() throws Exception { indexDoc("test_index-2", "1", "field", "value"); flush("test_index-2"); final Settings settings = Settings.builder() - .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1) + .put("number_of_shards", 1) // testing without "index" prefix .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0) .build(); final RolloverResponse response = client().admin().indices().prepareRolloverIndex("test_alias") 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 0c6e178274f1d..a7b942d6b0c24 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 @@ -62,7 +62,6 @@ public class MetadataRolloverService { private final MetadataCreateIndexService createIndexService; private final MetadataIndexAliasesService indexAliasesService; private final IndexNameExpressionResolver indexNameExpressionResolver; - @Inject public MetadataRolloverService(ThreadPool threadPool, MetadataCreateIndexService createIndexService, MetadataIndexAliasesService indexAliasesService, diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexService.java b/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexService.java index 93c245afd438a..944683a1cc39f 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexService.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexService.java @@ -277,10 +277,7 @@ public void createIndex(final CreateIndexClusterStateUpdateRequest request, private void onlyCreateIndex(final CreateIndexClusterStateUpdateRequest request, final ActionListener listener) { - Settings.Builder updatedSettingsBuilder = Settings.builder(); - Settings build = updatedSettingsBuilder.put(request.settings()).normalizePrefix(IndexMetadata.INDEX_SETTING_PREFIX).build(); - indexScopedSettings.validate(build, true); // we do validate here - index setting must be consistent - request.settings(build); + applyCreateIndexRequestInternal(request); clusterService.submitStateUpdateTask( "create-index [" + request.index() + "], cause [" + request.cause() + "]", new AckedClusterStateUpdateTask<>(Priority.URGENT, request, listener) { @@ -306,12 +303,20 @@ public void onFailure(String source, Exception e) { }); } + private void applyCreateIndexRequestInternal(CreateIndexClusterStateUpdateRequest createIndexClusterStateRequest){ + Settings.Builder updatedSettingsBuilder = Settings.builder(); + Settings build = updatedSettingsBuilder.put(createIndexClusterStateRequest.settings()).normalizePrefix(IndexMetadata.INDEX_SETTING_PREFIX).build(); + indexScopedSettings.validate(build,true); + createIndexClusterStateRequest.settings(build); + } /** * Handles the cluster state transition to a version that reflects the {@link CreateIndexClusterStateUpdateRequest}. * All the requested changes are firstly validated before mutating the {@link ClusterState}. */ public ClusterState applyCreateIndexRequest(ClusterState currentState, CreateIndexClusterStateUpdateRequest request, boolean silent, BiConsumer metadataTransformer) throws Exception { + + applyCreateIndexRequestInternal(request); logger.trace("executing IndexCreationTask for [{}] against cluster state version [{}]", request, currentState.version()); validate(request, currentState); diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/rollover/MetadataRolloverServiceTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/rollover/MetadataRolloverServiceTests.java index 11d21ea3e6926..b7ff766a07fe6 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/rollover/MetadataRolloverServiceTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/rollover/MetadataRolloverServiceTests.java @@ -47,6 +47,7 @@ import org.elasticsearch.common.CheckedFunction; import org.elasticsearch.common.Strings; import org.elasticsearch.common.UUIDs; +import org.elasticsearch.common.settings.IndexScopedSettings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.xcontent.json.JsonXContent; @@ -471,7 +472,7 @@ public void testRolloverClusterState() throws Exception { when(mockIndexNameExpressionResolver.resolveDateMathExpression(any())).then(returnsFirstArg()); MetadataCreateIndexService createIndexService = new MetadataCreateIndexService(Settings.EMPTY, - clusterService, indicesService, allocationService, null, env, null, testThreadPool, null, Collections.emptyList(), false); + clusterService, indicesService, allocationService, null, env, IndexScopedSettings.DEFAULT_SCOPED_SETTINGS, testThreadPool, null, Collections.emptyList(), false); MetadataIndexAliasesService indexAliasesService = new MetadataIndexAliasesService(clusterService, indicesService, new AliasValidator(), null, xContentRegistry()); MetadataRolloverService rolloverService = new MetadataRolloverService(testThreadPool, createIndexService, indexAliasesService, @@ -536,7 +537,7 @@ public void testRolloverClusterStateForDataStream() throws Exception { when(mockIndexNameExpressionResolver.resolveDateMathExpression(any())).then(returnsFirstArg()); MetadataCreateIndexService createIndexService = new MetadataCreateIndexService(Settings.EMPTY, - clusterService, indicesService, allocationService, null, env, null, testThreadPool, null, Collections.emptyList(), false); + clusterService, indicesService, allocationService, null, env, IndexScopedSettings.DEFAULT_SCOPED_SETTINGS, testThreadPool, null, Collections.emptyList(), false); MetadataIndexAliasesService indexAliasesService = new MetadataIndexAliasesService(clusterService, indicesService, new AliasValidator(), null, xContentRegistry()); MetadataRolloverService rolloverService = new MetadataRolloverService(testThreadPool, createIndexService, indexAliasesService, From 3a7ecb88cffa0f856b634922f86e8789879cbc45 Mon Sep 17 00:00:00 2001 From: Gaurav Chandani Date: Wed, 10 Jun 2020 17:46:57 +0530 Subject: [PATCH 2/4] Added new Test case and renamed methods --- .../admin/indices/rollover/RolloverIT.java | 37 ++++++++++++++++++- .../metadata/MetadataCreateIndexService.java | 6 +-- 2 files changed, 39 insertions(+), 4 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 c0ab8966b44b7..acae80f5c93b3 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 @@ -162,7 +162,7 @@ public void testRolloverWithIndexSettings() throws Exception { indexDoc("test_index-2", "1", "field", "value"); flush("test_index-2"); final Settings settings = Settings.builder() - .put("number_of_shards", 1) // testing without "index" prefix + .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1) .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0) .build(); final RolloverResponse response = client().admin().indices().prepareRolloverIndex("test_alias") @@ -187,6 +187,41 @@ public void testRolloverWithIndexSettings() throws Exception { } } + public void testRolloverWithIndexSettingsWithoutPrefix() throws Exception { + Alias testAlias = new Alias("test_alias"); + boolean explicitWriteIndex = randomBoolean(); + if (explicitWriteIndex) { + testAlias.writeIndex(true); + } + assertAcked(prepareCreate("test_index-2").addAlias(testAlias).get()); + indexDoc("test_index-2", "1", "field", "value"); + flush("test_index-2"); + final Settings settings = Settings.builder() + .put("number_of_shards", 1) + .put("number_of_replicas", 0) + .build(); + final RolloverResponse response = client().admin().indices().prepareRolloverIndex("test_alias") + .settings(settings).alias(new Alias("extra_alias")).get(); + assertThat(response.getOldIndex(), equalTo("test_index-2")); + assertThat(response.getNewIndex(), equalTo("test_index-000003")); + assertThat(response.isDryRun(), equalTo(false)); + assertThat(response.isRolledOver(), equalTo(true)); + assertThat(response.getConditionStatus().size(), equalTo(0)); + final ClusterState state = client().admin().cluster().prepareState().get().getState(); + final IndexMetadata oldIndex = state.metadata().index("test_index-2"); + final IndexMetadata newIndex = state.metadata().index("test_index-000003"); + assertThat(newIndex.getNumberOfShards(), equalTo(1)); + assertThat(newIndex.getNumberOfReplicas(), equalTo(0)); + assertTrue(newIndex.getAliases().containsKey("test_alias")); + assertTrue(newIndex.getAliases().containsKey("extra_alias")); + if (explicitWriteIndex) { + assertFalse(oldIndex.getAliases().get("test_alias").writeIndex()); + assertTrue(newIndex.getAliases().get("test_alias").writeIndex()); + } else { + assertFalse(oldIndex.getAliases().containsKey("test_alias")); + } + } + public void testRolloverDryRun() throws Exception { if (randomBoolean()) { PutIndexTemplateRequestBuilder putTemplate = client().admin().indices() diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexService.java b/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexService.java index f422d1c35237e..4d92f50e28f62 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexService.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexService.java @@ -280,7 +280,7 @@ public void createIndex(final CreateIndexClusterStateUpdateRequest request, private void onlyCreateIndex(final CreateIndexClusterStateUpdateRequest request, final ActionListener listener) { - applyCreateIndexRequestInternal(request); + normalizeRequestSetting(request); clusterService.submitStateUpdateTask( "create-index [" + request.index() + "], cause [" + request.cause() + "]", new AckedClusterStateUpdateTask<>(Priority.URGENT, request, listener) { @@ -306,7 +306,7 @@ public void onFailure(String source, Exception e) { }); } - private void applyCreateIndexRequestInternal(CreateIndexClusterStateUpdateRequest createIndexClusterStateRequest){ + private void normalizeRequestSetting(CreateIndexClusterStateUpdateRequest createIndexClusterStateRequest){ Settings.Builder updatedSettingsBuilder = Settings.builder(); Settings build = updatedSettingsBuilder.put(createIndexClusterStateRequest.settings()).normalizePrefix(IndexMetadata.INDEX_SETTING_PREFIX).build(); indexScopedSettings.validate(build,true); @@ -319,7 +319,7 @@ private void applyCreateIndexRequestInternal(CreateIndexClusterStateUpdateReques public ClusterState applyCreateIndexRequest(ClusterState currentState, CreateIndexClusterStateUpdateRequest request, boolean silent, BiConsumer metadataTransformer) throws Exception { - applyCreateIndexRequestInternal(request); + normalizeRequestSetting(request); logger.trace("executing IndexCreationTask for [{}] against cluster state version [{}]", request, currentState.version()); validate(request, currentState); From 1f68cb6bad2d10e865d1289ed29ff940b23a4a5b Mon Sep 17 00:00:00 2001 From: Gaurav Chandani Date: Fri, 12 Jun 2020 17:32:08 +0530 Subject: [PATCH 3/4] Fixed checkstyle errors --- .../cluster/metadata/MetadataCreateIndexService.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexService.java b/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexService.java index 4d92f50e28f62..288a404608da7 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexService.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexService.java @@ -306,10 +306,11 @@ public void onFailure(String source, Exception e) { }); } - private void normalizeRequestSetting(CreateIndexClusterStateUpdateRequest createIndexClusterStateRequest){ + private void normalizeRequestSetting(CreateIndexClusterStateUpdateRequest createIndexClusterStateRequest) { Settings.Builder updatedSettingsBuilder = Settings.builder(); - Settings build = updatedSettingsBuilder.put(createIndexClusterStateRequest.settings()).normalizePrefix(IndexMetadata.INDEX_SETTING_PREFIX).build(); - indexScopedSettings.validate(build,true); + Settings build = updatedSettingsBuilder.put(createIndexClusterStateRequest.settings()) + .normalizePrefix(IndexMetadata.INDEX_SETTING_PREFIX).build(); + indexScopedSettings.validate(build, true); createIndexClusterStateRequest.settings(build); } /** From 39e2c85119c4a7b9dd594eea95bce4d82f8e4da0 Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Mon, 15 Jun 2020 15:03:33 -0600 Subject: [PATCH 4/4] Fix checkstyle in MetadataRolloverServiceTests --- .../indices/rollover/MetadataRolloverServiceTests.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/rollover/MetadataRolloverServiceTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/rollover/MetadataRolloverServiceTests.java index 282a8f221b80f..86ac6ecaa7ce4 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/rollover/MetadataRolloverServiceTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/rollover/MetadataRolloverServiceTests.java @@ -484,8 +484,8 @@ public void testRolloverClusterState() throws Exception { ShardLimitValidator shardLimitValidator = new ShardLimitValidator(Settings.EMPTY, clusterService); MetadataCreateIndexService createIndexService = new MetadataCreateIndexService(Settings.EMPTY, - clusterService, indicesService, allocationService, null, shardLimitValidator, env, IndexScopedSettings.DEFAULT_SCOPED_SETTINGS, - testThreadPool, null, Collections.emptyList(), false); + clusterService, indicesService, allocationService, null, shardLimitValidator, env, + IndexScopedSettings.DEFAULT_SCOPED_SETTINGS, testThreadPool, null, Collections.emptyList(), false); MetadataIndexAliasesService indexAliasesService = new MetadataIndexAliasesService(clusterService, indicesService, new AliasValidator(), null, xContentRegistry()); MetadataRolloverService rolloverService = new MetadataRolloverService(testThreadPool, createIndexService, indexAliasesService, @@ -561,8 +561,8 @@ public void testRolloverClusterStateForDataStream() throws Exception { ShardLimitValidator shardLimitValidator = new ShardLimitValidator(Settings.EMPTY, clusterService); MetadataCreateIndexService createIndexService = new MetadataCreateIndexService(Settings.EMPTY, - clusterService, indicesService, allocationService, null, shardLimitValidator, env, IndexScopedSettings.DEFAULT_SCOPED_SETTINGS, - testThreadPool, null, Collections.emptyList(), false); + clusterService, indicesService, allocationService, null, shardLimitValidator, env, + IndexScopedSettings.DEFAULT_SCOPED_SETTINGS, testThreadPool, null, Collections.emptyList(), false); MetadataIndexAliasesService indexAliasesService = new MetadataIndexAliasesService(clusterService, indicesService, new AliasValidator(), null, xContentRegistry()); MetadataRolloverService rolloverService = new MetadataRolloverService(testThreadPool, createIndexService, indexAliasesService,