From d20d90aceb2b687239654d6f013f61f7f4cc1512 Mon Sep 17 00:00:00 2001 From: Andrei Dan Date: Fri, 31 Jan 2020 09:54:29 +0000 Subject: [PATCH] Fix testThatNonExistingTemplatesAreAddedImmediately (#51668) This addresses another race condition that could yield this test flaky. --- .../core/template/IndexTemplateRegistry.java | 3 +++ .../SnapshotLifecycleTemplateRegistryTests.java | 16 ++++++++++++---- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/template/IndexTemplateRegistry.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/template/IndexTemplateRegistry.java index d0f6e983f3033..015d7cd612058 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/template/IndexTemplateRegistry.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/template/IndexTemplateRegistry.java @@ -140,6 +140,9 @@ private void addTemplatesIfMissing(ClusterState state) { creationCheck.set(false); logger.trace("not adding index template [{}] for [{}], because it already exists", templateName, getOrigin()); } + } else { + logger.trace("skipping the creation of index template [{}] for [{}], because its creation is in progress", + templateName, getOrigin()); } } } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/slm/history/SnapshotLifecycleTemplateRegistryTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/slm/history/SnapshotLifecycleTemplateRegistryTests.java index 26c71d1aa468c..bdf87ab0035a0 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/slm/history/SnapshotLifecycleTemplateRegistryTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/slm/history/SnapshotLifecycleTemplateRegistryTests.java @@ -65,6 +65,7 @@ import static org.elasticsearch.xpack.core.slm.history.SnapshotLifecycleTemplateRegistry.SLM_POLICY_NAME; import static org.elasticsearch.xpack.core.slm.history.SnapshotLifecycleTemplateRegistry.SLM_TEMPLATE_NAME; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.instanceOf; import static org.mockito.Mockito.mock; @@ -136,10 +137,17 @@ public void testThatNonExistingTemplatesAreAddedImmediately() throws Exception { assertBusy(() -> assertThat(calledTimes.get(), equalTo(registry.getTemplateConfigs().size()))); calledTimes.set(0); - // now delete one template from the cluster state and lets retry - ClusterChangedEvent newEvent = createClusterChangedEvent(Collections.emptyList(), nodes); - registry.clusterChanged(newEvent); - assertBusy(() -> assertThat(calledTimes.get(), equalTo(1))); + + // attempting to register the event multiple times as a race condition can yield this test flaky, namely: + // when calling registry.clusterChanged(newEvent) the templateCreationsInProgress state that the IndexTemplateRegistry maintains + // might've not yet been updated to reflect that the first template registration was complete, so a second template registration + // will not be issued anymore, leaving calledTimes to 0 + assertBusy(() -> { + // now delete one template from the cluster state and lets retry + ClusterChangedEvent newEvent = createClusterChangedEvent(Collections.emptyList(), nodes); + registry.clusterChanged(newEvent); + assertThat(calledTimes.get(), greaterThan(1)); + }); } public void testThatNonExistingPoliciesAreAddedImmediately() throws Exception {