Browse Source

Avoid unnecessary setup and teardown in docs tests (#51430)

The docs tests have recently been running much slower than before (see #49753).

The gist here is that with ILM/SLM we do a lot of unnecessary setup / teardown work on each
test. Compounded with the slightly slower cluster state storage mechanism, this causes the
tests to run much slower.

In particular, on RAMDisk, docs:check is taking

ES 7.4: 6:55 minutes
ES master: 16:09 minutes
ES with this commit: 6:52 minutes

on SSD, docs:check is taking

ES 7.4: ??? minutes
ES master: 32:20 minutes
ES with this commit: 11:21 minutes
Yannick Welsch 5 years ago
parent
commit
e5dd459745

+ 7 - 0
docs/reference/indices/get-index-template.asciidoc

@@ -18,6 +18,13 @@ PUT _template/template_1
 }
 --------------------------------------------------
 // TESTSETUP
+
+[source,console]
+--------------------------------------------------
+DELETE _template/template_1
+--------------------------------------------------
+// TEARDOWN
+
 ////
 
 [source,console]

+ 36 - 27
docs/reference/indices/templates.asciidoc

@@ -32,6 +32,15 @@ PUT _template/template_1
 --------------------------------------------------
 // TESTSETUP
 
+//////////////////////////
+
+[source,console]
+--------------------------------------------------
+DELETE _template/template_1
+--------------------------------------------------
+// TEARDOWN
+
+//////////////////////////
 
 [[put-index-template-api-request]]
 ==== {api-request-title}
@@ -62,7 +71,7 @@ You can use C-style /* */ block comments in index templates.
 You can include comments anywhere in the request body,
 except before the opening curly bracket.
 
-[[getting]]	
+[[getting]]
 ===== Getting templates
 
 See <<indices-get-template>>.
@@ -194,12 +203,12 @@ Note, for mappings, the merging is "deep", meaning that specific
 object/property based mappings can easily be added/overridden on higher
 order templates, with lower order templates providing the basis.
 
-NOTE: Multiple matching templates with the same order value will 
+NOTE: Multiple matching templates with the same order value will
 result in a non-deterministic merging order.
 
 
 [[versioning-templates]]
-===== Template versioning	
+===== Template versioning
 
 You can use the `version` parameter
 to add an optional version number to an index template.
@@ -210,39 +219,39 @@ The `version`	parameter is completely optional
 and not automatically generated by {es}.
 
 To unset a `version`,
-replace the template without specifying	one.	
+replace the template without specifying	one.
 
 [source,console]
---------------------------------------------------	
-PUT /_template/template_1	
-{	
-    "index_patterns" : ["*"],	
-    "order" : 0,	
-    "settings" : {	
-        "number_of_shards" : 1	
-    },	
-    "version": 123	
-}	
---------------------------------------------------	
+--------------------------------------------------
+PUT /_template/template_1
+{
+    "index_patterns" : ["*"],
+    "order" : 0,
+    "settings" : {
+        "number_of_shards" : 1
+    },
+    "version": 123
+}
+--------------------------------------------------
 
 To check the `version`,
 you can	use the <<indices-get-template, get index template>> API
 with the <<common-options-response-filtering, `filter_path`>> query parameter
-to return only the version number:	
+to return only the version number:
 
 [source,console]
---------------------------------------------------	
-GET /_template/template_1?filter_path=*.version	
---------------------------------------------------	
-// TEST[continued]	
+--------------------------------------------------
+GET /_template/template_1?filter_path=*.version
+--------------------------------------------------
+// TEST[continued]
 
 The API returns the following response:
 
 [source,console-result]
---------------------------------------------------	
-{	
-  "template_1" : {	
-    "version" : 123	
-  }	
-}	
---------------------------------------------------	
+--------------------------------------------------
+{
+  "template_1" : {
+    "version" : 123
+  }
+}
+--------------------------------------------------

+ 26 - 2
docs/src/test/java/org/elasticsearch/smoketest/DocsClientYamlTestSuiteIT.java

@@ -58,8 +58,7 @@ import static java.util.Collections.singletonMap;
 import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg;
 
 //The default 20 minutes timeout isn't always enough, please do not increase further than 30 before analyzing what makes this suite so slow
-// gh#49753, tv#49753 : increasing timeout to 40 minutes until this gets fixes properly
-@TimeoutSuite(millis = 40 * TimeUnits.MINUTE)
+@TimeoutSuite(millis = 30 * TimeUnits.MINUTE)
 public class DocsClientYamlTestSuiteIT extends ESClientYamlSuiteTestCase {
 
     public DocsClientYamlTestSuiteIT(@Name("yaml") ClientYamlTestCandidate testCandidate) {
@@ -108,6 +107,31 @@ public class DocsClientYamlTestSuiteIT extends ESClientYamlSuiteTestCase {
         }
     }
 
+    @Override
+    protected boolean preserveSLMPoliciesUponCompletion() {
+        return isSLMTest() == false;
+    }
+
+    @Override
+    protected boolean preserveILMPoliciesUponCompletion() {
+        return isILMTest() == false;
+    }
+
+    @Override
+    protected boolean preserveTemplatesUponCompletion() {
+        return true;
+    }
+
+    protected boolean isSLMTest() {
+        String testName = getTestName();
+        return testName != null && (testName.contains("/slm/") || testName.contains("\\slm\\"));
+    }
+
+    protected boolean isILMTest() {
+        String testName = getTestName();
+        return testName != null && (testName.contains("/ilm/") || testName.contains("\\ilm\\"));
+    }
+
     protected boolean isMachineLearningTest() {
         String testName = getTestName();
         return testName != null && (testName.contains("/ml/") || testName.contains("\\ml\\"));

+ 2 - 2
test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java

@@ -43,6 +43,7 @@ import org.elasticsearch.common.io.PathUtils;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.unit.TimeValue;
 import org.elasticsearch.common.util.concurrent.ThreadContext;
+import org.elasticsearch.common.util.set.Sets;
 import org.elasticsearch.common.xcontent.DeprecationHandler;
 import org.elasticsearch.common.xcontent.NamedXContentRegistry;
 import org.elasticsearch.common.xcontent.XContentBuilder;
@@ -78,7 +79,6 @@ import java.security.NoSuchAlgorithmException;
 import java.security.cert.CertificateException;
 import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -459,7 +459,7 @@ public abstract class ESRestTestCase extends ESTestCase {
      * A set of ILM policies that should be preserved between runs.
      */
     protected Set<String> preserveILMPolicyIds() {
-        return Collections.singleton("ilm-history-ilm-policy");
+        return Sets.newHashSet("ilm-history-ilm-policy", "slm-history-ilm-policy", "watch-history-ilm-policy");
     }
 
     /**

+ 3 - 4
x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java

@@ -43,7 +43,6 @@ import org.elasticsearch.cluster.ClusterState;
 import org.elasticsearch.cluster.ClusterStateUpdateTask;
 import org.elasticsearch.cluster.ack.AckedRequest;
 import org.elasticsearch.cluster.ack.ClusterStateUpdateResponse;
-import org.elasticsearch.cluster.metadata.MetaData;
 import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.common.Nullable;
 import org.elasticsearch.common.Priority;
@@ -2021,7 +2020,7 @@ public final class TokenService {
 
             if (state.nodes().isLocalNodeElectedMaster()) {
                 if (XPackPlugin.isReadyForXPackCustomMetadata(state)) {
-                    installTokenMetadata(state.metaData());
+                    installTokenMetadata(state);
                 } else {
                     logger.debug("cannot add token metadata to cluster as the following nodes might not understand the metadata: {}",
                         () -> XPackPlugin.nodesNotReadyForXPackCustomMetadata(state));
@@ -2044,8 +2043,8 @@ public final class TokenService {
     // to prevent too many cluster state update tasks to be queued for doing the same update
     private final AtomicBoolean installTokenMetadataInProgress = new AtomicBoolean(false);
 
-    private void installTokenMetadata(MetaData metaData) {
-        if (metaData.custom(TokenMetaData.TYPE) == null) {
+    private void installTokenMetadata(ClusterState state) {
+        if (state.custom(TokenMetaData.TYPE) == null) {
             if (installTokenMetadataInProgress.compareAndSet(false, true)) {
                 clusterService.submitStateUpdateTask("install-token-metadata", new ClusterStateUpdateTask(Priority.URGENT) {
                     @Override