Browse Source

Fix testThatNonExistingTemplatesAreAddedImmediately (#51668)

This addresses another race condition that could yield this test flaky.
Andrei Dan 5 years ago
parent
commit
d20d90aceb

+ 3 - 0
x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/template/IndexTemplateRegistry.java

@@ -140,6 +140,9 @@ public abstract class IndexTemplateRegistry implements ClusterStateListener {
                     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());
             }
         }
     }

+ 12 - 4
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.ilm.LifecycleSettings.SLM_HISTORY_IND
 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 class SnapshotLifecycleTemplateRegistryTests extends ESTestCase {
         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 {