Skip to content

Commit

Permalink
Unify transport settings naming (#36623) (#36660)
Browse files Browse the repository at this point in the history
This commit updates our transport settings for 7.0. It generally takes a
few approaches. First, for normal transport settings, it usestransport.
instead of transport.tcp. Second, it uses transport.tcp, http.tcp,
or network.tcp for all settings that are proxies for OS level socket
settings. Third, it marks the network.tcp.connect_timeout setting for
removal. Network service level settings are only settings that apply to
both the http and transport modules. There is no connect timeout in
http. Fourth, it moves all the transport settings to a single class
TransportSettings similar to the HttpTransportSettings class.

This commit does not actually remove any settings. It just adds the new
renamed settings and adds todos for settings that will be deprecated.
  • Loading branch information
Tim-Brooks authored Dec 17, 2018
1 parent fef9d92 commit 8443c6b
Show file tree
Hide file tree
Showing 46 changed files with 387 additions and 288 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -272,11 +272,11 @@ public Netty4HttpServerTransport(Settings settings, NetworkService networkServic
this.port = SETTING_HTTP_PORT.get(settings);
// we can't make the network.bind_host a fallback since we already fall back to http.host hence the extra conditional here
List<String> httpBindHost = SETTING_HTTP_BIND_HOST.get(settings);
this.bindHosts = (httpBindHost.isEmpty() ? NetworkService.GLOBAL_NETWORK_BINDHOST_SETTING.get(settings) : httpBindHost)
this.bindHosts = (httpBindHost.isEmpty() ? NetworkService.GLOBAL_NETWORK_BIND_HOST_SETTING.get(settings) : httpBindHost)
.toArray(Strings.EMPTY_ARRAY);
// we can't make the network.publish_host a fallback since we already fall back to http.host hence the extra conditional here
List<String> httpPublishHost = SETTING_HTTP_PUBLISH_HOST.get(settings);
this.publishHosts = (httpPublishHost.isEmpty() ? NetworkService.GLOBAL_NETWORK_PUBLISHHOST_SETTING.get(settings) : httpPublishHost)
this.publishHosts = (httpPublishHost.isEmpty() ? NetworkService.GLOBAL_NETWORK_PUBLISH_HOST_SETTING.get(settings) : httpPublishHost)
.toArray(Strings.EMPTY_ARRAY);
this.tcpNoDelay = SETTING_HTTP_TCP_NO_DELAY.get(settings);
this.tcpKeepAlive = SETTING_HTTP_TCP_KEEP_ALIVE.get(settings);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
import org.elasticsearch.indices.breaker.CircuitBreakerService;
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.transport.TcpTransport;
import org.elasticsearch.transport.TransportSettings;

import java.io.IOException;
import java.net.InetSocketAddress;
Expand Down Expand Up @@ -149,22 +150,22 @@ private Bootstrap createClientBootstrap(NioEventLoopGroup eventLoopGroup) {
bootstrap.group(eventLoopGroup);
bootstrap.channel(NioSocketChannel.class);

bootstrap.option(ChannelOption.TCP_NODELAY, TCP_NO_DELAY.get(settings));
bootstrap.option(ChannelOption.SO_KEEPALIVE, TCP_KEEP_ALIVE.get(settings));
bootstrap.option(ChannelOption.TCP_NODELAY, TransportSettings.TCP_NO_DELAY.get(settings));
bootstrap.option(ChannelOption.SO_KEEPALIVE, TransportSettings.TCP_KEEP_ALIVE.get(settings));

final ByteSizeValue tcpSendBufferSize = TCP_SEND_BUFFER_SIZE.get(settings);
final ByteSizeValue tcpSendBufferSize = TransportSettings.TCP_SEND_BUFFER_SIZE.get(settings);
if (tcpSendBufferSize.getBytes() > 0) {
bootstrap.option(ChannelOption.SO_SNDBUF, Math.toIntExact(tcpSendBufferSize.getBytes()));
}

final ByteSizeValue tcpReceiveBufferSize = TCP_RECEIVE_BUFFER_SIZE.get(settings);
final ByteSizeValue tcpReceiveBufferSize = TransportSettings.TCP_RECEIVE_BUFFER_SIZE.get(settings);
if (tcpReceiveBufferSize.getBytes() > 0) {
bootstrap.option(ChannelOption.SO_RCVBUF, Math.toIntExact(tcpReceiveBufferSize.getBytes()));
}

bootstrap.option(ChannelOption.RCVBUF_ALLOCATOR, recvByteBufAllocator);

final boolean reuseAddress = TCP_REUSE_ADDRESS.get(settings);
final boolean reuseAddress = TransportSettings.TCP_REUSE_ADDRESS.get(settings);
bootstrap.option(ChannelOption.SO_REUSEADDR, reuseAddress);

return bootstrap;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
import org.elasticsearch.mocksocket.MockSocket;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.transport.TcpTransport;
import org.elasticsearch.transport.TransportSettings;
import org.junit.After;
import org.junit.Before;

Expand All @@ -51,8 +51,8 @@ public class Netty4SizeHeaderFrameDecoderTests extends ESTestCase {

private final Settings settings = Settings.builder()
.put("node.name", "NettySizeHeaderFrameDecoderTests")
.put(TcpTransport.BIND_HOST.getKey(), "127.0.0.1")
.put(TcpTransport.PORT.getKey(), "0")
.put(TransportSettings.BIND_HOST.getKey(), "127.0.0.1")
.put(TransportSettings.PORT.getKey(), "0")
.build();

private ThreadPool threadPool;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@
import org.elasticsearch.test.ESIntegTestCase.Scope;
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.transport.TcpChannel;
import org.elasticsearch.transport.TcpTransport;
import org.elasticsearch.transport.Transport;
import org.elasticsearch.transport.TransportSettings;

import java.io.IOException;
import java.net.InetSocketAddress;
Expand Down Expand Up @@ -80,7 +80,7 @@ public void testThatConnectionFailsAsIntended() throws Exception {
fail("Expected exception, but didn't happen");
} catch (ElasticsearchException e) {
assertThat(e.getMessage(), containsString("MY MESSAGE"));
assertThat(channelProfileName, is(TcpTransport.DEFAULT_PROFILE));
assertThat(channelProfileName, is(TransportSettings.DEFAULT_PROFILE));
}
}

Expand Down Expand Up @@ -116,7 +116,7 @@ protected String handleRequest(TcpChannel channel, String profileName,
InetSocketAddress remoteAddress, byte status) throws IOException {
String action = super.handleRequest(channel, profileName, stream, requestId, messageLengthBytes, version,
remoteAddress, status);
channelProfileName = TcpTransport.DEFAULT_PROFILE;
channelProfileName = TransportSettings.DEFAULT_PROFILE;
return action;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.elasticsearch.threadpool.TestThreadPool;
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.transport.TcpTransport;
import org.elasticsearch.transport.TransportSettings;
import org.junit.Before;

import java.util.Collections;
Expand All @@ -53,7 +54,7 @@ public void setup() {
public void testThatNettyCanBindToMultiplePorts() throws Exception {
Settings settings = Settings.builder()
.put("network.host", host)
.put(TcpTransport.PORT.getKey(), 22) // will not actually bind to this
.put(TransportSettings.PORT.getKey(), 22) // will not actually bind to this
.put("transport.profiles.default.port", 0)
.put("transport.profiles.client1.port", 0)
.build();
Expand All @@ -70,7 +71,7 @@ public void testThatNettyCanBindToMultiplePorts() throws Exception {
public void testThatDefaultProfileInheritsFromStandardSettings() throws Exception {
Settings settings = Settings.builder()
.put("network.host", host)
.put(TcpTransport.PORT.getKey(), 0)
.put(TransportSettings.PORT.getKey(), 0)
.put("transport.profiles.client1.port", 0)
.build();

Expand All @@ -87,7 +88,7 @@ public void testThatProfileWithoutPortSettingsFails() throws Exception {

Settings settings = Settings.builder()
.put("network.host", host)
.put(TcpTransport.PORT.getKey(), 0)
.put(TransportSettings.PORT.getKey(), 0)
.put("transport.profiles.client1.whatever", "foo")
.build();

Expand All @@ -103,7 +104,7 @@ public void testThatProfileWithoutPortSettingsFails() throws Exception {
public void testThatDefaultProfilePortOverridesGeneralConfiguration() throws Exception {
Settings settings = Settings.builder()
.put("network.host", host)
.put(TcpTransport.PORT.getKey(), 22) // will not actually bind to this
.put(TransportSettings.PORT.getKey(), 22) // will not actually bind to this
.put("transport.profiles.default.port", 0)
.build();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,8 @@
import org.elasticsearch.transport.ConnectTransportException;
import org.elasticsearch.transport.ConnectionProfile;
import org.elasticsearch.transport.TcpChannel;
import org.elasticsearch.transport.TcpTransport;
import org.elasticsearch.transport.Transport;
import org.elasticsearch.transport.TransportService;
import org.elasticsearch.transport.TransportSettings;

import java.net.InetAddress;
import java.net.UnknownHostException;
Expand Down Expand Up @@ -75,7 +74,7 @@ public void executeHandshake(DiscoveryNode node, TcpChannel channel, ConnectionP

@Override
protected MockTransportService build(Settings settings, Version version, ClusterSettings clusterSettings, boolean doHandshake) {
settings = Settings.builder().put(settings).put(TcpTransport.PORT.getKey(), "0").build();
settings = Settings.builder().put(settings).put(TransportSettings.PORT.getKey(), "0").build();
MockTransportService transportService = nettyFromThreadPool(settings, threadPool, version, clusterSettings, doHandshake);
transportService.start();
return transportService;
Expand All @@ -97,9 +96,9 @@ public void testBindUnavailableAddress() {
int port = serviceA.boundAddress().publishAddress().getPort();
Settings settings = Settings.builder()
.put(Node.NODE_NAME_SETTING.getKey(), "foobar")
.put(TransportService.TRACE_LOG_INCLUDE_SETTING.getKey(), "")
.put(TransportService.TRACE_LOG_EXCLUDE_SETTING.getKey(), "NOTHING")
.put("transport.tcp.port", port)
.put(TransportSettings.TRACE_LOG_INCLUDE_SETTING.getKey(), "")
.put(TransportSettings.TRACE_LOG_EXCLUDE_SETTING.getKey(), "NOTHING")
.put(TransportSettings.PORT.getKey(), port)
.build();
ClusterSettings clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS);
BindTransportException bindTransportException = expectThrows(BindTransportException.class, () -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@
import org.elasticsearch.env.NodeEnvironment;
import org.elasticsearch.node.Node;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.transport.TcpTransport;
import org.elasticsearch.transport.TransportSettings;

import java.io.IOException;
import java.util.Arrays;
Expand Down Expand Up @@ -142,11 +142,11 @@ public class TribeService extends AbstractLifecycleComponent {
// these settings should be passed through to each tribe client, if they are not set explicitly
private static final List<Setting<?>> PASS_THROUGH_SETTINGS = Arrays.asList(
NetworkService.GLOBAL_NETWORK_HOST_SETTING,
NetworkService.GLOBAL_NETWORK_BINDHOST_SETTING,
NetworkService.GLOBAL_NETWORK_PUBLISHHOST_SETTING,
TcpTransport.HOST,
TcpTransport.BIND_HOST,
TcpTransport.PUBLISH_HOST
NetworkService.GLOBAL_NETWORK_BIND_HOST_SETTING,
NetworkService.GLOBAL_NETWORK_PUBLISH_HOST_SETTING,
TransportSettings.HOST,
TransportSettings.BIND_HOST,
TransportSettings.PUBLISH_HOST
);
private final String onConflict;
private final Set<String> droppedIndices = ConcurrentCollections.newConcurrentSet();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ public List<TransportAddress> buildDynamicHosts(HostsResolver hostsResolver) {
InetAddress ipAddress = null;
try {
ipAddress = networkService.resolvePublishHostAddresses(
NetworkService.GLOBAL_NETWORK_PUBLISHHOST_SETTING.get(settings).toArray(Strings.EMPTY_ARRAY));
NetworkService.GLOBAL_NETWORK_PUBLISH_HOST_SETTING.get(settings).toArray(Strings.EMPTY_ARRAY));
logger.trace("ip of current node: [{}]", ipAddress);
} catch (IOException e) {
// We can't find the publish host address... Hmmm. Too bad :-(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
import org.elasticsearch.plugin.discovery.azure.classic.AzureDiscoveryPlugin;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.test.ESIntegTestCase;
import org.elasticsearch.transport.TcpTransport;
import org.elasticsearch.transport.TransportSettings;
import org.junit.AfterClass;
import org.junit.BeforeClass;
import org.junit.ClassRule;
Expand Down Expand Up @@ -118,7 +118,7 @@ protected Settings nodeSettings(int nodeOrdinal) {
return Settings.builder().put(super.nodeSettings(nodeOrdinal))
.put(DiscoveryModule.DISCOVERY_HOSTS_PROVIDER_SETTING.getKey(), AzureDiscoveryPlugin.AZURE)
.put(Environment.PATH_LOGS_SETTING.getKey(), resolve)
.put(TcpTransport.PORT.getKey(), 0)
.put(TransportSettings.PORT.getKey(), 0)
.put(Node.WRITE_PORTS_FILE_SETTING.getKey(), "true")
.put(AzureComputeService.Management.ENDPOINT_SETTING.getKey(), "https://" + InetAddress.getLoopbackAddress().getHostAddress() +
":" + httpsServer.getAddress().getPort())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ private InetAddress[] resolveEc2(String host, InetAddress ... expected) throws I
NetworkService networkService = new NetworkService(Collections.singletonList(new Ec2NameResolver()));

InetAddress[] addresses = networkService.resolveBindHostAddresses(
NetworkService.GLOBAL_NETWORK_BINDHOST_SETTING.get(nodeSettings).toArray(Strings.EMPTY_ARRAY));
NetworkService.GLOBAL_NETWORK_BIND_HOST_SETTING.get(nodeSettings).toArray(Strings.EMPTY_ARRAY));
if (expected == null) {
fail("We should get an IOException, resolved addressed:" + Arrays.toString(addresses));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ public List<TransportAddress> buildDynamicHosts(HostsResolver hostsResolver) {
String ipAddress = null;
try {
InetAddress inetAddress = networkService.resolvePublishHostAddresses(
NetworkService.GLOBAL_NETWORK_PUBLISHHOST_SETTING.get(settings).toArray(Strings.EMPTY_ARRAY));
NetworkService.GLOBAL_NETWORK_PUBLISH_HOST_SETTING.get(settings).toArray(Strings.EMPTY_ARRAY));
if (inetAddress != null) {
ipAddress = NetworkAddress.format(inetAddress);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ private void resolveGce(String gceNetworkSetting, InetAddress[] expected) throws
NetworkService networkService = new NetworkService(Collections.singletonList(new GceNameResolver(mock)));
try {
InetAddress[] addresses = networkService.resolveBindHostAddresses(
NetworkService.GLOBAL_NETWORK_BINDHOST_SETTING.get(nodeSettings).toArray(Strings.EMPTY_ARRAY));
NetworkService.GLOBAL_NETWORK_BIND_HOST_SETTING.get(nodeSettings).toArray(Strings.EMPTY_ARRAY));
if (expected == null) {
fail("We should get a IllegalArgumentException when setting network.host: _gce:doesnotexist_");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.elasticsearch.plugins.PluginsService;
import org.elasticsearch.secure_sm.SecureSM;
import org.elasticsearch.transport.TcpTransport;
import org.elasticsearch.transport.TransportSettings;

import java.io.IOException;
import java.net.SocketPermission;
Expand Down Expand Up @@ -370,7 +371,7 @@ private static void addSocketPermissionForTransportProfiles(final Permissions po
* @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 = TcpTransport.PORT.get(settings);
final String transportRange = TransportSettings.PORT.get(settings);
addSocketPermissionForPortRange(policy, transportRange);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,9 @@
import org.elasticsearch.search.SearchModule;
import org.elasticsearch.threadpool.ExecutorBuilder;
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.transport.TcpTransport;
import org.elasticsearch.transport.Transport;
import org.elasticsearch.transport.TransportService;
import org.elasticsearch.transport.TransportSettings;

import java.io.Closeable;
import java.util.ArrayList;
Expand Down Expand Up @@ -103,7 +103,7 @@ public abstract class TransportClient extends AbstractClient {

private static PluginsService newPluginService(final Settings settings, Collection<Class<? extends Plugin>> plugins) {
final Settings.Builder settingsBuilder = Settings.builder()
.put(TcpTransport.PING_SCHEDULE.getKey(), "5s") // enable by default the transport schedule ping interval
.put(TransportSettings.PING_SCHEDULE.getKey(), "5s") // enable by default the transport schedule ping interval
.put(InternalSettingsPreparer.prepareSettings(settings))
.put(NetworkService.NETWORK_SERVER.getKey(), false)
.put(CLIENT_TYPE_SETTING_S.getKey(), CLIENT_TYPE);
Expand Down Expand Up @@ -137,7 +137,7 @@ private static ClientTemplate buildTemplate(Settings providedSettings, Settings
Settings.builder()
.put(defaultSettings)
.put(pluginsService.updatedSettings())
.put(TcpTransport.FEATURE_PREFIX + "." + TRANSPORT_CLIENT_FEATURE, true)
.put(TransportSettings.FEATURE_PREFIX + "." + TRANSPORT_CLIENT_FEATURE, true)
.build();
final List<Closeable> resourcesToClose = new ArrayList<>();
final ThreadPool threadPool = new ThreadPool(settings);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,14 @@ public final class NetworkService {

/** By default, we bind to loopback interfaces */
public static final String DEFAULT_NETWORK_HOST = "_local_";
public static final Setting<Boolean> NETWORK_SERVER =
Setting.boolSetting("network.server", true, Property.NodeScope);
public static final Setting<List<String>> GLOBAL_NETWORK_HOST_SETTING =
Setting.listSetting("network.host", Collections.emptyList(), Function.identity(), Property.NodeScope);
public static final Setting<List<String>> GLOBAL_NETWORK_BINDHOST_SETTING =
public static final Setting<List<String>> GLOBAL_NETWORK_BIND_HOST_SETTING =
Setting.listSetting("network.bind_host", GLOBAL_NETWORK_HOST_SETTING, Function.identity(), Property.NodeScope);
public static final Setting<List<String>> GLOBAL_NETWORK_PUBLISHHOST_SETTING =
public static final Setting<List<String>> GLOBAL_NETWORK_PUBLISH_HOST_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 Setting<Boolean> TCP_NO_DELAY =
Setting.boolSetting("network.tcp.no_delay", true, Property.NodeScope);
Expand All @@ -57,6 +58,7 @@ public final class NetworkService {
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);
// TODO: Deprecate in 7.0
public static final Setting<TimeValue> TCP_CONNECT_TIMEOUT =
Setting.timeSetting("network.tcp.connect_timeout", new TimeValue(30, TimeUnit.SECONDS), Property.NodeScope);

Expand Down
Loading

0 comments on commit 8443c6b

Please sign in to comment.