honeymoose/OpenSearch
1
0
Fork 0
mirror of https://github.com/honeymoose/OpenSearch.git synced 2025-02-17 10:25:15 +00:00
Code Issues Packages Projects Releases Wiki Activity

Tests: Remove global shared cluster

Browse Source 
This was previously attempted in #8854. I revived that branch and did
some performance testing as was suggested in the comments there.

I fixed all the errors, mostly just the rest tests, which
needed to have http enabled on the node settings (the global cluster
previously had this always enabled). I also addressed the comments from
that issue.

My performance tests involved running the entire test suite on my
desktop which has 6 cores, 16GB of ram, and nothing else was being
run on the box at the time. I ran each set of settings 3 times and
took the average time.

| mode    | master | patch | diff |
| ------- | ------ | ----- | ---- |
| local   | 409s   | 417s  | +2%  |
| network | 368s   | 380s  | +3%  |

This increase in average time is clearly worthwhile to pay to achieve
isolation of tests. One caveat is the way I fixed the rest tests
is still to have one cluster for the entire suite, so all the rest
tests can still potentially affect each other, but this is an
issue for another day.

There were some oddities that I noticed while running these tests
that I would like to point out, as they probably deserve some
investigation (but orthogonal to this PR):
* The total test run times are highly variable (more than a minute between the min and max)
* Running in network mode is on average actually *faster* than local mode. How is this possible!?
This commit is contained in:
Robert Muir 2014-12-09 13:42:56 -05:00 committed by Ryan Ernst
parent 6125ae1fdf
commit 1e015e6e33
6 changed files with 69 additions and 101 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
docs/reference/testing/testing-framework.asciidoc
View File

