Skip to content

Commit

Permalink
Avoid unnecessary setup and teardown in docs tests (#51430)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
ywelsch committed Jan 28, 2020
1 parent 4f56b9f commit ffcc490
Show file tree
Hide file tree
Showing 5 changed files with 74 additions and 35 deletions.
7 changes: 7 additions & 0 deletions docs/reference/indices/get-index-template.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,13 @@ PUT _template/template_1
}
--------------------------------------------------
// TESTSETUP
[source,console]
--------------------------------------------------
DELETE _template/template_1
--------------------------------------------------
// TEARDOWN
////

[source,console]
Expand Down
63 changes: 36 additions & 27 deletions docs/reference/indices/templates.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,15 @@ PUT _template/template_1
--------------------------------------------------
// TESTSETUP

//////////////////////////
[source,console]
--------------------------------------------------
DELETE _template/template_1
--------------------------------------------------
// TEARDOWN
//////////////////////////

[[put-index-template-api-request]]
==== {api-request-title}
Expand Down Expand Up @@ -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>>.
Expand Down Expand Up @@ -196,12 +205,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.
Expand All @@ -212,39 +221,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
}
}
--------------------------------------------------
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,7 @@
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) {
Expand Down Expand Up @@ -108,6 +107,31 @@ public void cleanup() throws Exception {
}
}

@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\\"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
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;
Expand Down Expand Up @@ -79,7 +80,6 @@
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;
Expand Down Expand Up @@ -484,7 +484,7 @@ protected boolean preserveILMPoliciesUponCompletion() {
* 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");
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@
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;
Expand Down Expand Up @@ -2022,7 +2021,7 @@ private void initialize(ClusterService clusterService) {

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));
Expand All @@ -2045,8 +2044,8 @@ private void initialize(ClusterService clusterService) {
// 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
Expand Down

0 comments on commit ffcc490

Please sign in to comment.