From 9251392f6bd22e750b3a2a9c396377f6092c85a8 Mon Sep 17 00:00:00 2001 From: Przemko Robakowski Date: Wed, 4 Mar 2020 08:37:03 +0100 Subject: [PATCH] Avoid race condition in ILMHistorySotre (#53039) * Avoid race condition in ILMHistorySotre This change modifies ILMHistoryStore to always apply correct settings and mappings, even if template is deleted and not yet recreated. This ensures that ILM history index is correctly managed by ILM and also fixes flaky history tests that were prone to triggenring this race. This commit also refactors and simplifies ILM history tests. Closes #50353 and #52853 * Review comment Co-authored-by: Elastic Machine --- .../ilm/TimeSeriesLifecycleActionsIT.java | 57 +++---------------- .../xpack/ilm/history/ILMHistoryStore.java | 19 ++++++- 2 files changed, 24 insertions(+), 52 deletions(-) diff --git a/x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/ilm/TimeSeriesLifecycleActionsIT.java b/x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/ilm/TimeSeriesLifecycleActionsIT.java index db71768a9b4f0..92b6ab6ba8632 100644 --- a/x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/ilm/TimeSeriesLifecycleActionsIT.java +++ b/x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/ilm/TimeSeriesLifecycleActionsIT.java @@ -1367,10 +1367,7 @@ public void testWaitForActiveShardsStep() throws Exception { assertBusy(() -> assertThat(getStepKeyForIndex(originalIndex), equalTo(PhaseCompleteStep.finalStep("hot").getKey()))); } - @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/50353") public void testHistoryIsWrittenWithSuccess() throws Exception { - String index = "success-index"; - createNewSingletonPolicy("hot", new RolloverAction(null, null, 1L)); Request createIndexTemplate = new Request("PUT", "_template/rolling_indexes"); createIndexTemplate.setJsonEntity("{" + @@ -1384,10 +1381,7 @@ public void testHistoryIsWrittenWithSuccess() throws Exception { "}"); client().performRequest(createIndexTemplate); - createIndexWithSettings(index + "-1", - Settings.builder().put(IndexMetaData.SETTING_NUMBER_OF_SHARDS, 1) - .put(IndexMetaData.SETTING_NUMBER_OF_REPLICAS, 0), - true); + createIndexWithSettings(index + "-1", Settings.builder(), true); // Index a document index(client(), index + "-1", "1", "foo", "bar"); @@ -1405,69 +1399,34 @@ public void testHistoryIsWrittenWithSuccess() throws Exception { assertBusy(() -> assertHistoryIsPresent(policy, index + "-1", true, "wait-for-yellow-step"), 30, TimeUnit.SECONDS); assertBusy(() -> assertHistoryIsPresent(policy, index + "-1", true, "check-rollover-ready"), 30, TimeUnit.SECONDS); assertBusy(() -> assertHistoryIsPresent(policy, index + "-1", true, "attempt-rollover"), 30, TimeUnit.SECONDS); - assertBusy(() -> assertHistoryIsPresent(policy, index + "-1", true, "update-rollover-lifecycle-date"), 30, TimeUnit.SECONDS); assertBusy(() -> assertHistoryIsPresent(policy, index + "-1", true, "set-indexing-complete"), 30, TimeUnit.SECONDS); - assertBusy(() -> assertHistoryIsPresent(policy, index + "-1", true, "completed"), 30, TimeUnit.SECONDS); + assertBusy(() -> assertHistoryIsPresent(policy, index + "-1", true, "complete"), 30, TimeUnit.SECONDS); assertBusy(() -> assertHistoryIsPresent(policy, index + "-000002", true, "check-rollover-ready"), 30, TimeUnit.SECONDS); } - @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/50353") public void testHistoryIsWrittenWithFailure() throws Exception { - String index = "failure-index"; - + createIndexWithSettings(index + "-1", Settings.builder(), false); createNewSingletonPolicy("hot", new RolloverAction(null, null, 1L)); - Request createIndexTemplate = new Request("PUT", "_template/rolling_indexes"); - createIndexTemplate.setJsonEntity("{" + - "\"index_patterns\": [\""+ index + "-*\"], \n" + - " \"settings\": {\n" + - " \"number_of_shards\": 1,\n" + - " \"number_of_replicas\": 0,\n" + - " \"index.lifecycle.name\": \"" + policy+ "\"\n" + - " }\n" + - "}"); - client().performRequest(createIndexTemplate); - - createIndexWithSettings(index + "-1", - Settings.builder().put(IndexMetaData.SETTING_NUMBER_OF_SHARDS, 1) - .put(IndexMetaData.SETTING_NUMBER_OF_REPLICAS, 0), - false); + updatePolicy(index + "-1", policy); // Index a document index(client(), index + "-1", "1", "foo", "bar"); Request refreshIndex = new Request("POST", "/" + index + "-1/_refresh"); client().performRequest(refreshIndex); - assertBusy(() -> assertThat(getStepKeyForIndex(index + "-1").getName(), equalTo(ErrorStep.NAME))); + assertBusy(() -> assertThat(getStepKeyForIndex(index + "-1").getName(), equalTo(ErrorStep.NAME)), 30, TimeUnit.SECONDS); assertBusy(() -> assertHistoryIsPresent(policy, index + "-1", false, "ERROR"), 30, TimeUnit.SECONDS); } - @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/50353") public void testHistoryIsWrittenWithDeletion() throws Exception { - String index = "delete-index"; - - createNewSingletonPolicy("delete", new DeleteAction()); - Request createIndexTemplate = new Request("PUT", "_template/delete_indexes"); - createIndexTemplate.setJsonEntity("{" + - "\"index_patterns\": [\""+ index + "\"], \n" + - " \"settings\": {\n" + - " \"number_of_shards\": 1,\n" + - " \"number_of_replicas\": 0,\n" + - " \"index.lifecycle.name\": \"" + policy+ "\"\n" + - " }\n" + - "}"); - client().performRequest(createIndexTemplate); - // Index should be created and then deleted by ILM createIndexWithSettings(index, Settings.builder(), false); + createNewSingletonPolicy("delete", new DeleteAction()); + updatePolicy(index, policy); - assertBusy(() -> { - logger.info("--> checking for index deletion..."); - Request existCheck = new Request("HEAD", "/" + index); - Response resp = client().performRequest(existCheck); - assertThat(resp.getStatusLine().getStatusCode(), equalTo(404)); - }); + assertBusy(() -> assertFalse(indexExists(index))); assertBusy(() -> { assertHistoryIsPresent(policy, index, true, "delete", "delete", "wait-for-shard-history-leases"); diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/history/ILMHistoryStore.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/history/ILMHistoryStore.java index 96c54e5adfca3..213c569098342 100644 --- a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/history/ILMHistoryStore.java +++ b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/history/ILMHistoryStore.java @@ -25,6 +25,7 @@ import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.metadata.AliasOrIndex; import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.ByteSizeUnit; import org.elasticsearch.common.unit.ByteSizeValue; @@ -32,6 +33,8 @@ import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.common.xcontent.XContentHelper; +import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.threadpool.ThreadPool; import java.io.Closeable; @@ -45,6 +48,7 @@ import static org.elasticsearch.xpack.core.ClientHelper.INDEX_LIFECYCLE_ORIGIN; import static org.elasticsearch.xpack.core.ilm.LifecycleSettings.LIFECYCLE_HISTORY_INDEX_ENABLED_SETTING; import static org.elasticsearch.xpack.ilm.history.ILMHistoryTemplateRegistry.INDEX_TEMPLATE_VERSION; +import static org.elasticsearch.xpack.ilm.history.ILMHistoryTemplateRegistry.TEMPLATE_ILM_HISTORY; /** * The {@link ILMHistoryStore} handles indexing {@link ILMHistoryItem} documents into the @@ -163,6 +167,7 @@ public void putAsync(ILMHistoryItem item) { * @param listener Called after the index has been created. `onResponse` called with `true` if the index was created, * `false` if it already existed. */ + @SuppressWarnings("unchecked") static void ensureHistoryIndex(Client client, ClusterState state, ActionListener listener) { final String initialHistoryIndexName = ILM_HISTORY_INDEX_PREFIX + "000001"; final AliasOrIndex ilmHistory = state.metaData().getAliasAndIndexLookup().get(ILM_HISTORY_ALIAS); @@ -171,11 +176,19 @@ static void ensureHistoryIndex(Client client, ClusterState state, ActionListener if (ilmHistory == null && initialHistoryIndex == null) { // No alias or index exists with the expected names, so create the index with appropriate alias logger.debug("creating ILM history index [{}]", initialHistoryIndexName); + + // Template below should be already defined as real index template but it can be deleted. To avoid race condition with its + // recreation we apply settings and mappings ourselves + byte[] templateBytes = TEMPLATE_ILM_HISTORY.loadBytes(); + Map templateAsMap = XContentHelper.convertToMap(new BytesArray(templateBytes, 0, templateBytes.length), + false, XContentType.JSON).v2(); + client.admin().indices().prepareCreate(initialHistoryIndexName) + .setSettings((Map) templateAsMap.get("settings")) + .setMapping((Map) templateAsMap.get("mappings")) .setWaitForActiveShards(1) - .addAlias(new Alias(ILM_HISTORY_ALIAS) - .writeIndex(true)) - .execute(new ActionListener() { + .addAlias(new Alias(ILM_HISTORY_ALIAS).writeIndex(true)) + .execute(new ActionListener<>() { @Override public void onResponse(CreateIndexResponse response) { listener.onResponse(true);