@ -129,19 +129,19 @@ In order to execute any actions, you have to use a client. You can use the `Elas
[[scoping]]
==== Scoping
By default the tests are run without restarting the cluster between tests or test classes in order to be as fast as possible. Of course all indices and templates are deleted between each test. However, sometimes you need to start a new cluster for each test or for a whole test suite - for example, if you load a certain plugin, but you do not want to load it for every test.
By default the tests are run with unique cluster per test suite. Of course all indices and templates are deleted between each test. However, sometimes you need to start a new cluster for each test - for example, if you load a certain plugin, but you do not want to load it for every test.
You can use the `@ClusterScope` annotation at class level to configure this behaviour
[source,java]
-----------------------------------------
@ClusterScope(scope=SUITE, numNodes=1)
@ClusterScope(scope=TEST, numNodes=1)
public class CustomSuggesterSearchTests extends ElasticsearchIntegrationTest {
// ... tests go here
}
-----------------------------------------
The above sample configures an own cluster for this test suite, which is the class. Other values could be `GLOBAL` (the default) or `TEST` in order to spawn a new cluster for each test. The `numNodes` settings allows you to only start a certain number of nodes, which can speed up test execution, as starting a new node is a costly and time consuming operation and might not be needed for this test.
The above sample configures the test to use a new cluster for each test method. The default scope is `SUITE` (one cluster for all test methods in the test). The `numNodes` settings allows you to only start a certain number of nodes, which can speed up test execution, as starting a new node is a costly and time consuming operation and might not be needed for this test.
[[changing-node-configuration]]

10
src/test/java/org/elasticsearch/network/DirectBufferNetworkTests.java
View File

@ -23,11 +23,14 @@ import org.apache.http.impl.client.HttpClients;
import org.apache.lucene.util.TestUtil;
import org.elasticsearch.action.index.IndexRequestBuilder;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.settings.ImmutableSettings;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.transport.InetSocketTransportAddress;
import org.elasticsearch.common.unit.ByteSizeValue;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.http.HttpServerTransport;
import org.elasticsearch.node.internal.InternalNode;
import org.elasticsearch.test.ElasticsearchIntegrationTest;
import org.elasticsearch.test.rest.client.http.HttpRequestBuilder;
import org.hamcrest.Matchers;
@ -44,6 +47,13 @@ import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitC
*/
public class DirectBufferNetworkTests extends ElasticsearchIntegrationTest {
@Override
protected Settings nodeSettings(int nodeOrdinal) {
return ImmutableSettings.builder()
.put(InternalNode.HTTP_ENABLED, true)
.put(super.nodeSettings(nodeOrdinal)).build();
}
/**
* This test validates that using large data sets (large docs + large API requests) don't
* cause a large direct byte buffer to be allocated internally in the sun.nio buffer cache.

10
src/test/java/org/elasticsearch/rest/CorsRegexDefaultTests.java
View File

@ -18,6 +18,9 @@
*/
package org.elasticsearch.rest;
import org.elasticsearch.common.settings.ImmutableSettings;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.node.internal.InternalNode;
import org.elasticsearch.test.ElasticsearchIntegrationTest;
import org.elasticsearch.test.rest.client.http.HttpResponse;
import org.junit.Test;
@ -30,6 +33,13 @@ import static org.hamcrest.Matchers.*;
*/
public class CorsRegexDefaultTests extends ElasticsearchIntegrationTest {
@Override
protected Settings nodeSettings(int nodeOrdinal) {
return ImmutableSettings.builder()
.put(InternalNode.HTTP_ENABLED, true)
.put(super.nodeSettings(nodeOrdinal)).build();
}
@Test
public void testCorsSettingDefaultBehaviourDoesNotReturnAnything() throws Exception {
String corsValue = "http://localhost:9200";

121
src/test/java/org/elasticsearch/test/ElasticsearchIntegrationTest.java
View File

@ -143,21 +143,17 @@ import static org.hamcrest.Matchers.*;
/**
* {@link ElasticsearchIntegrationTest} is an abstract base class to run integration
* tests against a JVM private Elasticsearch Cluster. The test class supports 3 different
* tests against a JVM private Elasticsearch Cluster. The test class supports 2 different
* cluster scopes.
* <ul>
* <li>{@link Scope#GLOBAL} - uses a cluster shared across test suites. This cluster doesn't allow any modifications to
* the cluster settings and will fail if any persistent cluster settings are applied during tear down.</li>
* <li>{@link Scope#TEST} - uses a new cluster for each individual test method.</li>
* <li>{@link Scope#SUITE} - uses a cluster shared across all test method in the same suite</li>
* <li>{@link Scope#SUITE} - uses a cluster shared across all test methods in the same suite</li>
* </ul>
* <p/>
* The most common test scope it {@link Scope#GLOBAL} which shares a cluster per JVM. This cluster is only set-up once
* and can be used as long as the tests work on a per index basis without changing any cluster wide settings or require
* any specific node configuration. This is the best performing option since it sets up the cluster only once.
* The most common test scope is {@link Scope#SUITE} which shares a cluster per test suite.
* <p/>
* If the tests need specific node settings or change persistent and/or transient cluster settings either {@link Scope#TEST}
* or {@link Scope#SUITE} should be used. To configure a scope for the test cluster the {@link ClusterScope} annotation
* If the test methods need specific node settings or change persistent and/or transient cluster settings {@link Scope#TEST}
* should be used. To configure a scope for the test cluster the {@link ClusterScope} annotation
* should be used, here is an example:
* <pre>
*
@ -166,12 +162,11 @@ import static org.hamcrest.Matchers.*;
* }
* </pre>
* <p/>
* If no {@link ClusterScope} annotation is present on an integration test the default scope it {@link Scope#GLOBAL}
* If no {@link ClusterScope} annotation is present on an integration test the default scope is {@link Scope#SUITE}
* <p/>
* A test cluster creates a set of nodes in the background before the test starts. The number of nodes in the cluster is
* determined at random and can change across tests. The minimum number of nodes in the shared global cluster is <code>2</code>.
* For other scopes the {@link ClusterScope} allows configuring the initial number of nodes that are created before
* the tests start.
* determined at random and can change across tests. The {@link ClusterScope} allows configuring the initial number of nodes
* that are created before the tests start.
* <p/>
* <pre>
* @ClusterScope(scope=Scope.SUITE, numDataNodes=3)
@ -204,7 +199,6 @@ public abstract class ElasticsearchIntegrationTest extends ElasticsearchTestCase
public static final String SUITE_CLUSTER_NODE_PREFIX = "node_s";
public static final String TEST_CLUSTER_NODE_PREFIX = "node_t";
private static TestCluster GLOBAL_CLUSTER;
/**
* Key used to set the transport client ratio via the commandline -D{@value #TESTS_CLIENT_RATIO}
*/
@ -277,9 +271,6 @@ public abstract class ElasticsearchIntegrationTest extends ElasticsearchTestCase
try {
final Scope currentClusterScope = getCurrentClusterScope();
switch (currentClusterScope) {
case GLOBAL:
currentCluster = buildAndPutCluster(currentClusterScope, SeedUtils.parseSeed(RandomizedContext.current().getRunnerSeedAsString()));
break;
case SUITE:
assert SUITE_SEED != null : "Suite seed was not initialized";
currentCluster = buildAndPutCluster(currentClusterScope, SUITE_SEED.longValue());
@ -571,12 +562,6 @@ public abstract class ElasticsearchIntegrationTest extends ElasticsearchTestCase
TestCluster testCluster = clusters.remove(clazz); // remove this cluster first
clearClusters(); // all leftovers are gone by now... this is really just a double safety if we miss something somewhere
switch (currentClusterScope) {
case GLOBAL:
// never put the global cluster into the map otherwise it will be closed
if (GLOBAL_CLUSTER == null) {
return GLOBAL_CLUSTER = buildWithPrivateContext(currentClusterScope, seed);
}
return GLOBAL_CLUSTER;
case SUITE:
if (testCluster == null) { // only build if it's not there yet
testCluster = buildWithPrivateContext(currentClusterScope, seed);
@ -588,7 +573,6 @@ public abstract class ElasticsearchIntegrationTest extends ElasticsearchTestCase
testCluster = buildTestCluster(currentClusterScope, seed);
break;
}
assert testCluster != GLOBAL_CLUSTER : "Global cluster must be handled via the GLOBAL_CLUSTER static member";
clusters.put(clazz, testCluster);
return testCluster;
}
@ -635,12 +619,6 @@ public abstract class ElasticsearchIntegrationTest extends ElasticsearchTestCase
// we also reset everything in the case we had a failure in the suite to make sure subsequent
// tests get a new / clean cluster
clearClusters();
if (currentCluster == GLOBAL_CLUSTER) {
if (GLOBAL_CLUSTER != null) {
IOUtils.closeWhileHandlingException(GLOBAL_CLUSTER);
}
GLOBAL_CLUSTER = null;
}
currentCluster = null;
}
if (currentCluster != null) {
@ -1430,11 +1408,6 @@ public abstract class ElasticsearchIntegrationTest extends ElasticsearchTestCase
* {@link org.elasticsearch.test.ElasticsearchIntegrationTest.ClusterScope} annotations on {@link org.elasticsearch.test.ElasticsearchIntegrationTest} subclasses.
*/
public static enum Scope {
/**
* A globally shared cluster. This cluster doesn't allow modification of transient or persistent
* cluster settings.
*/
GLOBAL,
/**
* A cluster shared across all method in a single test suite
*/
@ -1447,16 +1420,16 @@ public abstract class ElasticsearchIntegrationTest extends ElasticsearchTestCase
/**
* Defines a cluster scope for a {@link org.elasticsearch.test.ElasticsearchIntegrationTest} subclass.
* By default if no {@link ClusterScope} annotation is present {@link org.elasticsearch.test.ElasticsearchIntegrationTest.Scope#GLOBAL} is used
* By default if no {@link ClusterScope} annotation is present {@link org.elasticsearch.test.ElasticsearchIntegrationTest.Scope#SUITE} is used
* together with randomly chosen settings like number of nodes etc.
*/
@Retention(RetentionPolicy.RUNTIME)
@Target({ElementType.TYPE})
public @interface ClusterScope {
/**
* Returns the scope. {@link org.elasticsearch.test.ElasticsearchIntegrationTest.Scope#GLOBAL} is default.
* Returns the scope. {@link org.elasticsearch.test.ElasticsearchIntegrationTest.Scope#SUITE} is default.
*/
Scope scope() default Scope.GLOBAL;
Scope scope() default Scope.SUITE;
/**
* Returns the number of nodes in the cluster. Default is <tt>-1</tt> which means
@ -1570,8 +1543,8 @@ public abstract class ElasticsearchIntegrationTest extends ElasticsearchTestCase
private static Scope getCurrentClusterScope(Class<?> clazz) {
ClusterScope annotation = getAnnotation(clazz);
// if we are not annotated assume global!
return annotation == null ? Scope.GLOBAL : annotation.scope();
// if we are not annotated assume suite!
return annotation == null ? Scope.SUITE : annotation.scope();
}
private int getNumDataNodes() {
@ -1639,12 +1612,6 @@ public abstract class ElasticsearchIntegrationTest extends ElasticsearchTestCase
SettingsSource settingsSource = InternalTestCluster.DEFAULT_SETTINGS_SOURCE;
final String nodePrefix;
switch (scope) {
case GLOBAL:
if (globalCompatibilityVersion().before(Version.V_1_2_0)) {
numClientNodes = 0;
}
nodePrefix = GLOBAL_CLUSTER_NODE_PREFIX;
break;
case TEST:
nodePrefix = TEST_CLUSTER_NODE_PREFIX;
break;
@ -1654,50 +1621,30 @@ public abstract class ElasticsearchIntegrationTest extends ElasticsearchTestCase
default:
throw new ElasticsearchException("Scope not supported: " + scope);
}
if (scope == Scope.GLOBAL) {
String cluster = System.getProperty(TESTS_CLUSTER);
if (Strings.hasLength(cluster)) {
String[] stringAddresses = cluster.split(",");
TransportAddress[] transportAddresses = new TransportAddress[stringAddresses.length];
int i = 0;
for (String stringAddress : stringAddresses) {
String[] split = stringAddress.split(":");
if (split.length < 2) {
throw new IllegalArgumentException("address [" + cluster + "] not valid");
}
try {
transportAddresses[i++] = new InetSocketTransportAddress(split[0], Integer.valueOf(split[1]));
} catch (NumberFormatException e) {
throw new IllegalArgumentException("port is not valid, expected number but was [" + split[1] + "]");
}
}
return new ExternalTestCluster(transportAddresses);
settingsSource = new SettingsSource() {
@Override
public Settings node(int nodeOrdinal) {
return ImmutableSettings.builder().put(InternalNode.HTTP_ENABLED, false).
put(nodeSettings(nodeOrdinal)).build();
}
@Override
public Settings transportClient() {
return transportClientSettings();
}
};
int numDataNodes = getNumDataNodes();
if (numDataNodes >= 0) {
minNumDataNodes = maxNumDataNodes = numDataNodes;
} else {
settingsSource = new SettingsSource() {
@Override
public Settings node(int nodeOrdinal) {
return ImmutableSettings.builder().put(InternalNode.HTTP_ENABLED, false).
put(nodeSettings(nodeOrdinal)).build();
}
@Override
public Settings transportClient() {
return transportClientSettings();
}
};
int numDataNodes = getNumDataNodes();
if (numDataNodes >= 0) {
minNumDataNodes = maxNumDataNodes = numDataNodes;
} else {
minNumDataNodes = getMinNumDataNodes();
maxNumDataNodes = getMaxNumDataNodes();
}
numClientNodes = getNumClientNodes();
enableRandomBenchNodes = enableRandomBenchNodes();
minNumDataNodes = getMinNumDataNodes();
maxNumDataNodes = getMaxNumDataNodes();
}
numClientNodes = getNumClientNodes();
enableRandomBenchNodes = enableRandomBenchNodes();
return new InternalTestCluster(seed, minNumDataNodes, maxNumDataNodes,
clusterName(scope.name(), Integer.toString(CHILD_JVM_ID), seed), settingsSource, numClientNodes,
enableRandomBenchNodes, enableHttpPipelining, CHILD_JVM_ID, nodePrefix);

15
src/test/java/org/elasticsearch/test/discovery/ClusterDiscoveryConfiguration.java
View File

@ -114,17 +114,10 @@ public class ClusterDiscoveryConfiguration extends SettingsSource {
}
private static int scopeId(ElasticsearchIntegrationTest.Scope scope) {
switch(scope) {
case GLOBAL:
//we reserve a special base port for global clusters, as they stick around
//the assumption is that no counter is needed as there's only one global cluster per jvm
return 0;
default:
//ports can be reused as suite or test clusters are never run concurrently
//we don't reuse the same port immediately though but leave some time to make sure ports are freed
//reserve 0 to global cluster, prevent conflicts between jvms by never going above 9
return 1 + portCounter.incrementAndGet() % 9;
}
//ports can be reused as suite or test clusters are never run concurrently
//we don't reuse the same port immediately though but leave some time to make sure ports are freed
//prevent conflicts between jvms by never going above 9
return portCounter.incrementAndGet() % 9;
}
@Override

8
src/test/java/org/elasticsearch/test/rest/ElasticsearchRestTests.java
View File

@ -30,6 +30,7 @@ import org.apache.lucene.util.TimeUnits;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.settings.ImmutableSettings;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.node.internal.InternalNode;
import org.elasticsearch.test.ElasticsearchIntegrationTest;
import org.elasticsearch.test.ElasticsearchIntegrationTest.ClusterScope;
import org.elasticsearch.test.rest.client.RestException;
@ -107,6 +108,13 @@ public class ElasticsearchRestTests extends ElasticsearchIntegrationTest {
blacklistPathMatchers = new PathMatcher[0];
}
}
@Override
protected Settings nodeSettings(int nodeOrdinal) {
return ImmutableSettings.builder()
.put(InternalNode.HTTP_ENABLED, true)
.put(super.nodeSettings(nodeOrdinal)).build();
}
@ParametersFactory
public static Iterable<Object[]> parameters() throws IOException, RestTestParseException {