Skip to content

Cleanup network / transport related settings #25489

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jul 2, 2017
Merged

Cleanup network / transport related settings #25489

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cc53edd
Cleanup network / transport related settings
s1monw Jun 30, 2017
561b40a
fix GCE tests
s1monw Jun 30, 2017
a3b7883
Merge branch 'master' into fix_network_settings
s1monw Jun 30, 2017
f22d716
apply feedback
s1monw Jul 2, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions core/src/main/java/org/elasticsearch/bootstrap/Security.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
import org.elasticsearch.env.Environment;
import org.elasticsearch.http.HttpTransportSettings;
import org.elasticsearch.plugins.PluginInfo;
import org.elasticsearch.transport.TransportSettings;
import org.elasticsearch.transport.TcpTransport;

import java.io.FilePermission;
import java.io.IOException;
Expand Down Expand Up @@ -324,8 +324,8 @@ private static void addSocketPermissionForTransportProfiles(
final Permissions policy,
final Settings settings) {
// transport is way over-engineered
final Map<String, Settings> profiles = new HashMap<>(TransportSettings.TRANSPORT_PROFILES_SETTING.get(settings).getAsGroups());
profiles.putIfAbsent(TransportSettings.DEFAULT_PROFILE, Settings.EMPTY);
final Map<String, Settings> profiles = new HashMap<>(TcpTransport.TRANSPORT_PROFILES_SETTING.get(settings).getAsGroups());
profiles.putIfAbsent(TcpTransport.DEFAULT_PROFILE, Settings.EMPTY);

// loop through all profiles and add permissions for each one, if it's valid; otherwise Netty transports are lenient and ignores it
for (final Map.Entry<String, Settings> entry : profiles.entrySet()) {
Expand All @@ -335,7 +335,7 @@ private static void addSocketPermissionForTransportProfiles(
// a profile is only valid if it's the default profile, or if it has an actual name and specifies a port
// TODO: can this leniency be removed?
final boolean valid =
TransportSettings.DEFAULT_PROFILE.equals(name) ||
TcpTransport.DEFAULT_PROFILE.equals(name) ||
(name != null && name.length() > 0 && profileSettings.get("port") != null);
if (valid) {
final String transportRange = profileSettings.get("port");
Expand All @@ -355,7 +355,7 @@ private static void addSocketPermissionForTransportProfiles(
* @param settings the {@link Settings} instance to read the transport settings from
*/
private static void addSocketPermissionForTransport(final Permissions policy, final Settings settings) {
final String transportRange = TransportSettings.PORT.get(settings);
final String transportRange = TcpTransport.PORT.get(settings);
addSocketPermissionForPortRange(policy, transportRange);
}

Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/org/elasticsearch/client/transport/TransportClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ private static ClientTemplate buildTemplate(Settings providedSettings, Settings
final List<Closeable> resourcesToClose = new ArrayList<>();
final ThreadPool threadPool = new ThreadPool(settings);
resourcesToClose.add(() -> ThreadPool.terminate(threadPool, 10, TimeUnit.SECONDS));
final NetworkService networkService = new NetworkService(settings, Collections.emptyList());
final NetworkService networkService = new NetworkService(Collections.emptyList());
try {
final List<Setting<?>> additionalSettings = new ArrayList<>(pluginsService.getPluginSettings());
final List<String> additionalSettingsFilter = new ArrayList<>(pluginsService.getPluginSettingsFilter());
Expand Down
88 changes: 35 additions & 53 deletions core/src/main/java/org/elasticsearch/common/network/NetworkService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,50 +19,45 @@

package org.elasticsearch.common.network;

import org.elasticsearch.common.Strings;
import org.elasticsearch.common.component.AbstractComponent;
import org.elasticsearch.common.settings.Setting;
import org.elasticsearch.common.settings.Setting.Property;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.unit.ByteSizeValue;
import org.elasticsearch.common.unit.TimeValue;

import java.io.IOException;
import java.net.InetAddress;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.concurrent.TimeUnit;
import java.util.function.Function;

public class NetworkService extends AbstractComponent {
public final class NetworkService {

/** By default, we bind to loopback interfaces */
public static final String DEFAULT_NETWORK_HOST = "_local_";

public static final Setting<List<String>> GLOBAL_NETWORK_HOST_SETTING =
Setting.listSetting("network.host", Arrays.asList(DEFAULT_NETWORK_HOST), Function.identity(), Property.NodeScope);
Setting.listSetting("network.host", Collections.emptyList(), Function.identity(), Property.NodeScope);
public static final Setting<List<String>> GLOBAL_NETWORK_BINDHOST_SETTING =
Setting.listSetting("network.bind_host", GLOBAL_NETWORK_HOST_SETTING, Function.identity(), Property.NodeScope);
public static final Setting<List<String>> GLOBAL_NETWORK_PUBLISHHOST_SETTING =
Setting.listSetting("network.publish_host", GLOBAL_NETWORK_HOST_SETTING, Function.identity(), Property.NodeScope);
public static final Setting<Boolean> NETWORK_SERVER = Setting.boolSetting("network.server", true, Property.NodeScope);

public static final class TcpSettings {
public static final Setting<Boolean> TCP_NO_DELAY =
Setting.boolSetting("network.tcp.no_delay", true, Property.NodeScope);
public static final Setting<Boolean> TCP_KEEP_ALIVE =
Setting.boolSetting("network.tcp.keep_alive", true, Property.NodeScope);
public static final Setting<Boolean> TCP_REUSE_ADDRESS =
Setting.boolSetting("network.tcp.reuse_address", NetworkUtils.defaultReuseAddress(), Property.NodeScope);
public static final Setting<ByteSizeValue> TCP_SEND_BUFFER_SIZE =
Setting.byteSizeSetting("network.tcp.send_buffer_size", new ByteSizeValue(-1), Property.NodeScope);
public static final Setting<ByteSizeValue> TCP_RECEIVE_BUFFER_SIZE =
Setting.byteSizeSetting("network.tcp.receive_buffer_size", new ByteSizeValue(-1), Property.NodeScope);
public static final Setting<TimeValue> TCP_CONNECT_TIMEOUT =
Setting.timeSetting("network.tcp.connect_timeout", new TimeValue(30, TimeUnit.SECONDS), Property.NodeScope);
}
public static final Setting<Boolean> TCP_NO_DELAY =
Setting.boolSetting("network.tcp.no_delay", true, Property.NodeScope);
public static final Setting<Boolean> TCP_KEEP_ALIVE =
Setting.boolSetting("network.tcp.keep_alive", true, Property.NodeScope);
public static final Setting<Boolean> TCP_REUSE_ADDRESS =
Setting.boolSetting("network.tcp.reuse_address", NetworkUtils.defaultReuseAddress(), Property.NodeScope);
public static final Setting<ByteSizeValue> TCP_SEND_BUFFER_SIZE =
Setting.byteSizeSetting("network.tcp.send_buffer_size", new ByteSizeValue(-1), Property.NodeScope);
public static final Setting<ByteSizeValue> TCP_RECEIVE_BUFFER_SIZE =
Setting.byteSizeSetting("network.tcp.receive_buffer_size", new ByteSizeValue(-1), Property.NodeScope);
public static final Setting<TimeValue> TCP_CONNECT_TIMEOUT =
Setting.timeSetting("network.tcp.connect_timeout", new TimeValue(30, TimeUnit.SECONDS), Property.NodeScope);

/**
* A custom name resolver can support custom lookup keys (my_net_key:ipv4) and also change
Expand All @@ -82,8 +77,7 @@ public interface CustomNameResolver {

private final List<CustomNameResolver> customNameResolvers;

public NetworkService(Settings settings, List<CustomNameResolver> customNameResolvers) {
super(settings);
public NetworkService(List<CustomNameResolver> customNameResolvers) {
this.customNameResolvers = customNameResolvers;
}

Expand All @@ -92,29 +86,23 @@ public NetworkService(Settings settings, List<CustomNameResolver> customNameReso
* not contain duplicate addresses.
*
* @param bindHosts list of hosts to bind to. this may contain special pseudo-hostnames
* such as _local_ (see the documentation). if it is null, it will be populated
* based on global default settings.
* such as _local_ (see the documentation). if it is null, it will fall back to _local_
*
* @return unique set of internet addresses
*/
public InetAddress[] resolveBindHostAddresses(String bindHosts[]) throws IOException {
// first check settings
if (bindHosts == null || bindHosts.length == 0) {
if (GLOBAL_NETWORK_BINDHOST_SETTING.exists(settings) || GLOBAL_NETWORK_HOST_SETTING.exists(settings)) {
// if we have settings use them (we have a fallback to GLOBAL_NETWORK_HOST_SETTING inline
bindHosts = GLOBAL_NETWORK_BINDHOST_SETTING.get(settings).toArray(Strings.EMPTY_ARRAY);
} else {
// next check any registered custom resolvers if any
if (customNameResolvers != null) {
for (CustomNameResolver customNameResolver : customNameResolvers) {
InetAddress addresses[] = customNameResolver.resolveDefault();
if (addresses != null) {
return addresses;
}
// next check any registered custom resolvers if any
Copy link
Member

@rjernst rjernst Jul 1, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I think remove "next", since this is no longer a second step in the method

if (customNameResolvers != null) {
Copy link
Member

@rjernst rjernst Jul 1, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this can ever be null (it is initialized as an empty list when filling in Node), so you can add Objects.requireNonNull in the ctor and remove this check?

Copy link
Member

@rjernst rjernst Jul 1, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And ditto for this comment and the one above in the similar method below.

for (CustomNameResolver customNameResolver : customNameResolvers) {
InetAddress addresses[] = customNameResolver.resolveDefault();
if (addresses != null) {
return addresses;
}
}
// we know it's not here. get the defaults
bindHosts = GLOBAL_NETWORK_BINDHOST_SETTING.get(settings).toArray(Strings.EMPTY_ARRAY);
}
// we know it's not here. get the defaults
bindHosts = new String[] {"_local_"};
}

InetAddress addresses[] = resolveInetAddresses(bindHosts);
Expand All @@ -140,29 +128,23 @@ public InetAddress[] resolveBindHostAddresses(String bindHosts[]) throws IOExcep
* If {@code publishHosts} resolves to more than one address, <b>then one is selected with magic</b>
*
* @param publishHosts list of hosts to publish as. this may contain special pseudo-hostnames
* such as _local_ (see the documentation). if it is null, it will be populated
* based on global default settings.
* such as _local_ (see the documentation). if it is null, it will fall back to _local_
* @return single internet address
*/
// TODO: needs to be InetAddress[]
public InetAddress resolvePublishHostAddresses(String publishHosts[]) throws IOException {
if (publishHosts == null || publishHosts.length == 0) {
if (GLOBAL_NETWORK_PUBLISHHOST_SETTING.exists(settings) || GLOBAL_NETWORK_HOST_SETTING.exists(settings)) {
// if we have settings use them (we have a fallback to GLOBAL_NETWORK_HOST_SETTING inline
publishHosts = GLOBAL_NETWORK_PUBLISHHOST_SETTING.get(settings).toArray(Strings.EMPTY_ARRAY);
} else {
// next check any registered custom resolvers if any
if (customNameResolvers != null) {
for (CustomNameResolver customNameResolver : customNameResolvers) {
InetAddress addresses[] = customNameResolver.resolveDefault();
if (addresses != null) {
return addresses[0];
}
// next check any registered custom resolvers if any
if (customNameResolvers != null) {
for (CustomNameResolver customNameResolver : customNameResolvers) {
InetAddress addresses[] = customNameResolver.resolveDefault();
if (addresses != null) {
return addresses[0];
}
}
// we know it's not here. get the defaults
publishHosts = GLOBAL_NETWORK_PUBLISHHOST_SETTING.get(settings).toArray(Strings.EMPTY_ARRAY);
}
// we know it's not here. get the defaults
publishHosts = new String[] {DEFAULT_NETWORK_HOST};
}

InetAddress addresses[] = resolveInetAddresses(publishHosts);
Expand Down
25 changes: 12 additions & 13 deletions core/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@
import org.elasticsearch.transport.TcpTransport;
import org.elasticsearch.transport.Transport;
import org.elasticsearch.transport.TransportService;
import org.elasticsearch.transport.TransportSettings;
import org.elasticsearch.tribe.TribeService;
import org.elasticsearch.watcher.ResourceWatcherService;

Expand Down Expand Up @@ -270,12 +269,12 @@ public void apply(Settings value, Settings current, Settings previous) {
HierarchyCircuitBreakerService.FIELDDATA_CIRCUIT_BREAKER_TYPE_SETTING,
HierarchyCircuitBreakerService.REQUEST_CIRCUIT_BREAKER_TYPE_SETTING,
Transport.TRANSPORT_TCP_COMPRESS,
TransportSettings.TRANSPORT_PROFILES_SETTING,
TransportSettings.HOST,
TransportSettings.PUBLISH_HOST,
TransportSettings.BIND_HOST,
TransportSettings.PUBLISH_PORT,
TransportSettings.PORT,
TcpTransport.TRANSPORT_PROFILES_SETTING,
TcpTransport.HOST,
TcpTransport.PUBLISH_HOST,
TcpTransport.BIND_HOST,
TcpTransport.PUBLISH_PORT,
TcpTransport.PORT,
TcpTransport.CONNECTIONS_PER_NODE_RECOVERY,
TcpTransport.CONNECTIONS_PER_NODE_BULK,
TcpTransport.CONNECTIONS_PER_NODE_REG,
Expand All @@ -292,12 +291,12 @@ public void apply(Settings value, Settings current, Settings previous) {
NetworkService.GLOBAL_NETWORK_HOST_SETTING,
NetworkService.GLOBAL_NETWORK_BINDHOST_SETTING,
NetworkService.GLOBAL_NETWORK_PUBLISHHOST_SETTING,
NetworkService.TcpSettings.TCP_NO_DELAY,
NetworkService.TcpSettings.TCP_KEEP_ALIVE,
NetworkService.TcpSettings.TCP_REUSE_ADDRESS,
NetworkService.TcpSettings.TCP_SEND_BUFFER_SIZE,
NetworkService.TcpSettings.TCP_RECEIVE_BUFFER_SIZE,
NetworkService.TcpSettings.TCP_CONNECT_TIMEOUT,
NetworkService.TCP_NO_DELAY,
NetworkService.TCP_KEEP_ALIVE,
NetworkService.TCP_REUSE_ADDRESS,
NetworkService.TCP_SEND_BUFFER_SIZE,
NetworkService.TCP_RECEIVE_BUFFER_SIZE,
NetworkService.TCP_CONNECT_TIMEOUT,
IndexSettings.QUERY_STRING_ANALYZE_WILDCARD,
IndexSettings.QUERY_STRING_ALLOW_LEADING_WILDCARD,
ScriptService.SCRIPT_CACHE_SIZE_SETTING,
Expand Down
6 changes: 1 addition & 5 deletions core/src/main/java/org/elasticsearch/node/Node.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@
import org.elasticsearch.cluster.routing.RoutingService;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.StopWatch;
import org.elasticsearch.common.SuppressForbidden;
import org.elasticsearch.common.component.Lifecycle;
import org.elasticsearch.common.component.LifecycleComponent;
import org.elasticsearch.common.inject.Binder;
Expand All @@ -61,7 +60,6 @@
import org.elasticsearch.common.inject.Module;
import org.elasticsearch.common.inject.ModulesBuilder;
import org.elasticsearch.common.inject.util.Providers;
import org.elasticsearch.common.io.PathUtils;
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
import org.elasticsearch.common.lease.Releasables;
import org.elasticsearch.common.logging.DeprecationLogger;
Expand Down Expand Up @@ -151,9 +149,7 @@
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import java.util.function.Consumer;
Expand Down Expand Up @@ -330,7 +326,7 @@ protected Node(final Environment environment, Collection<Class<? extends Plugin>
final SettingsModule settingsModule = new SettingsModule(this.settings, additionalSettings, additionalSettingsFilter);
scriptModule.registerClusterSettingsListeners(settingsModule.getClusterSettings());
resourcesToClose.add(resourceWatcherService);
final NetworkService networkService = new NetworkService(settings,
final NetworkService networkService = new NetworkService(
getCustomNameResolvers(pluginsService.filterPlugins(DiscoveryPlugin.class)));
final ClusterService clusterService = new ClusterService(settings, settingsModule.getClusterSettings(), threadPool);
clusterService.addListener(scriptModule.getScriptService());
Expand Down
Loading