Fix testThatNonExistingTemplatesAreAddedImmediately (elastic#51668)

This addresses another race condition that could yield this test flaky.
henningandersen · Jan 31, 2020 · d20d90a · d20d90a
1 parent c61888f
commit d20d90a
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 4 deletions.
diff --git a/...lugin/core/src/main/java/org/elasticsearch/xpack/core/template/IndexTemplateRegistry.java b/...lugin/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/...java/org/elasticsearch/xpack/core/slm/history/SnapshotLifecycleTemplateRegistryTests.java b/...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 {