From 6678da8c2892849891d5609f6d06d9b0720c2b4d Mon Sep 17 00:00:00 2001 From: javanna Date: Fri, 9 May 2014 21:58:29 +0200 Subject: [PATCH] [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 | 2 +- .../test/abort_benchmark/10_basic.yaml | 3 +- rest-api-spec/test/benchmark/10_basic.yaml | 5 +-- .../test/list_benchmarks/10_basic.yaml | 3 +- .../action/bench/BenchmarkNegativeTest.java | 4 +- .../test/ElasticsearchIntegrationTest.java | 14 ++++++- .../test/ExternalTestCluster.java | 11 ++++++ .../test/ImmutableTestCluster.java | 5 +++ .../org/elasticsearch/test/TestCluster.java | 39 ++++++++++++++++--- .../test/rest/support/Features.java | 11 ++---- .../org/elasticsearch/tribe/TribeTests.java | 2 +- 11 files changed, 75 insertions(+), 24 deletions(-) diff --git a/dev-tools/build_release.py b/dev-tools/build_release.py index 8085c26f42758..79ad1758f1078 100644 --- a/dev-tools/build_release.py +++ b/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() diff --git a/rest-api-spec/test/abort_benchmark/10_basic.yaml b/rest-api-spec/test/abort_benchmark/10_basic.yaml index f9928b7e42cdb..a48a6e2927f0b 100644 --- a/rest-api-spec/test/abort_benchmark/10_basic.yaml +++ b/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: diff --git a/rest-api-spec/test/benchmark/10_basic.yaml b/rest-api-spec/test/benchmark/10_basic.yaml index 287b698665581..9340b26b4d8d7 100644 --- a/rest-api-spec/test/benchmark/10_basic.yaml +++ b/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 } diff --git a/rest-api-spec/test/list_benchmarks/10_basic.yaml b/rest-api-spec/test/list_benchmarks/10_basic.yaml index b0c1649b7fefe..7ce1b7ac42942 100644 --- a/rest-api-spec/test/list_benchmarks/10_basic.yaml +++ b/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: {} diff --git a/src/test/java/org/elasticsearch/action/bench/BenchmarkNegativeTest.java b/src/test/java/org/elasticsearch/action/bench/BenchmarkNegativeTest.java index 3a5719388166c..534fde5d8fda3 100644 --- a/src/test/java/org/elasticsearch/action/bench/BenchmarkNegativeTest.java +++ b/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"; diff --git a/src/test/java/org/elasticsearch/test/ElasticsearchIntegrationTest.java b/src/test/java/org/elasticsearch/test/ElasticsearchIntegrationTest.java index 087a1dae22f35..96e8202cd6820 100644 --- a/src/test/java/org/elasticsearch/test/ElasticsearchIntegrationTest.java +++ b/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 -1 which means a random * ratio in the interval [0..1] 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); } /** diff --git a/src/test/java/org/elasticsearch/test/ExternalTestCluster.java b/src/test/java/org/elasticsearch/test/ExternalTestCluster.java index e7160d94c1f60..a46b3cd5ae21a 100644 --- a/src/test/java/org/elasticsearch/test/ExternalTestCluster.java +++ b/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; diff --git a/src/test/java/org/elasticsearch/test/ImmutableTestCluster.java b/src/test/java/org/elasticsearch/test/ImmutableTestCluster.java index e75f6cfef22a7..e23766b14c0f8 100644 --- a/src/test/java/org/elasticsearch/test/ImmutableTestCluster.java +++ b/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. diff --git a/src/test/java/org/elasticsearch/test/TestCluster.java b/src/test/java/org/elasticsearch/test/TestCluster.java index c700253057bb9..cb07db561a64f 100644 --- a/src/test/java/org/elasticsearch/test/TestCluster.java +++ b/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 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 dataNodeAndClients() { return Collections2.filter(nodes.values(), new DataNodePredicate()); } @@ -1274,6 +1290,17 @@ public boolean apply(NodeAndClient nodeAndClient) { } } + private synchronized Collection benchNodeAndClients() { + return Collections2.filter(nodes.values(), new BenchNodePredicate()); + } + + private static final class BenchNodePredicate implements Predicate { + @Override + public boolean apply(NodeAndClient nodeAndClient) { + return nodeAndClient.node.settings().getAsBoolean("node.bench", false); + } + } + private static final class EntryNodePredicate implements Predicate> { private final Predicate delegateNodePredicate; diff --git a/src/test/java/org/elasticsearch/test/rest/support/Features.java b/src/test/java/org/elasticsearch/test/rest/support/Features.java index c51d49e7ad7fd..d763be701746b 100644 --- a/src/test/java/org/elasticsearch/test/rest/support/Features.java +++ b/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 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 getSupported() { - return SUPPORTED; - } } diff --git a/src/test/java/org/elasticsearch/tribe/TribeTests.java b/src/test/java/org/elasticsearch/tribe/TribeTests.java index b35763be8e044..769b1848f0b53 100644 --- a/src/test/java/org/elasticsearch/tribe/TribeTests.java +++ b/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); }