Skip to content

Commit

Permalink
Use file-based discovery not MockUncasedHostsProvider (#33554)
Browse files Browse the repository at this point in the history
Today we use a special unicast hosts provider, the `MockUncasedHostsProvider`,
in many integration tests, to deal with the dynamic nature of the allocation of
ports to nodes. However #33241 allows us to use file-based discovery to achieve
the same goal, so the special test-only `MockUncasedHostsProvider` is no longer
required.

This change removes `MockUncasedHostProvider` and replaces it with file-based
discovery in tests based on `EsIntegTestCase`.
  • Loading branch information
DaveCTurner authored Sep 13, 2018
1 parent 94f6d45 commit 5a3fd8e
Show file tree
Hide file tree
Showing 10 changed files with 178 additions and 164 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ public DiscoveryModule(Settings settings, ThreadPool threadPool, TransportServic
if (discoverySupplier == null) {
throw new IllegalArgumentException("Unknown discovery type [" + discoveryType + "]");
}
Loggers.getLogger(getClass(), settings).info("using discovery type [{}]", discoveryType);
Loggers.getLogger(getClass(), settings).info("using discovery type [{}] and host providers {}", discoveryType, hostsProviderNames);
discovery = Objects.requireNonNull(discoverySupplier.get());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@
import java.util.Collection;
import java.util.List;
import java.util.Set;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.atomic.AtomicBoolean;

import static java.util.Collections.emptySet;
Expand Down Expand Up @@ -113,6 +112,7 @@ public void blockActions(String... actions) {
@Override
protected Settings nodeSettings(int nodeOrdinal) {
return Settings.builder()
.put(super.nodeSettings(nodeOrdinal))
// manual collection or upon cluster forming.
.put(NodeEnvironment.MAX_LOCAL_STORAGE_NODES_SETTING.getKey(), 2)
.put(InternalClusterInfoService.INTERNAL_CLUSTER_INFO_TIMEOUT_SETTING.getKey(), "1s")
Expand All @@ -121,8 +121,7 @@ protected Settings nodeSettings(int nodeOrdinal) {

@Override
protected Collection<Class<? extends Plugin>> nodePlugins() {
return Arrays.asList(TestPlugin.class,
MockTransportService.TestPlugin.class);
return Arrays.asList(TestPlugin.class, MockTransportService.TestPlugin.class);
}

public void testClusterInfoServiceCollectsInformation() throws Exception {
Expand Down Expand Up @@ -172,7 +171,7 @@ public void testClusterInfoServiceCollectsInformation() throws Exception {
}
}

public void testClusterInfoServiceInformationClearOnError() throws InterruptedException, ExecutionException {
public void testClusterInfoServiceInformationClearOnError() {
internalCluster().startNodes(2,
// manually control publishing
Settings.builder().put(InternalClusterInfoService.INTERNAL_CLUSTER_INFO_UPDATE_INTERVAL_SETTING.getKey(), "60m").build());
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
/*
* Licensed to Elasticsearch under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

package org.elasticsearch.discovery.zen;

import org.elasticsearch.action.admin.cluster.node.info.NodesInfoRequest;
import org.elasticsearch.action.admin.cluster.node.info.NodesInfoResponse;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.test.ESIntegTestCase;

import static org.elasticsearch.discovery.DiscoveryModule.DISCOVERY_HOSTS_PROVIDER_SETTING;
import static org.elasticsearch.discovery.zen.SettingsBasedHostsProvider.DISCOVERY_ZEN_PING_UNICAST_HOSTS_SETTING;
import static org.elasticsearch.discovery.zen.SettingsBasedHostsProvider.LIMIT_LOCAL_PORTS_COUNT;
import static org.elasticsearch.transport.TcpTransport.PORT;

@ESIntegTestCase.ClusterScope(scope = ESIntegTestCase.Scope.TEST, numDataNodes = 0, numClientNodes = 0)
public class SettingsBasedHostProviderIT extends ESIntegTestCase {

@Override
protected Settings nodeSettings(int nodeOrdinal) {
Settings.Builder builder = Settings.builder().put(super.nodeSettings(nodeOrdinal));

// super.nodeSettings enables file-based discovery, but here we disable it again so we can test the static list:
if (randomBoolean()) {
builder.putList(DISCOVERY_HOSTS_PROVIDER_SETTING.getKey());
} else {
builder.remove(DISCOVERY_HOSTS_PROVIDER_SETTING.getKey());
}

// super.nodeSettings sets this to an empty list, which disables any search for other nodes, but here we want this to happen:
builder.remove(DISCOVERY_ZEN_PING_UNICAST_HOSTS_SETTING.getKey());

return builder.build();
}

public void testClusterFormsWithSingleSeedHostInSettings() {
final String seedNodeName = internalCluster().startNode();
final NodesInfoResponse nodesInfoResponse
= client(seedNodeName).admin().cluster().nodesInfo(new NodesInfoRequest("_local")).actionGet();
final String seedNodeAddress = nodesInfoResponse.getNodes().get(0).getTransport().getAddress().publishAddress().toString();
logger.info("--> using seed node address {}", seedNodeAddress);

int extraNodes = randomIntBetween(1, 5);
internalCluster().startNodes(extraNodes,
Settings.builder().putList(DISCOVERY_ZEN_PING_UNICAST_HOSTS_SETTING.getKey(), seedNodeAddress).build());

ensureStableCluster(extraNodes + 1);
}

public void testClusterFormsByScanningPorts() {
// This test will fail if all 4 ports just less than the one used by the first node are already bound by something else. It's hard
// to know how often this might happen in reality, so let's try it and see.

final String seedNodeName = internalCluster().startNode();
final NodesInfoResponse nodesInfoResponse
= client(seedNodeName).admin().cluster().nodesInfo(new NodesInfoRequest("_local")).actionGet();
final int seedNodePort = nodesInfoResponse.getNodes().get(0).getTransport().getAddress().publishAddress().getPort();
final int minPort = randomIntBetween(seedNodePort - LIMIT_LOCAL_PORTS_COUNT + 1, seedNodePort - 1);
final String portSpec = minPort + "-" + seedNodePort;

logger.info("--> using port specification [{}]", portSpec);
internalCluster().startNode(Settings.builder().put(PORT.getKey(), portSpec));
ensureStableCluster(2);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,10 @@ protected Collection<Class<? extends Plugin>> nodePlugins() {
protected Settings nodeSettings(int nodeOrdinal) {
boolean lowLevelCancellation = randomBoolean();
logger.info("Using lowLevelCancellation: {}", lowLevelCancellation);
return Settings.builder().put(SearchService.LOW_LEVEL_CANCELLATION_SETTING.getKey(), lowLevelCancellation).build();
return Settings.builder()
.put(super.nodeSettings(nodeOrdinal))
.put(SearchService.LOW_LEVEL_CANCELLATION_SETTING.getKey(), lowLevelCancellation)
.build();
}

private void indexTestData() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,8 @@
import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_REPLICAS;
import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_SHARDS;
import static org.elasticsearch.common.util.CollectionUtils.eagerPartition;
import static org.elasticsearch.discovery.DiscoveryModule.DISCOVERY_HOSTS_PROVIDER_SETTING;
import static org.elasticsearch.discovery.zen.SettingsBasedHostsProvider.DISCOVERY_ZEN_PING_UNICAST_HOSTS_SETTING;
import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery;
import static org.elasticsearch.test.XContentTestUtils.convertToMap;
import static org.elasticsearch.test.XContentTestUtils.differenceBetweenMapsIgnoringArrayOrder;
Expand Down Expand Up @@ -1808,7 +1810,9 @@ protected Settings nodeSettings(int nodeOrdinal) {
// wait short time for other active shards before actually deleting, default 30s not needed in tests
.put(IndicesStore.INDICES_STORE_DELETE_SHARD_TIMEOUT.getKey(), new TimeValue(1, TimeUnit.SECONDS))
// randomly enable low-level search cancellation to make sure it does not alter results
.put(SearchService.LOW_LEVEL_CANCELLATION_SETTING.getKey(), randomBoolean());
.put(SearchService.LOW_LEVEL_CANCELLATION_SETTING.getKey(), randomBoolean())
.putList(DISCOVERY_ZEN_PING_UNICAST_HOSTS_SETTING.getKey()) // empty list disables a port scan for other nodes
.putList(DISCOVERY_HOSTS_PROVIDER_SETTING.getKey(), "file");
if (rarely()) {
// Sometimes adjust the minimum search thread pool size, causing
// QueueResizingEsThreadPoolExecutor to be used instead of a regular
Expand Down Expand Up @@ -1921,7 +1925,7 @@ protected NodeConfigurationSource getNodeConfigSource() {
networkSettings.put(NetworkModule.TRANSPORT_TYPE_KEY, getTestTransportType());
}

NodeConfigurationSource nodeConfigurationSource = new NodeConfigurationSource() {
return new NodeConfigurationSource() {
@Override
public Settings nodeSettings(int nodeOrdinal) {
return Settings.builder()
Expand Down Expand Up @@ -1955,7 +1959,6 @@ public Collection<Class<? extends Plugin>> transportClientPlugins() {
return Collections.unmodifiableCollection(plugins);
}
};
return nodeConfigurationSource;
}

/**
Expand Down Expand Up @@ -2029,7 +2032,7 @@ protected Collection<Class<? extends Plugin>> getMockPlugins() {
public static final class TestSeedPlugin extends Plugin {
@Override
public List<Setting<?>> getSettings() {
return Arrays.asList(INDEX_TEST_SEED_SETTING);
return Collections.singletonList(INDEX_TEST_SEED_SETTING);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
import java.util.Collection;
import java.util.Collections;

import static org.elasticsearch.discovery.zen.SettingsBasedHostsProvider.DISCOVERY_ZEN_PING_UNICAST_HOSTS_SETTING;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.lessThanOrEqualTo;
Expand Down Expand Up @@ -197,6 +198,7 @@ private Node newNode() {
// turning on the real memory circuit breaker leads to spurious test failures. As have no full control over heap usage, we
// turn it off for these tests.
.put(HierarchyCircuitBreakerService.USE_REAL_MEMORY_USAGE_SETTING.getKey(), false)
.putList(DISCOVERY_ZEN_PING_UNICAST_HOSTS_SETTING.getKey()) // empty list disables a port scan for other nodes
.put(nodeSettings()) // allow test cases to provide their own settings or override these
.build();
Collection<Class<? extends Plugin>> plugins = getPlugins();
Expand Down
Loading

0 comments on commit 5a3fd8e

Please sign in to comment.