[TEST] randomly added node.bench=true to client node in test cluster …

…and re-enabled REST benchmark tests based on number of bench nodes available

In our REST tests we already have support for features and skip sections that allow to skip tests if a feature is not supported.
We can then add a skip section based on the benchmark feature to the benchmark tests and execute them only when they are supported, knowing that they need at least a node with node.bench settings within the cluster. We can check that this requirement is met by calling the nodes info api.

This way we can dynamically decide whether to execute those tests or not and we don't need to have a node.bench around all the time. In fact, given that the REST tests use the GLOBAL cluster, we want to be able to randomize settings as much as possible and run tests against default settings as well. Also, this mechanism can be easily supported by the external cluster implementation that is used during the release process.

Introduced ability to disable benchmark nodes which is needed by BenchmarkNegativeTest.

dev-tools/build_release.py

@@ -399,7 +399,7 @@ def smoke_test_release(release, files, expected_hash, plugins):
     else:
       background = '-d'
     print('  Starting elasticsearch deamon from [%s]' % os.path.join(tmp_dir, 'elasticsearch-%s' % release))
-    run('%s; %s -Des.node.name=smoke_tester -Des.cluster.name=prepare_release -Des.discovery.zen.ping.multicast.enabled=false %s'
+    run('%s; %s -Des.node.name=smoke_tester -Des.cluster.name=prepare_release -Des.discovery.zen.ping.multicast.enabled=false -Des.node.bench=true %s'
          % (java_exe(), es_run_path, background))
     conn = HTTPConnection('127.0.0.1', 9200, 20);
     wait_for_node_startup()

rest-api-spec/test/abort_benchmark/10_basic.yaml

@@ -2,8 +2,7 @@
 "Test benchmark abort":
 
   - skip:
-        version:     "0 - 9999"
-        reason:      Disabled until nodes run with node.bench set to true
+        features:    "benchmark"
 
   - do:
         abort_benchmark:

rest-api-spec/test/benchmark/10_basic.yaml

@@ -2,8 +2,7 @@
 "Test benchmark submit":
 
   - skip:
-        version:     "0 - 9999"
-        reason:      Disabled until nodes run with node.bench set to true
+        features:    "benchmark"
 
   - do:
       indices.create:
@@ -30,5 +29,5 @@
                             "query":
                                 "match": { "_all": "*" }
 
-  - match: { status: complete }
+  - match: { status: COMPLETE }
 

rest-api-spec/test/list_benchmarks/10_basic.yaml

@@ -2,8 +2,7 @@
 "Test benchmark list":
 
   - skip:
-        version:     "0 - 9999"
-        reason:      Disabled until nodes run with node.bench set to true
+        features:    "benchmark"
 
   - do:
         list_benchmarks: {}

src/test/java/org/elasticsearch/action/bench/BenchmarkNegativeTest.java

@@ -25,10 +25,12 @@
 
 import org.junit.Test;
 
+import static org.elasticsearch.test.ElasticsearchIntegrationTest.*;
+
 /**
  * Tests for negative situations where we cannot run benchmarks
  */
-@ElasticsearchIntegrationTest.ClusterScope(scope = ElasticsearchIntegrationTest.Scope.SUITE)
+@ClusterScope(scope = Scope.SUITE, enableRandomBenchNodes = false)
 public class BenchmarkNegativeTest extends ElasticsearchIntegrationTest {
 
     private static final String INDEX_NAME = "test_index";

src/test/java/org/elasticsearch/test/ElasticsearchIntegrationTest.java

@@ -1118,6 +1118,12 @@ public static enum Scope {
          */
         int numClientNodes() default TestCluster.DEFAULT_NUM_CLIENT_NODES;
 
+        /**
+         * Returns whether the ability to randomly have benchmark (client) nodes as part of the cluster needs to be enabled.
+         * Default is {@link org.elasticsearch.test.TestCluster#DEFAULT_ENABLE_RANDOM_BENCH_NODES}.
+         */
+        boolean enableRandomBenchNodes() default TestCluster.DEFAULT_ENABLE_RANDOM_BENCH_NODES;
+
         /**
          * Returns the transport client ratio. By default this returns <code>-1</code> which means a random
          * ratio in the interval <code>[0..1]</code> is used.
@@ -1219,6 +1225,11 @@ private int getNumClientNodes() {
         return annotation == null ? TestCluster.DEFAULT_NUM_CLIENT_NODES : annotation.numClientNodes();
     }
 
+    private boolean enableRandomBenchNodes() {
+        ClusterScope annotation = getAnnotation(this.getClass());
+        return annotation == null ? TestCluster.DEFAULT_ENABLE_RANDOM_BENCH_NODES : annotation.enableRandomBenchNodes();
+    }
+
     private boolean randomDynamicTemplates() {
         ClusterScope annotation = getAnnotation(this.getClass());
         return annotation == null ? true : annotation.randomDynamicTemplates();
@@ -1255,7 +1266,8 @@ public Settings settings(int nodeOrdinal) {
         }
 
         int numClientNodes = getNumClientNodes();
-        return new TestCluster(currentClusterSeed, minNumDataNodes, maxNumDataNodes, clusterName(scope.name(), ElasticsearchTestCase.CHILD_VM_ID, currentClusterSeed), nodeSettingsSource, numClientNodes);
+        boolean enableRandomBenchNodes = enableRandomBenchNodes();
+        return new TestCluster(currentClusterSeed, minNumDataNodes, maxNumDataNodes, clusterName(scope.name(), ElasticsearchTestCase.CHILD_VM_ID, currentClusterSeed), nodeSettingsSource, numClientNodes, enableRandomBenchNodes);
     }
 
     /**

src/test/java/org/elasticsearch/test/ExternalTestCluster.java

@@ -47,6 +47,7 @@ public final class ExternalTestCluster extends ImmutableTestCluster {
     private final InetSocketAddress[] httpAddresses;
 
     private final int numDataNodes;
+    private final int numBenchNodes;
 
     public ExternalTestCluster(TransportAddress... transportAddresses) {
         this.client = new TransportClient(ImmutableSettings.settingsBuilder().put("client.transport.ignore_cluster_name", true))
@@ -55,14 +56,19 @@ public ExternalTestCluster(TransportAddress... transportAddresses) {
         NodesInfoResponse nodeInfos = this.client.admin().cluster().prepareNodesInfo().clear().setSettings(true).setHttp(true).get();
         httpAddresses = new InetSocketAddress[nodeInfos.getNodes().length];
         int dataNodes = 0;
+        int benchNodes = 0;
         for (int i = 0; i < nodeInfos.getNodes().length; i++) {
             NodeInfo nodeInfo = nodeInfos.getNodes()[i];
             httpAddresses[i] = ((InetSocketTransportAddress) nodeInfo.getHttp().address().publishAddress()).address();
             if (nodeInfo.getSettings().getAsBoolean("node.data", true)) {
                 dataNodes++;
             }
+            if (nodeInfo.getSettings().getAsBoolean("node.bench", false)) {
+                benchNodes++;
+            }
         }
         this.numDataNodes = dataNodes;
+        this.numBenchNodes = benchNodes;
         logger.info("Setup ExternalTestCluster [{}] made of [{}] nodes", nodeInfos.getClusterName().value(), size());
     }
 
@@ -86,6 +92,11 @@ public int numDataNodes() {
         return numDataNodes;
     }
 
+    @Override
+    public int numBenchNodes() {
+        return numBenchNodes;
+    }
+
     @Override
     public InetSocketAddress[] httpAddresses() {
         return httpAddresses;

src/test/java/org/elasticsearch/test/ImmutableTestCluster.java

@@ -100,6 +100,11 @@ public void assertAfterTest() {
      */
     public abstract int numDataNodes();
 
+    /**
+     * Returns the number of bench nodes in the cluster.
+     */
+    public abstract int numBenchNodes();
+
     /**
      * Returns the http addresses of the nodes within the cluster.
      * Can be used to run REST tests against the test cluster.

src/test/java/org/elasticsearch/test/TestCluster.java

@@ -82,6 +82,7 @@
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
 
+import static com.carrotsearch.randomizedtesting.RandomizedTest.frequently;
 import static com.carrotsearch.randomizedtesting.RandomizedTest.systemPropertyAsBoolean;
 import static org.apache.lucene.util.LuceneTestCase.rarely;
 import static org.apache.lucene.util.LuceneTestCase.usually;
@@ -126,6 +127,8 @@ public final class TestCluster extends ImmutableTestCluster {
     static final int DEFAULT_MIN_NUM_CLIENT_NODES = 0;
     static final int DEFAULT_MAX_NUM_CLIENT_NODES = 1;
 
+    static final boolean DEFAULT_ENABLE_RANDOM_BENCH_NODES = true;
+
     /* sorted map to make traverse order reproducible, concurrent since we do checks on it not within a sync block */
     private final NavigableMap<String, NodeAndClient> nodes = new TreeMap<>();
 
@@ -148,19 +151,21 @@ public final class TestCluster extends ImmutableTestCluster {
 
     private final int numSharedClientNodes;
 
+    private final boolean enableRandomBenchNodes;
+
     private final NodeSettingsSource nodeSettingsSource;
 
     private final ExecutorService executor;
 
     public TestCluster(long clusterSeed, String clusterName) {
-        this(clusterSeed, DEFAULT_MIN_NUM_DATA_NODES, DEFAULT_MAX_NUM_DATA_NODES, clusterName, NodeSettingsSource.EMPTY, DEFAULT_NUM_CLIENT_NODES);
+        this(clusterSeed, DEFAULT_MIN_NUM_DATA_NODES, DEFAULT_MAX_NUM_DATA_NODES, clusterName, NodeSettingsSource.EMPTY, DEFAULT_NUM_CLIENT_NODES, DEFAULT_ENABLE_RANDOM_BENCH_NODES);
     }
 
-    public TestCluster(long clusterSeed, int minNumDataNodes, int maxNumDataNodes, String clusterName, int numClientNodes) {
-        this(clusterSeed, minNumDataNodes, maxNumDataNodes, clusterName, NodeSettingsSource.EMPTY, numClientNodes);
+    public TestCluster(long clusterSeed, int minNumDataNodes, int maxNumDataNodes, String clusterName, int numClientNodes, boolean enableRandomBenchNodes) {
+        this(clusterSeed, minNumDataNodes, maxNumDataNodes, clusterName, NodeSettingsSource.EMPTY, numClientNodes, enableRandomBenchNodes);
     }
 
-    public TestCluster(long clusterSeed, int minNumDataNodes, int maxNumDataNodes, String clusterName, NodeSettingsSource nodeSettingsSource, int numClientNodes) {
+    public TestCluster(long clusterSeed, int minNumDataNodes, int maxNumDataNodes, String clusterName, NodeSettingsSource nodeSettingsSource, int numClientNodes, boolean enableRandomBenchNodes) {
         this.clusterName = clusterName;
 
         if (minNumDataNodes < 0 || maxNumDataNodes < 0) {
@@ -188,6 +193,8 @@ public TestCluster(long clusterSeed, int minNumDataNodes, int maxNumDataNodes, S
         }
         assert this.numSharedClientNodes >=0;
 
+        this.enableRandomBenchNodes = enableRandomBenchNodes;
+
         /*
          *  TODO
          *  - we might want start some master only nodes?
@@ -752,8 +759,12 @@ private synchronized void reset(boolean wipeData) {
             NodeAndClient nodeAndClient = nodes.get(buildNodeName);
             if (nodeAndClient == null) {
                 changed = true;
-                Settings clientSettings = ImmutableSettings.builder().put("node.data", "false").put("node.master", "false").build();
-                nodeAndClient = buildNode(i, sharedNodesSeeds[i], clientSettings, Version.CURRENT);
+                Builder clientSettingsBuilder = ImmutableSettings.builder().put("node.data", false).put("node.master", false);
+                if (enableRandomBenchNodes && frequently()) {
+                    //client nodes might also be bench nodes
+                    clientSettingsBuilder.put("node.bench", true);
+                }
+                nodeAndClient = buildNode(i, sharedNodesSeeds[i], clientSettingsBuilder.build(), Version.CURRENT);
                 nodeAndClient.node.start();
                 logger.info("Start Shared Node [{}] not shared", nodeAndClient.name);
             }
@@ -1243,6 +1254,11 @@ public int numDataNodes() {
         return dataNodeAndClients().size();
     }
 
+    @Override
+    public int numBenchNodes() {
+        return benchNodeAndClients().size();
+    }
+
     private synchronized Collection<NodeAndClient> dataNodeAndClients() {
         return Collections2.filter(nodes.values(), new DataNodePredicate());
     }
@@ -1274,6 +1290,17 @@ public boolean apply(NodeAndClient nodeAndClient) {
         }
     }
 
+    private synchronized Collection<NodeAndClient> benchNodeAndClients() {
+        return Collections2.filter(nodes.values(), new BenchNodePredicate());
+    }
+
+    private static final class BenchNodePredicate implements Predicate<NodeAndClient> {
+        @Override
+        public boolean apply(NodeAndClient nodeAndClient) {
+            return nodeAndClient.node.settings().getAsBoolean("node.bench", false);
+        }
+    }
+
     private static final class EntryNodePredicate implements Predicate<Map.Entry<String, NodeAndClient>> {
         private final Predicate<NodeAndClient> delegateNodePredicate;
 

src/test/java/org/elasticsearch/test/rest/support/Features.java

@@ -20,6 +20,7 @@
 package org.elasticsearch.test.rest.support;
 
 import com.google.common.collect.Lists;
+import org.elasticsearch.test.ElasticsearchIntegrationTest;
 
 import java.util.List;
 
@@ -44,17 +45,13 @@ private Features() {
      */
     public static boolean areAllSupported(List<String> features) {
         for (String feature : features) {
+            if ("benchmark".equals(feature) && ElasticsearchIntegrationTest.immutableCluster().numBenchNodes() > 0) {
+                continue;
+            }
             if (!SUPPORTED.contains(feature)) {
                 return false;
             }
         }
         return true;
     }
-
-    /**
-     * Returns all the supported features
-     */
-    public static List<String> getSupported() {
-        return SUPPORTED;
-    }
 }

src/test/java/org/elasticsearch/tribe/TribeTests.java

@@ -58,7 +58,7 @@ public class TribeTests extends ElasticsearchIntegrationTest {
     public static void setupSecondCluster() throws Exception {
         ElasticsearchIntegrationTest.beforeClass();
         // create another cluster
-        cluster2 = new TestCluster(randomLong(), 2, 2, Strings.randomBase64UUID(getRandom()), 0);
+        cluster2 = new TestCluster(randomLong(), 2, 2, Strings.randomBase64UUID(getRandom()), 0, false);
         cluster2.beforeTest(getRandom(), 0.1);
         cluster2.ensureAtLeastNumDataNodes(2);
     }