Skip to content

Commit

Permalink
[TEST] make ESSingleNodeTestCase tests repeatable (#22283)
Browse files Browse the repository at this point in the history 
If we conditionally do random things, e.g. initialize a node only after the first test, we have to make sure that we unconditionally create a new seed calling random.nextLong(), then initialize the node under a private randomness context. This makes sure that any random usage through Randomness.get() will retrieve the proper random instance through RandomizedContext.current().getRandom(). When running under private randomness, the context will return the Random instance that was created with the provided seed (forked from the main random instance) rather than the main Random that's exposed to tests as well. Otherwise tests become non repeatable because that initialization part happens only before the first executed test.
  • Loading branch information
@javanna
javanna authored Dec 21, 2016
1 parent 3451252 commit ae01a51
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 53 deletions.
4 changes: 0 additions & 4 deletions buildSrc/src/main/resources/checkstyle_suppressions.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -993,13 +993,9 @@
<suppress files="test[/\\]framework[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]util[/\\]MockBigArrays.java" checks="LineLength" />
<suppress files="test[/\\]framework[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]search[/\\]aggregations[/\\]bucket[/\\]script[/\\]NativeSignificanceScoreScriptWithParams.java" checks="LineLength" />
<suppress files="test[/\\]framework[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]test[/\\]BackgroundIndexer.java" checks="LineLength" />
<suppress files="test[/\\]framework[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]test[/\\]CompositeTestCluster.java" checks="LineLength" />
<suppress files="test[/\\]framework[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]test[/\\]CorruptionUtils.java" checks="LineLength" />
<suppress files="test[/\\]framework[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]test[/\\]ESBackcompatTestCase.java" checks="LineLength" />
<suppress files="test[/\\]framework[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]test[/\\]ESIntegTestCase.java" checks="LineLength" />
<suppress files="test[/\\]framework[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]test[/\\]ESSingleNodeTestCase.java" checks="LineLength" />
<suppress files="test[/\\]framework[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]test[/\\]ESTestCase.java" checks="LineLength" />
<suppress files="test[/\\]framework[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]test[/\\]ExternalNode.java" checks="LineLength" />
<suppress files="test[/\\]framework[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]test[/\\]IndexSettingsModule.java" checks="LineLength" />
<suppress files="test[/\\]framework[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]test[/\\]InternalTestCluster.java" checks="LineLength" />
<suppress files="test[/\\]framework[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]test[/\\]MockIndexEventListener.java" checks="LineLength" />
Expand Down
85 changes: 37 additions & 48 deletions test/framework/src/main/java/org/elasticsearch/test/ESSingleNodeTestCase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,7 @@
*/
package org.elasticsearch.test;

import java.io.IOException;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;

import com.carrotsearch.randomizedtesting.RandomizedContext;
import org.apache.lucene.util.IOUtils;
import org.elasticsearch.action.admin.cluster.health.ClusterHealthResponse;
import org.elasticsearch.action.admin.indices.create.CreateIndexRequestBuilder;
Expand All @@ -44,6 +38,7 @@
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.env.Environment;
import org.elasticsearch.env.NodeEnvironment;
import org.elasticsearch.index.Index;
import org.elasticsearch.index.IndexService;
import org.elasticsearch.indices.IndicesService;
Expand All @@ -56,11 +51,16 @@
import org.elasticsearch.test.discovery.TestZenDiscovery;
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.transport.MockTcpTransportPlugin;
import org.junit.After;
import org.junit.AfterClass;
import org.junit.Before;
import org.junit.BeforeClass;

import java.io.IOException;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;

import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.lessThanOrEqualTo;
Expand All @@ -73,62 +73,56 @@ public abstract class ESSingleNodeTestCase extends ESTestCase {

private static Node NODE = null;

private void reset() throws IOException {
assert NODE != null;
stopNode();
startNode();
}

protected void startNode() {
protected void startNode(long seed) throws Exception {
assert NODE == null;
NODE = newNode();
// we must wait for the node to actually be up and running. otherwise the node might have started, elected itself master but might not yet have removed the
NODE = RandomizedContext.current().runWithPrivateRandomness(seed, this::newNode);
// we must wait for the node to actually be up and running. otherwise the node might have started,
// elected itself master but might not yet have removed the
// SERVICE_UNAVAILABLE/1/state not recovered / initialized block
ClusterHealthResponse clusterHealthResponse = client().admin().cluster().prepareHealth().setWaitForGreenStatus().get();
assertFalse(clusterHealthResponse.isTimedOut());
client().admin().indices()
.preparePutTemplate("random_index_template")
.preparePutTemplate("one_shard_index_template")
.setPatterns(Collections.singletonList("*"))
.setOrder(0)
.setSettings(Settings.builder().put(IndexMetaData.SETTING_NUMBER_OF_SHARDS, 1)
.put(IndexMetaData.SETTING_NUMBER_OF_REPLICAS, 0)).get();
}

protected static void stopNode() throws IOException {
private static void stopNode() throws IOException {
Node node = NODE;
NODE = null;
IOUtils.close(node);
}

private void cleanup(boolean resetNode) throws IOException {
assertAcked(client().admin().indices().prepareDelete("*").get());
if (resetNode) {
reset();
}
MetaData metaData = client().admin().cluster().prepareState().get().getState().getMetaData();
assertThat("test leaves persistent cluster metadata behind: " + metaData.persistentSettings().getAsMap(),
metaData.persistentSettings().getAsMap().size(), equalTo(0));
assertThat("test leaves transient cluster metadata behind: " + metaData.transientSettings().getAsMap(),
metaData.transientSettings().getAsMap().size(), equalTo(0));
}

@Before
@Override
public void setUp() throws Exception {
super.setUp();
//the seed has to be created regardless of whether it will be used or not, for repeatability
long seed = random().nextLong();
// Create the node lazily, on the first test. This is ok because we do not randomize any settings,
// only the cluster name. This allows us to have overridden properties for plugins and the version to use.
if (NODE == null) {
startNode();
startNode(seed);
}
}

@After
@Override
public void tearDown() throws Exception {
logger.info("[{}#{}]: cleaning up after test", getTestClass().getSimpleName(), getTestName());
super.tearDown();
cleanup(resetNodeAfterTest());
assertAcked(client().admin().indices().prepareDelete("*").get());
MetaData metaData = client().admin().cluster().prepareState().get().getState().getMetaData();
assertThat("test leaves persistent cluster metadata behind: " + metaData.persistentSettings().getAsMap(),
metaData.persistentSettings().getAsMap().size(), equalTo(0));
assertThat("test leaves transient cluster metadata behind: " + metaData.transientSettings().getAsMap(),
metaData.transientSettings().getAsMap().size(), equalTo(0));
if (resetNodeAfterTest()) {
assert NODE != null;
stopNode();
//the seed can be created within this if as it will either be executed before every test method or will never be.
startNode(random().nextLong());
}
}

@BeforeClass
Expand All @@ -150,7 +144,6 @@ protected boolean resetNodeAfterTest() {
return false;
}


/** The plugin classes that should be added to the node. */
protected Collection<Class<? extends Plugin>> getPlugins() {
return Collections.emptyList();
Expand All @@ -171,20 +164,21 @@ protected Settings nodeSettings() {
private Node newNode() {
final Path tempDir = createTempDir();
Settings settings = Settings.builder()
.put(ClusterName.CLUSTER_NAME_SETTING.getKey(), InternalTestCluster.clusterName("single-node-cluster", randomLong()))
.put(ClusterName.CLUSTER_NAME_SETTING.getKey(), InternalTestCluster.clusterName("single-node-cluster", random().nextLong()))
.put(Environment.PATH_HOME_SETTING.getKey(), tempDir)
.put(Environment.PATH_REPO_SETTING.getKey(), tempDir.resolve("repo"))
// TODO: use a consistent data path for custom paths
// This needs to tie into the ESIntegTestCase#indexSettings() method
.put(Environment.PATH_SHARED_DATA_SETTING.getKey(), createTempDir().getParent())
.put("node.name", nodeName())
.put("node.name", "node_s_0")
.put("script.inline", "true")
.put("script.stored", "true")
.put(ScriptService.SCRIPT_MAX_COMPILATIONS_PER_MINUTE.getKey(), 1000)
.put(EsExecutors.PROCESSORS_SETTING.getKey(), 1) // limit the number of threads created
.put(NetworkModule.HTTP_ENABLED.getKey(), false)
.put("transport.type", MockTcpTransportPlugin.MOCK_TCP_TRANSPORT_NAME)
.put(Node.NODE_DATA_SETTING.getKey(), true)
.put(NodeEnvironment.NODE_ID_SEED_SETTING.getKey(), random().nextLong())
.put(nodeSettings()) // allow test cases to provide their own settings or override these
.build();
Collection<Class<? extends Plugin>> plugins = getPlugins();
Expand Down Expand Up @@ -212,13 +206,6 @@ public Client client() {
return NODE.client();
}

/**
* Returns the single test nodes name.
*/
public String nodeName() {
return "node_s_0";
}

/**
* Return a reference to the singleton node.
*/
Expand Down Expand Up @@ -274,7 +261,8 @@ protected IndexService createIndex(String index, CreateIndexRequestBuilder creat
// Wait for the index to be allocated so that cluster state updates don't override
// changes that would have been done locally
ClusterHealthResponse health = client().admin().cluster()
.health(Requests.clusterHealthRequest(index).waitForYellowStatus().waitForEvents(Priority.LANGUID).waitForNoRelocatingShards(true)).actionGet();
.health(Requests.clusterHealthRequest(index).waitForYellowStatus().waitForEvents(Priority.LANGUID)
.waitForNoRelocatingShards(true)).actionGet();
assertThat(health.getStatus(), lessThanOrEqualTo(ClusterHealthStatus.YELLOW));
assertThat("Cluster must be a single node cluster", health.getNumberOfDataNodes(), equalTo(1));
IndicesService instanceFromNode = getInstanceFromNode(IndicesService.class);
Expand Down Expand Up @@ -316,7 +304,8 @@ public ClusterHealthStatus ensureGreen(String... indices) {
*/
public ClusterHealthStatus ensureGreen(TimeValue timeout, String... indices) {
ClusterHealthResponse actionGet = client().admin().cluster()
.health(Requests.clusterHealthRequest(indices).timeout(timeout).waitForGreenStatus().waitForEvents(Priority.LANGUID).waitForNoRelocatingShards(true)).actionGet();
.health(Requests.clusterHealthRequest(indices).timeout(timeout).waitForGreenStatus().waitForEvents(Priority.LANGUID)
.waitForNoRelocatingShards(true)).actionGet();
if (actionGet.isTimedOut()) {
logger.info("ensureGreen timed out, cluster state:\n{}\n{}", client().admin().cluster().prepareState().get().getState(),
client().admin().cluster().preparePendingClusterTasks().get());
Expand Down
3 changes: 2 additions & 1 deletion test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -795,7 +795,8 @@ public static <T> List<T> randomSubsetOf(Collection<T> collection) {
*/
public static <T> List<T> randomSubsetOf(int size, Collection<T> collection) {
if (size > collection.size()) {
throw new IllegalArgumentException("Can\'t pick " + size + " random objects from a collection of " + collection.size() + " objects");
throw new IllegalArgumentException("Can\'t pick " + size + " random objects from a collection of " +
collection.size() + " objects");
}
List<T> tempList = new ArrayList<>(collection);
Collections.shuffle(tempList, random());
Expand Down

0 comments on commit ae01a51

Please sign in to comment.