From ad547eb7fb304ffa75b9793c34899da38a250cc3 Mon Sep 17 00:00:00 2001
From: Dawid Weiss <dawid.weiss@carrotsearch.com>
Date: Mon, 24 Feb 2014 13:46:35 +0100
Subject: [PATCH] Remove nanoTime in global cluster randomization in favor of
 deriving the seed from the main master seed. Removed shared cluster's seed
 entirely.

The problem here is that if you don't give cluster's seed then test times
fluctuate oddly, even for a fixed -Dtests.seed=... This shouldn't be the
case -- ideally, the test ran with the same master seed should reproduce
pretty much with the same execution time (and internal logic, obviously).

From the code point of view "global" variables are indeed a problem
because JUnit has no notion of before-suite hooks. And RandomizedRunner
doesn't support context accesses in static class initializers (this is
intentional because there is no way to determine when such initializers
will be executed). A workaround is to move such static global variables to
lazily-initialized methods and invoke them (once) in @BeforeClass hooks.
---
 .../test/ElasticsearchIntegrationTest.java    | 18 ++++++++++---
 .../org/elasticsearch/test/TestCluster.java   | 25 ++++---------------
 .../junit/listeners/ReproduceInfoPrinter.java | 18 +------------
 .../rest/junit/RestReproduceInfoPrinter.java  |  7 +-----
 .../test/rest/junit/RestTestSuiteRunner.java  |  7 +++---
 5 files changed, 24 insertions(+), 51 deletions(-)

diff --git a/src/test/java/org/elasticsearch/test/ElasticsearchIntegrationTest.java b/src/test/java/org/elasticsearch/test/ElasticsearchIntegrationTest.java
index 004f3edea1d..84d9b0e3fed 100644
--- a/src/test/java/org/elasticsearch/test/ElasticsearchIntegrationTest.java
+++ b/src/test/java/org/elasticsearch/test/ElasticsearchIntegrationTest.java
@@ -19,6 +19,8 @@
 package org.elasticsearch.test;
 
 import com.carrotsearch.hppc.ObjectArrayList;
+import com.carrotsearch.randomizedtesting.RandomizedContext;
+import com.carrotsearch.randomizedtesting.SeedUtils;
 import com.google.common.base.Joiner;
 import org.apache.lucene.util.AbstractRandomizedTest;
 import org.elasticsearch.ElasticsearchIllegalArgumentException;
@@ -64,6 +66,7 @@ import org.elasticsearch.search.SearchService;
 import org.elasticsearch.test.client.RandomizingClient;
 import org.junit.After;
 import org.junit.Before;
+import org.junit.BeforeClass;
 import org.junit.Ignore;
 
 import java.io.IOException;
@@ -76,7 +79,6 @@ import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.ExecutionException;
 
-import static org.elasticsearch.test.TestCluster.SHARED_CLUSTER_SEED;
 import static org.elasticsearch.test.TestCluster.clusterName;
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoFailures;
@@ -133,7 +135,6 @@ import static org.hamcrest.Matchers.equalTo;
  * This class supports the following system properties (passed with -Dkey=value to the application)
  * <ul>
  * <li>-D{@value #TESTS_CLIENT_RATIO} - a double value in the interval [0..1] which defines the ration between node and transport clients used</li>
- * <li>-D{@value TestCluster#TESTS_CLUSTER_SEED} - a random seed used to initialize the clusters random context.
  * <li>-D{@value TestCluster#TESTS_ENABLE_MOCK_MODULES} - a boolean value to enable or disable mock modules. This is
  * useful to test the system without asserting modules that to make sure they don't hide any bugs in production.</li>
  * <li>-D{@value #INDEX_SEED_SETTING} - a random seed used to initialize the index random context.
@@ -143,8 +144,7 @@ import static org.hamcrest.Matchers.equalTo;
 @Ignore
 @AbstractRandomizedTest.IntegrationTests
 public abstract class ElasticsearchIntegrationTest extends ElasticsearchTestCase {
-
-    private static final TestCluster GLOBAL_CLUSTER = new TestCluster(SHARED_CLUSTER_SEED, clusterName("shared", ElasticsearchTestCase.CHILD_VM_ID, SHARED_CLUSTER_SEED));
+    private static TestCluster GLOBAL_CLUSTER;
 
     /**
      * Key used to set the transport client ratio via the commandline -D{@value #TESTS_CLIENT_RATIO}
@@ -169,6 +169,16 @@ public abstract class ElasticsearchIntegrationTest extends ElasticsearchTestCase
 
     private static final Map<Class<?>, TestCluster> clusters = new IdentityHashMap<Class<?>, TestCluster>();
 
+    @BeforeClass
+    public final static void beforeClass() throws Exception {
+        // Initialize lazily. No need for volatiles/ CASs since each JVM runs at most one test
+        // suite at any given moment.
+        if (GLOBAL_CLUSTER == null) {
+            long masterSeed = SeedUtils.parseSeed(RandomizedContext.current().getRunnerSeedAsString());
+            GLOBAL_CLUSTER = new TestCluster(masterSeed, clusterName("shared", ElasticsearchTestCase.CHILD_VM_ID, masterSeed));
+        }
+    }
+    
     @Before
     public final void before() throws IOException {
         assert Thread.getDefaultUncaughtExceptionHandler() instanceof ElasticsearchUncaughtExceptionHandler;
diff --git a/src/test/java/org/elasticsearch/test/TestCluster.java b/src/test/java/org/elasticsearch/test/TestCluster.java
index d714454522e..fe729d43823 100644
--- a/src/test/java/org/elasticsearch/test/TestCluster.java
+++ b/src/test/java/org/elasticsearch/test/TestCluster.java
@@ -37,7 +37,6 @@ import org.elasticsearch.cluster.ClusterState;
 import org.elasticsearch.cluster.node.DiscoveryNode;
 import org.elasticsearch.cluster.node.DiscoveryNodes;
 import org.elasticsearch.cluster.routing.ShardRouting;
-import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.io.FileSystemUtils;
 import org.elasticsearch.common.logging.ESLogger;
 import org.elasticsearch.common.logging.Loggers;
@@ -91,17 +90,11 @@ public final class TestCluster implements Iterable<Client> {
     private final ESLogger logger = Loggers.getLogger(getClass());
 
     /**
-     * The random seed for the shared  test cluster used in the current JVM.
-     */
-    public static final long SHARED_CLUSTER_SEED = clusterSeed();
-
-    /**
-     * Key used to set the shared cluster random seed via the commandline -D{@value #TESTS_CLUSTER_SEED}
-     */
-    public static final String TESTS_CLUSTER_SEED = "tests.cluster_seed";
-
-    /**
-     * Key used to set the shared cluster random seed via the commandline -D{@value #TESTS_CLUSTER_SEED}
+     * A boolean value to enable or disable mock modules. This is useful to test the
+     * system without asserting modules that to make sure they don't hide any bugs in
+     * production.
+     * 
+     * @see ElasticsearchIntegrationTest
      */
     public static final String TESTS_ENABLE_MOCK_MODULES = "tests.enable_mock_modules";
 
@@ -118,14 +111,6 @@ public final class TestCluster implements Iterable<Client> {
 
     static final int DEFAULT_MAX_NUM_NODES = 6;
 
-    private static long clusterSeed() {
-        String property = System.getProperty(TESTS_CLUSTER_SEED);
-        if (!Strings.hasLength(property)) {
-            return System.nanoTime();
-        }
-        return SeedUtils.parseSeed(property);
-    }
-
     /* sorted map to make traverse order reproducible */
     private final TreeMap<String, NodeAndClient> nodes = newTreeMap();
 
diff --git a/src/test/java/org/elasticsearch/test/junit/listeners/ReproduceInfoPrinter.java b/src/test/java/org/elasticsearch/test/junit/listeners/ReproduceInfoPrinter.java
index e45b90cdffc..9f96310f83d 100644
--- a/src/test/java/org/elasticsearch/test/junit/listeners/ReproduceInfoPrinter.java
+++ b/src/test/java/org/elasticsearch/test/junit/listeners/ReproduceInfoPrinter.java
@@ -20,12 +20,10 @@ package org.elasticsearch.test.junit.listeners;
 
 import com.carrotsearch.randomizedtesting.RandomizedContext;
 import com.carrotsearch.randomizedtesting.ReproduceErrorMessageBuilder;
-import com.carrotsearch.randomizedtesting.SeedUtils;
 import com.carrotsearch.randomizedtesting.TraceFormatting;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.logging.ESLogger;
 import org.elasticsearch.common.logging.Loggers;
-import org.elasticsearch.test.ElasticsearchIntegrationTest;
 import org.elasticsearch.test.ElasticsearchTestCase;
 import org.elasticsearch.test.TestCluster;
 import org.junit.internal.AssumptionViolatedException;
@@ -34,8 +32,6 @@ import org.junit.runner.notification.Failure;
 import org.junit.runner.notification.RunListener;
 
 import static com.carrotsearch.randomizedtesting.SysGlobals.SYSPROP_ITERATIONS;
-import static org.elasticsearch.test.TestCluster.SHARED_CLUSTER_SEED;
-import static org.elasticsearch.test.TestCluster.TESTS_CLUSTER_SEED;
 
 /**
  * A {@link RunListener} that emits to {@link System#err} a string with command
@@ -66,11 +62,7 @@ public class ReproduceInfoPrinter extends RunListener {
         final StringBuilder b = new StringBuilder();
         b.append("FAILURE  : ").append(d.getDisplayName()).append("\n");
         b.append("REPRODUCE WITH  : mvn test");
-        ReproduceErrorMessageBuilder builder = reproduceErrorMessageBuilder(b).appendAllOpts(failure.getDescription());
-
-        if (mustAppendClusterSeed(failure)) {
-            appendClusterSeed(builder);
-        }
+        reproduceErrorMessageBuilder(b).appendAllOpts(failure.getDescription());
 
         b.append("\n");
         b.append("Throwable:\n");
@@ -81,14 +73,6 @@ public class ReproduceInfoPrinter extends RunListener {
         logger.error(b.toString());
     }
 
-    protected boolean mustAppendClusterSeed(Failure failure) {
-        return ElasticsearchIntegrationTest.class.isAssignableFrom(failure.getDescription().getTestClass());
-    }
-
-    protected void appendClusterSeed(ReproduceErrorMessageBuilder builder) {
-        builder.appendOpt(TESTS_CLUSTER_SEED, SeedUtils.formatSeed(SHARED_CLUSTER_SEED));
-    }
-
     protected ReproduceErrorMessageBuilder reproduceErrorMessageBuilder(StringBuilder b) {
         return new MavenMessageBuilder(b);
     }
diff --git a/src/test/java/org/elasticsearch/test/rest/junit/RestReproduceInfoPrinter.java b/src/test/java/org/elasticsearch/test/rest/junit/RestReproduceInfoPrinter.java
index 67a9126ea95..735fbf66d05 100644
--- a/src/test/java/org/elasticsearch/test/rest/junit/RestReproduceInfoPrinter.java
+++ b/src/test/java/org/elasticsearch/test/rest/junit/RestReproduceInfoPrinter.java
@@ -25,8 +25,8 @@ import org.elasticsearch.common.logging.ESLogger;
 import org.elasticsearch.common.logging.Loggers;
 import org.elasticsearch.test.junit.listeners.ReproduceInfoPrinter;
 import org.elasticsearch.test.rest.ElasticsearchRestTests;
+import org.elasticsearch.test.rest.junit.RestTestSuiteRunner.RunMode;
 import org.junit.runner.Description;
-import org.junit.runner.notification.Failure;
 
 import java.util.Arrays;
 
@@ -42,11 +42,6 @@ class RestReproduceInfoPrinter extends ReproduceInfoPrinter {
 
     protected static final ESLogger logger = Loggers.getLogger(RestReproduceInfoPrinter.class);
 
-    @Override
-    protected boolean mustAppendClusterSeed(Failure failure) {
-        return isTestCluster();
-    }
-
     private static boolean isTestCluster() {
         return runMode() == RunMode.TEST_CLUSTER;
     }
diff --git a/src/test/java/org/elasticsearch/test/rest/junit/RestTestSuiteRunner.java b/src/test/java/org/elasticsearch/test/rest/junit/RestTestSuiteRunner.java
index ffd0e157b6a..96cf24aa8bf 100644
--- a/src/test/java/org/elasticsearch/test/rest/junit/RestTestSuiteRunner.java
+++ b/src/test/java/org/elasticsearch/test/rest/junit/RestTestSuiteRunner.java
@@ -59,7 +59,6 @@ import java.util.regex.Pattern;
 import static com.carrotsearch.randomizedtesting.SeedUtils.parseSeedChain;
 import static com.carrotsearch.randomizedtesting.StandaloneRandomizedContext.*;
 import static com.carrotsearch.randomizedtesting.SysGlobals.*;
-import static org.elasticsearch.test.TestCluster.SHARED_CLUSTER_SEED;
 import static org.elasticsearch.test.TestCluster.clusterName;
 import static org.elasticsearch.test.rest.junit.DescriptionHelper.*;
 import static org.hamcrest.Matchers.equalTo;
@@ -184,8 +183,8 @@ public class RestTestSuiteRunner extends ParentRunner<RestTestCandidate> {
 
         List<InetSocketAddress> addresses = Lists.newArrayList();
         if (runMode == RunMode.TEST_CLUSTER) {
-            this.testCluster = new TestCluster(SHARED_CLUSTER_SEED, 1, 3,
-                    clusterName("REST-tests", ElasticsearchTestCase.CHILD_VM_ID, SHARED_CLUSTER_SEED));
+            this.testCluster = new TestCluster(initialSeed, 1, 3,
+                    clusterName("REST-tests", ElasticsearchTestCase.CHILD_VM_ID, initialSeed));
             this.testCluster.beforeTest(runnerRandomness.getRandom(), 0.0f);
             for (HttpServerTransport httpServerTransport : testCluster.getInstances(HttpServerTransport.class)) {
                 addresses.add(((InetSocketTransportAddress) httpServerTransport.boundAddress().publishAddress()).address());
@@ -353,7 +352,7 @@ public class RestTestSuiteRunner extends ParentRunner<RestTestCandidate> {
             return "elasticsearch REST Tests - not run";
         }
         if (runMode == RunMode.TEST_CLUSTER) {
-            return String.format(Locale.ROOT, "elasticsearch REST Tests - test cluster %s", SeedUtils.formatSeed(SHARED_CLUSTER_SEED));
+            return String.format(Locale.ROOT, "elasticsearch REST Tests - test cluster");
         }
         if (runMode == RunMode.EXTERNAL_CLUSTER) {
             return String.format(Locale.ROOT, "elasticsearch REST Tests - external cluster %s", System.getProperty(REST_TESTS_MODE));