From 05b46d47f5a4456aaafa81cd6cc148498b07581a Mon Sep 17 00:00:00 2001 From: Yannick Welsch Date: Thu, 9 Jul 2020 09:08:43 +0200 Subject: [PATCH 01/15] Set specific keepalive options by default on supported platforms --- .../rest-api-spec/test/20_settings.yml | 9 ++++++ docs/reference/modules/transport.asciidoc | 15 +++++---- .../transport/netty4/Netty4Transport.java | 32 +++++++++---------- .../common/network/NetworkService.java | 13 ++++++-- .../transport/TransportSettings.java | 15 +++++---- 5 files changed, 52 insertions(+), 32 deletions(-) create mode 100644 distribution/docker/src/test/resources/rest-api-spec/test/20_settings.yml diff --git a/distribution/docker/src/test/resources/rest-api-spec/test/20_settings.yml b/distribution/docker/src/test/resources/rest-api-spec/test/20_settings.yml new file mode 100644 index 0000000000000..60dfb9f0c8e55 --- /dev/null +++ b/distribution/docker/src/test/resources/rest-api-spec/test/20_settings.yml @@ -0,0 +1,9 @@ +--- +"Default TCP keepalive settings": + - do: + cluster.get_settings: + include_defaults: true + + - match: { defaults.network.tcp.keep_idle: "60" } + - match: { defaults.network.tcp.keep_interval: "10" } + - match: { defaults.network.tcp.keep_count: "3" } diff --git a/docs/reference/modules/transport.asciidoc b/docs/reference/modules/transport.asciidoc index aadf2539126fa..d5a22c2f59d71 100644 --- a/docs/reference/modules/transport.asciidoc +++ b/docs/reference/modules/transport.asciidoc @@ -88,19 +88,22 @@ example above: determines the time in seconds that a connection must be idle before starting to send TCP keepalive probes. Only applicable on Linux and Mac, and requires JDK 11 or newer. - Defaults to -1, which does not set this option at the socket level, but - uses default system configuration instead. + Defaults to 60 on applicable configurations, and -1 otherwise, which does + not set this option at the socket level, but uses the default system + configuration instead. * `tcp.keep_interval`: Configures the `TCP_KEEPINTVL` option for this socket, which determines the time in seconds between sending TCP keepalive probes. Only applicable on Linux and Mac, and requires JDK 11 or newer. - Defaults to -1, which does not set this option at the socket level, but - uses default system configuration instead. + Defaults to 10 on applicable configurations, and -1 otherwise, which does + not set this option at the socket level, but uses the default system + configuration instead. * `tcp.keep_count`: Configures the `TCP_KEEPCNT` option for this socket, which determines the number of unacknowledged TCP keepalive probes that may be sent on a connection before it is dropped. Only applicable on Linux and Mac, and requires JDK 11 or newer. - Defaults to -1, which does not set this option at the socket level, but - uses default system configuration instead. + Defaults to 3 on applicable configurations, and -1 otherwise, which does + not set this option at the socket level, but uses the default system + configuration instead. * `tcp.reuse_address`: Configures the `SO_REUSEADDR` option for this socket * `tcp.send_buffer_size`: Configures the send buffer size of the socket * `tcp.receive_buffer_size`: Configures the receive buffer size of the socket diff --git a/modules/transport-netty4/src/main/java/org/elasticsearch/transport/netty4/Netty4Transport.java b/modules/transport-netty4/src/main/java/org/elasticsearch/transport/netty4/Netty4Transport.java index 4fb4bfdd25c70..6d02163296268 100644 --- a/modules/transport-netty4/src/main/java/org/elasticsearch/transport/netty4/Netty4Transport.java +++ b/modules/transport-netty4/src/main/java/org/elasticsearch/transport/netty4/Netty4Transport.java @@ -149,25 +149,23 @@ private Bootstrap createClientBootstrap(SharedGroupFactory.SharedGroup sharedGro bootstrap.option(ChannelOption.TCP_NODELAY, TransportSettings.TCP_NO_DELAY.get(settings)); bootstrap.option(ChannelOption.SO_KEEPALIVE, TransportSettings.TCP_KEEP_ALIVE.get(settings)); if (TransportSettings.TCP_KEEP_ALIVE.get(settings)) { - // Netty logs a warning if it can't set the option, so try this only on supported platforms - if (IOUtils.LINUX || IOUtils.MAC_OS_X) { - if (TransportSettings.TCP_KEEP_IDLE.get(settings) >= 0) { - final SocketOption keepIdleOption = NetUtils.getTcpKeepIdleSocketOptionOrNull(); - if (keepIdleOption != null) { - bootstrap.option(NioChannelOption.of(keepIdleOption), TransportSettings.TCP_KEEP_IDLE.get(settings)); - } + // Note that Netty logs a warning if it can't set the option + if (TransportSettings.TCP_KEEP_IDLE.get(settings) >= 0) { + final SocketOption keepIdleOption = NetUtils.getTcpKeepIdleSocketOptionOrNull(); + if (keepIdleOption != null) { + bootstrap.option(NioChannelOption.of(keepIdleOption), TransportSettings.TCP_KEEP_IDLE.get(settings)); } - if (TransportSettings.TCP_KEEP_INTERVAL.get(settings) >= 0) { - final SocketOption keepIntervalOption = NetUtils.getTcpKeepIntervalSocketOptionOrNull(); - if (keepIntervalOption != null) { - bootstrap.option(NioChannelOption.of(keepIntervalOption), TransportSettings.TCP_KEEP_INTERVAL.get(settings)); - } + } + if (TransportSettings.TCP_KEEP_INTERVAL.get(settings) >= 0) { + final SocketOption keepIntervalOption = NetUtils.getTcpKeepIntervalSocketOptionOrNull(); + if (keepIntervalOption != null) { + bootstrap.option(NioChannelOption.of(keepIntervalOption), TransportSettings.TCP_KEEP_INTERVAL.get(settings)); } - if (TransportSettings.TCP_KEEP_COUNT.get(settings) >= 0) { - final SocketOption keepCountOption = NetUtils.getTcpKeepCountSocketOptionOrNull(); - if (keepCountOption != null) { - bootstrap.option(NioChannelOption.of(keepCountOption), TransportSettings.TCP_KEEP_COUNT.get(settings)); - } + } + if (TransportSettings.TCP_KEEP_COUNT.get(settings) >= 0) { + final SocketOption keepCountOption = NetUtils.getTcpKeepCountSocketOptionOrNull(); + if (keepCountOption != null) { + bootstrap.option(NioChannelOption.of(keepCountOption), TransportSettings.TCP_KEEP_COUNT.get(settings)); } } } diff --git a/server/src/main/java/org/elasticsearch/common/network/NetworkService.java b/server/src/main/java/org/elasticsearch/common/network/NetworkService.java index 9de04f71271fe..908a71348a8a4 100644 --- a/server/src/main/java/org/elasticsearch/common/network/NetworkService.java +++ b/server/src/main/java/org/elasticsearch/common/network/NetworkService.java @@ -19,9 +19,12 @@ package org.elasticsearch.common.network; +import org.elasticsearch.bootstrap.JavaVersion; +import org.elasticsearch.common.Booleans; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Setting.Property; import org.elasticsearch.common.unit.ByteSizeValue; +import org.elasticsearch.core.internal.io.IOUtils; import java.io.IOException; import java.net.InetAddress; @@ -50,12 +53,16 @@ public final class NetworkService { Setting.boolSetting("network.tcp.no_delay", true, Property.NodeScope); public static final Setting TCP_KEEP_ALIVE = Setting.boolSetting("network.tcp.keep_alive", true, Property.NodeScope); + public static final boolean SET_EXTRA_KEEP_ALIVE_OPTIONS = + Booleans.parseBoolean(System.getProperty("es.network.tcp.extra_keep_alive_options", "false")) == false && + (IOUtils.LINUX || IOUtils.MAC_OS_X) && + JavaVersion.current().compareTo(JavaVersion.parse("11")) >= 0; public static final Setting TCP_KEEP_IDLE = - Setting.intSetting("network.tcp.keep_idle", -1, -1, Property.NodeScope); + Setting.intSetting("network.tcp.keep_idle", SET_EXTRA_KEEP_ALIVE_OPTIONS ? 60 : -1, -1, Property.NodeScope); public static final Setting TCP_KEEP_INTERVAL = - Setting.intSetting("network.tcp.keep_interval", -1, -1, Property.NodeScope); + Setting.intSetting("network.tcp.keep_interval", SET_EXTRA_KEEP_ALIVE_OPTIONS ? 10 : -1, -1, Property.NodeScope); public static final Setting TCP_KEEP_COUNT = - Setting.intSetting("network.tcp.keep_count", -1, -1, Property.NodeScope); + Setting.intSetting("network.tcp.keep_count", SET_EXTRA_KEEP_ALIVE_OPTIONS ? 3 : -1, -1, Property.NodeScope); public static final Setting TCP_REUSE_ADDRESS = Setting.boolSetting("network.tcp.reuse_address", NetworkUtils.defaultReuseAddress(), Property.NodeScope); public static final Setting TCP_SEND_BUFFER_SIZE = diff --git a/server/src/main/java/org/elasticsearch/transport/TransportSettings.java b/server/src/main/java/org/elasticsearch/transport/TransportSettings.java index 506fb399962fd..b9d3fd0f0cb22 100644 --- a/server/src/main/java/org/elasticsearch/transport/TransportSettings.java +++ b/server/src/main/java/org/elasticsearch/transport/TransportSettings.java @@ -18,11 +18,13 @@ */ package org.elasticsearch.transport; +import org.elasticsearch.bootstrap.JavaVersion; import org.elasticsearch.common.network.NetworkService; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.ByteSizeValue; import org.elasticsearch.common.unit.TimeValue; +import org.elasticsearch.core.internal.io.IOUtils; import java.util.Arrays; import java.util.List; @@ -30,6 +32,7 @@ import java.util.function.Function; import static java.util.Collections.emptyList; +import static org.elasticsearch.common.network.NetworkService.SET_EXTRA_KEEP_ALIVE_OPTIONS; import static org.elasticsearch.common.settings.Setting.affixKeySetting; import static org.elasticsearch.common.settings.Setting.boolSetting; import static org.elasticsearch.common.settings.Setting.intSetting; @@ -81,20 +84,20 @@ public final class TransportSettings { affixKeySetting("transport.profiles.", "tcp.keep_alive", key -> boolSetting(key, TCP_KEEP_ALIVE, Setting.Property.NodeScope)); public static final Setting TCP_KEEP_IDLE = - intSetting("transport.tcp.keep_idle", NetworkService.TCP_KEEP_IDLE, -1, Setting.Property.NodeScope); + intSetting("transport.tcp.keep_idle", NetworkService.TCP_KEEP_IDLE, SET_EXTRA_KEEP_ALIVE_OPTIONS ? 60 : -1, Setting.Property.NodeScope); public static final Setting.AffixSetting TCP_KEEP_IDLE_PROFILE = affixKeySetting("transport.profiles.", "tcp.keep_idle", - key -> intSetting(key, TCP_KEEP_IDLE, -1, Setting.Property.NodeScope)); + key -> intSetting(key, TCP_KEEP_IDLE, SET_EXTRA_KEEP_ALIVE_OPTIONS ? 60 : -1, Setting.Property.NodeScope)); public static final Setting TCP_KEEP_INTERVAL = - intSetting("transport.tcp.keep_interval", NetworkService.TCP_KEEP_INTERVAL, -1, Setting.Property.NodeScope); + intSetting("transport.tcp.keep_interval", NetworkService.TCP_KEEP_INTERVAL, SET_EXTRA_KEEP_ALIVE_OPTIONS ? 10 : -1, Setting.Property.NodeScope); public static final Setting.AffixSetting TCP_KEEP_INTERVAL_PROFILE = affixKeySetting("transport.profiles.", "tcp.keep_interval", - key -> intSetting(key, TCP_KEEP_INTERVAL, -1, Setting.Property.NodeScope)); + key -> intSetting(key, TCP_KEEP_INTERVAL, SET_EXTRA_KEEP_ALIVE_OPTIONS ? 10 : -1, Setting.Property.NodeScope)); public static final Setting TCP_KEEP_COUNT = - intSetting("transport.tcp.keep_count", NetworkService.TCP_KEEP_COUNT, -1, Setting.Property.NodeScope); + intSetting("transport.tcp.keep_count", NetworkService.TCP_KEEP_COUNT, SET_EXTRA_KEEP_ALIVE_OPTIONS ? 3 : -1, Setting.Property.NodeScope); public static final Setting.AffixSetting TCP_KEEP_COUNT_PROFILE = affixKeySetting("transport.profiles.", "tcp.keep_count", - key -> intSetting(key, TCP_KEEP_COUNT, -1, Setting.Property.NodeScope)); + key -> intSetting(key, TCP_KEEP_COUNT, SET_EXTRA_KEEP_ALIVE_OPTIONS ? 3 : -1, Setting.Property.NodeScope)); public static final Setting TCP_REUSE_ADDRESS = boolSetting("transport.tcp.reuse_address", NetworkService.TCP_REUSE_ADDRESS, Setting.Property.NodeScope); public static final Setting.AffixSetting TCP_REUSE_ADDRESS_PROFILE = From e3aa4c0c43a1a57d780671d49dc9eb84815818c9 Mon Sep 17 00:00:00 2001 From: Yannick Welsch Date: Thu, 9 Jul 2020 10:23:10 +0200 Subject: [PATCH 02/15] checkstyle --- .../elasticsearch/transport/TransportSettings.java | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/transport/TransportSettings.java b/server/src/main/java/org/elasticsearch/transport/TransportSettings.java index b9d3fd0f0cb22..d6e0a29d60eac 100644 --- a/server/src/main/java/org/elasticsearch/transport/TransportSettings.java +++ b/server/src/main/java/org/elasticsearch/transport/TransportSettings.java @@ -18,13 +18,11 @@ */ package org.elasticsearch.transport; -import org.elasticsearch.bootstrap.JavaVersion; import org.elasticsearch.common.network.NetworkService; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.ByteSizeValue; import org.elasticsearch.common.unit.TimeValue; -import org.elasticsearch.core.internal.io.IOUtils; import java.util.Arrays; import java.util.List; @@ -84,17 +82,20 @@ public final class TransportSettings { affixKeySetting("transport.profiles.", "tcp.keep_alive", key -> boolSetting(key, TCP_KEEP_ALIVE, Setting.Property.NodeScope)); public static final Setting TCP_KEEP_IDLE = - intSetting("transport.tcp.keep_idle", NetworkService.TCP_KEEP_IDLE, SET_EXTRA_KEEP_ALIVE_OPTIONS ? 60 : -1, Setting.Property.NodeScope); + intSetting("transport.tcp.keep_idle", NetworkService.TCP_KEEP_IDLE, + SET_EXTRA_KEEP_ALIVE_OPTIONS ? 60 : -1, Setting.Property.NodeScope); public static final Setting.AffixSetting TCP_KEEP_IDLE_PROFILE = affixKeySetting("transport.profiles.", "tcp.keep_idle", key -> intSetting(key, TCP_KEEP_IDLE, SET_EXTRA_KEEP_ALIVE_OPTIONS ? 60 : -1, Setting.Property.NodeScope)); public static final Setting TCP_KEEP_INTERVAL = - intSetting("transport.tcp.keep_interval", NetworkService.TCP_KEEP_INTERVAL, SET_EXTRA_KEEP_ALIVE_OPTIONS ? 10 : -1, Setting.Property.NodeScope); + intSetting("transport.tcp.keep_interval", NetworkService.TCP_KEEP_INTERVAL, + SET_EXTRA_KEEP_ALIVE_OPTIONS ? 10 : -1, Setting.Property.NodeScope); public static final Setting.AffixSetting TCP_KEEP_INTERVAL_PROFILE = affixKeySetting("transport.profiles.", "tcp.keep_interval", key -> intSetting(key, TCP_KEEP_INTERVAL, SET_EXTRA_KEEP_ALIVE_OPTIONS ? 10 : -1, Setting.Property.NodeScope)); public static final Setting TCP_KEEP_COUNT = - intSetting("transport.tcp.keep_count", NetworkService.TCP_KEEP_COUNT, SET_EXTRA_KEEP_ALIVE_OPTIONS ? 3 : -1, Setting.Property.NodeScope); + intSetting("transport.tcp.keep_count", NetworkService.TCP_KEEP_COUNT, + SET_EXTRA_KEEP_ALIVE_OPTIONS ? 3 : -1, Setting.Property.NodeScope); public static final Setting.AffixSetting TCP_KEEP_COUNT_PROFILE = affixKeySetting("transport.profiles.", "tcp.keep_count", key -> intSetting(key, TCP_KEEP_COUNT, SET_EXTRA_KEEP_ALIVE_OPTIONS ? 3 : -1, Setting.Property.NodeScope)); From 05c860643e2958aa9ed00bb2ce8752b576af4afd Mon Sep 17 00:00:00 2001 From: Yannick Welsch Date: Thu, 9 Jul 2020 11:03:24 +0200 Subject: [PATCH 03/15] Oops --- .../transport/TransportSettings.java | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/transport/TransportSettings.java b/server/src/main/java/org/elasticsearch/transport/TransportSettings.java index d6e0a29d60eac..506fb399962fd 100644 --- a/server/src/main/java/org/elasticsearch/transport/TransportSettings.java +++ b/server/src/main/java/org/elasticsearch/transport/TransportSettings.java @@ -30,7 +30,6 @@ import java.util.function.Function; import static java.util.Collections.emptyList; -import static org.elasticsearch.common.network.NetworkService.SET_EXTRA_KEEP_ALIVE_OPTIONS; import static org.elasticsearch.common.settings.Setting.affixKeySetting; import static org.elasticsearch.common.settings.Setting.boolSetting; import static org.elasticsearch.common.settings.Setting.intSetting; @@ -82,23 +81,20 @@ public final class TransportSettings { affixKeySetting("transport.profiles.", "tcp.keep_alive", key -> boolSetting(key, TCP_KEEP_ALIVE, Setting.Property.NodeScope)); public static final Setting TCP_KEEP_IDLE = - intSetting("transport.tcp.keep_idle", NetworkService.TCP_KEEP_IDLE, - SET_EXTRA_KEEP_ALIVE_OPTIONS ? 60 : -1, Setting.Property.NodeScope); + intSetting("transport.tcp.keep_idle", NetworkService.TCP_KEEP_IDLE, -1, Setting.Property.NodeScope); public static final Setting.AffixSetting TCP_KEEP_IDLE_PROFILE = affixKeySetting("transport.profiles.", "tcp.keep_idle", - key -> intSetting(key, TCP_KEEP_IDLE, SET_EXTRA_KEEP_ALIVE_OPTIONS ? 60 : -1, Setting.Property.NodeScope)); + key -> intSetting(key, TCP_KEEP_IDLE, -1, Setting.Property.NodeScope)); public static final Setting TCP_KEEP_INTERVAL = - intSetting("transport.tcp.keep_interval", NetworkService.TCP_KEEP_INTERVAL, - SET_EXTRA_KEEP_ALIVE_OPTIONS ? 10 : -1, Setting.Property.NodeScope); + intSetting("transport.tcp.keep_interval", NetworkService.TCP_KEEP_INTERVAL, -1, Setting.Property.NodeScope); public static final Setting.AffixSetting TCP_KEEP_INTERVAL_PROFILE = affixKeySetting("transport.profiles.", "tcp.keep_interval", - key -> intSetting(key, TCP_KEEP_INTERVAL, SET_EXTRA_KEEP_ALIVE_OPTIONS ? 10 : -1, Setting.Property.NodeScope)); + key -> intSetting(key, TCP_KEEP_INTERVAL, -1, Setting.Property.NodeScope)); public static final Setting TCP_KEEP_COUNT = - intSetting("transport.tcp.keep_count", NetworkService.TCP_KEEP_COUNT, - SET_EXTRA_KEEP_ALIVE_OPTIONS ? 3 : -1, Setting.Property.NodeScope); + intSetting("transport.tcp.keep_count", NetworkService.TCP_KEEP_COUNT, -1, Setting.Property.NodeScope); public static final Setting.AffixSetting TCP_KEEP_COUNT_PROFILE = affixKeySetting("transport.profiles.", "tcp.keep_count", - key -> intSetting(key, TCP_KEEP_COUNT, SET_EXTRA_KEEP_ALIVE_OPTIONS ? 3 : -1, Setting.Property.NodeScope)); + key -> intSetting(key, TCP_KEEP_COUNT, -1, Setting.Property.NodeScope)); public static final Setting TCP_REUSE_ADDRESS = boolSetting("transport.tcp.reuse_address", NetworkService.TCP_REUSE_ADDRESS, Setting.Property.NodeScope); public static final Setting.AffixSetting TCP_REUSE_ADDRESS_PROFILE = From 5239fe95b753382aa4e3b90c01d439e7e30cd394 Mon Sep 17 00:00:00 2001 From: Yannick Welsch Date: Thu, 16 Jul 2020 11:37:35 +0200 Subject: [PATCH 04/15] revise docs --- docs/reference/modules/transport.asciidoc | 26 +++++++++++------------ 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/docs/reference/modules/transport.asciidoc b/docs/reference/modules/transport.asciidoc index d5a22c2f59d71..75e8894ec7169 100644 --- a/docs/reference/modules/transport.asciidoc +++ b/docs/reference/modules/transport.asciidoc @@ -86,24 +86,22 @@ example above: * `tcp.keep_alive`: Configures the `SO_KEEPALIVE` option for this socket * `tcp.keep_idle`: Configures the `TCP_KEEPIDLE` option for this socket, which determines the time in seconds that a connection must be idle before - starting to send TCP keepalive probes. - Only applicable on Linux and Mac, and requires JDK 11 or newer. - Defaults to 60 on applicable configurations, and -1 otherwise, which does - not set this option at the socket level, but uses the default system - configuration instead. + starting to send TCP keepalive probes. A value of `-1` means not to set + this option at the socket level but to use the system default instead. + Only applicable on Linux and Mac, and requires Java 11 or newer. + Defaults to 60 on applicable configurations, and -1 otherwise. * `tcp.keep_interval`: Configures the `TCP_KEEPINTVL` option for this socket, which determines the time in seconds between sending TCP keepalive probes. - Only applicable on Linux and Mac, and requires JDK 11 or newer. - Defaults to 10 on applicable configurations, and -1 otherwise, which does - not set this option at the socket level, but uses the default system - configuration instead. + A value of `-1` means not to set this option at the socket level but to + use the system default instead. + Only applicable on Linux and Mac, and requires Java 11 or newer. + Defaults to 10 on applicable configurations, and -1 otherwise. * `tcp.keep_count`: Configures the `TCP_KEEPCNT` option for this socket, which determines the number of unacknowledged TCP keepalive probes that may be - sent on a connection before it is dropped. - Only applicable on Linux and Mac, and requires JDK 11 or newer. - Defaults to 3 on applicable configurations, and -1 otherwise, which does - not set this option at the socket level, but uses the default system - configuration instead. + sent on a connection before it is dropped. A value of `-1` means not to set + this option at the socket level but to use the system default instead. + Only applicable on Linux and Mac, and requires Java 11 or newer. + Defaults to 3 on applicable configurations, and -1 otherwise. * `tcp.reuse_address`: Configures the `SO_REUSEADDR` option for this socket * `tcp.send_buffer_size`: Configures the send buffer size of the socket * `tcp.receive_buffer_size`: Configures the receive buffer size of the socket From b1b3bbc7ceb2ac84a2c87c04da06d5af2642ffd4 Mon Sep 17 00:00:00 2001 From: Yannick Welsch Date: Fri, 17 Jul 2020 15:28:12 +0200 Subject: [PATCH 05/15] align code --- .../transport/netty4/Netty4Transport.java | 34 +++++++++---------- 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/modules/transport-netty4/src/main/java/org/elasticsearch/transport/netty4/Netty4Transport.java b/modules/transport-netty4/src/main/java/org/elasticsearch/transport/netty4/Netty4Transport.java index 6d02163296268..060f2265ce749 100644 --- a/modules/transport-netty4/src/main/java/org/elasticsearch/transport/netty4/Netty4Transport.java +++ b/modules/transport-netty4/src/main/java/org/elasticsearch/transport/netty4/Netty4Transport.java @@ -213,26 +213,24 @@ private void createServerBootstrap(ProfileSettings profileSettings, SharedGroupF serverBootstrap.childOption(ChannelOption.TCP_NODELAY, profileSettings.tcpNoDelay); serverBootstrap.childOption(ChannelOption.SO_KEEPALIVE, profileSettings.tcpKeepAlive); if (profileSettings.tcpKeepAlive) { - // Netty logs a warning if it can't set the option, so try this only on supported platforms - if (IOUtils.LINUX || IOUtils.MAC_OS_X) { - if (profileSettings.tcpKeepIdle >= 0) { - final SocketOption keepIdleOption = NetUtils.getTcpKeepIdleSocketOptionOrNull(); - if (keepIdleOption != null) { - serverBootstrap.childOption(NioChannelOption.of(keepIdleOption), profileSettings.tcpKeepIdle); - } + // Note that Netty logs a warning if it can't set the option + if (profileSettings.tcpKeepIdle >= 0) { + final SocketOption keepIdleOption = NetUtils.getTcpKeepIdleSocketOptionOrNull(); + if (keepIdleOption != null) { + serverBootstrap.childOption(NioChannelOption.of(keepIdleOption), profileSettings.tcpKeepIdle); } - if (profileSettings.tcpKeepInterval >= 0) { - final SocketOption keepIntervalOption = NetUtils.getTcpKeepIntervalSocketOptionOrNull(); - if (keepIntervalOption != null) { - serverBootstrap.childOption(NioChannelOption.of(keepIntervalOption), profileSettings.tcpKeepInterval); - } - + } + if (profileSettings.tcpKeepInterval >= 0) { + final SocketOption keepIntervalOption = NetUtils.getTcpKeepIntervalSocketOptionOrNull(); + if (keepIntervalOption != null) { + serverBootstrap.childOption(NioChannelOption.of(keepIntervalOption), profileSettings.tcpKeepInterval); } - if (profileSettings.tcpKeepCount >= 0) { - final SocketOption keepCountOption = NetUtils.getTcpKeepCountSocketOptionOrNull(); - if (keepCountOption != null) { - serverBootstrap.childOption(NioChannelOption.of(keepCountOption), profileSettings.tcpKeepCount); - } + + } + if (profileSettings.tcpKeepCount >= 0) { + final SocketOption keepCountOption = NetUtils.getTcpKeepCountSocketOptionOrNull(); + if (keepCountOption != null) { + serverBootstrap.childOption(NioChannelOption.of(keepCountOption), profileSettings.tcpKeepCount); } } } From 29b18d53598806585c6d6a0171551669d295e5d8 Mon Sep 17 00:00:00 2001 From: Yannick Welsch Date: Sat, 18 Jul 2020 22:13:01 +0200 Subject: [PATCH 06/15] New approach --- .../rest-api-spec/test/20_settings.yml | 9 --- docs/reference/modules/transport.asciidoc | 8 ++- .../core/internal/net/NetUtils.java | 46 +++++++++++++++ .../nio/SocketChannelContext.java | 1 + .../transport/CopyBytesSocketChannel.java | 2 +- .../transport/Netty4NioSocketChannel.java | 45 ++++++++++++++ .../transport/NettyAllocator.java | 2 +- .../transport/netty4/Netty4Transport.java | 15 ++++- .../netty4/SimpleNetty4TransportTests.java | 59 +++++++++++++++++++ .../nio/SimpleNioTransportTests.java | 54 +++++++++++++++++ .../common/network/NetworkService.java | 13 +--- 11 files changed, 227 insertions(+), 27 deletions(-) delete mode 100644 distribution/docker/src/test/resources/rest-api-spec/test/20_settings.yml create mode 100644 modules/transport-netty4/src/main/java/org/elasticsearch/transport/Netty4NioSocketChannel.java diff --git a/distribution/docker/src/test/resources/rest-api-spec/test/20_settings.yml b/distribution/docker/src/test/resources/rest-api-spec/test/20_settings.yml deleted file mode 100644 index 60dfb9f0c8e55..0000000000000 --- a/distribution/docker/src/test/resources/rest-api-spec/test/20_settings.yml +++ /dev/null @@ -1,9 +0,0 @@ ---- -"Default TCP keepalive settings": - - do: - cluster.get_settings: - include_defaults: true - - - match: { defaults.network.tcp.keep_idle: "60" } - - match: { defaults.network.tcp.keep_interval: "10" } - - match: { defaults.network.tcp.keep_count: "3" } diff --git a/docs/reference/modules/transport.asciidoc b/docs/reference/modules/transport.asciidoc index 75e8894ec7169..f11376002c698 100644 --- a/docs/reference/modules/transport.asciidoc +++ b/docs/reference/modules/transport.asciidoc @@ -89,19 +89,21 @@ example above: starting to send TCP keepalive probes. A value of `-1` means not to set this option at the socket level but to use the system default instead. Only applicable on Linux and Mac, and requires Java 11 or newer. - Defaults to 60 on applicable configurations, and -1 otherwise. + Defaults to -1. On applicable configurations, this value is set to 5 minutes + if the system default or user-configured setting is higher than that. * `tcp.keep_interval`: Configures the `TCP_KEEPINTVL` option for this socket, which determines the time in seconds between sending TCP keepalive probes. A value of `-1` means not to set this option at the socket level but to use the system default instead. Only applicable on Linux and Mac, and requires Java 11 or newer. - Defaults to 10 on applicable configurations, and -1 otherwise. + Defaults to -1. On applicable configurations, this value is set to 5 minutes + if the system default or user-configured setting is higher than that. * `tcp.keep_count`: Configures the `TCP_KEEPCNT` option for this socket, which determines the number of unacknowledged TCP keepalive probes that may be sent on a connection before it is dropped. A value of `-1` means not to set this option at the socket level but to use the system default instead. Only applicable on Linux and Mac, and requires Java 11 or newer. - Defaults to 3 on applicable configurations, and -1 otherwise. + Defaults to -1. * `tcp.reuse_address`: Configures the `SO_REUSEADDR` option for this socket * `tcp.send_buffer_size`: Configures the send buffer size of the socket * `tcp.receive_buffer_size`: Configures the receive buffer size of the socket diff --git a/libs/core/src/main/java/org/elasticsearch/core/internal/net/NetUtils.java b/libs/core/src/main/java/org/elasticsearch/core/internal/net/NetUtils.java index 3df1cc3c513c9..4a2bbcca2e45d 100644 --- a/libs/core/src/main/java/org/elasticsearch/core/internal/net/NetUtils.java +++ b/libs/core/src/main/java/org/elasticsearch/core/internal/net/NetUtils.java @@ -19,8 +19,13 @@ package org.elasticsearch.core.internal.net; +import java.io.IOException; import java.lang.reflect.Field; import java.net.SocketOption; +import java.net.StandardSocketOptions; +import java.nio.channels.ClosedChannelException; +import java.nio.channels.NetworkChannel; +import java.util.Arrays; /** * Utilities for network-related methods. @@ -59,4 +64,45 @@ private static SocketOption getExtendedSocketOptionOrNull(String fieldNam return null; } } + + /** + * If SO_KEEPALIVE is enabled (default), this method ensures sane default values for the extended socket options + * TCP_KEEPIDLE and TCP_KEEPINTERVAL. The default value for TCP_KEEPIDLE is system dependent, but is typically 2 hours. + * Such a high value can result in firewalls eagerly closing these connections. To tell any intermediate devices that + * the connection remains alive, we explicitly set these options to 5 minutes if the defaults are higher than that. + */ + public static void setSaneDefaultKeepAliveOptions(NetworkChannel socketChannel) { + assert socketChannel != null; + try { + if (socketChannel.supportedOptions().contains(StandardSocketOptions.SO_KEEPALIVE)) { + final Boolean keepalive = socketChannel.getOption(StandardSocketOptions.SO_KEEPALIVE); + if (keepalive != null && keepalive.booleanValue()) { + for (SocketOption option : Arrays.asList( + NetUtils.getTcpKeepIdleSocketOptionOrNull(), + NetUtils.getTcpKeepIntervalSocketOptionOrNull())) { + setMinValueForSocketOption(socketChannel, option, 300); + } + } + } + } catch (Exception e) { + assert e instanceof IOException || e instanceof ClosedChannelException : + "unexpected exception when setting channel option: " + e.getClass() + ": " + e.getMessage(); + } + } + + private static void setMinValueForSocketOption(NetworkChannel socketChannel, SocketOption option, int minValue) { + if (option != null && socketChannel.supportedOptions().contains(option)) { + try { + final Integer currentIdleVal = socketChannel.getOption(option); + if (currentIdleVal != null && currentIdleVal.intValue() > minValue) { + socketChannel.setOption(option, minValue); + } + } catch (Exception e) { + // Getting an exception here should be ok when concurrently closing the channel + // An UnsupportedOperationException or IllegalArgumentException, however, should not happen + assert e instanceof IOException || e instanceof ClosedChannelException : + "unexpected exception when setting channel option: " + e.getClass() + ": " + e.getMessage(); + } + } + } } diff --git a/libs/nio/src/main/java/org/elasticsearch/nio/SocketChannelContext.java b/libs/nio/src/main/java/org/elasticsearch/nio/SocketChannelContext.java index b62e7178bc17e..c640d0fc38bf9 100644 --- a/libs/nio/src/main/java/org/elasticsearch/nio/SocketChannelContext.java +++ b/libs/nio/src/main/java/org/elasticsearch/nio/SocketChannelContext.java @@ -350,6 +350,7 @@ private void configureSocket(Socket socket, boolean isConnectComplete) throws IO } } } + NetUtils.setSaneDefaultKeepAliveOptions(socket.getChannel()); socket.setTcpNoDelay(socketConfig.tcpNoDelay()); int tcpSendBufferSize = socketConfig.tcpSendBufferSize(); if (tcpSendBufferSize > 0) { diff --git a/modules/transport-netty4/src/main/java/org/elasticsearch/transport/CopyBytesSocketChannel.java b/modules/transport-netty4/src/main/java/org/elasticsearch/transport/CopyBytesSocketChannel.java index 711dcc095b517..09d3a93fb3149 100644 --- a/modules/transport-netty4/src/main/java/org/elasticsearch/transport/CopyBytesSocketChannel.java +++ b/modules/transport-netty4/src/main/java/org/elasticsearch/transport/CopyBytesSocketChannel.java @@ -59,7 +59,7 @@ * local buffer with a defined size. */ @SuppressForbidden(reason = "Channel#write") -public class CopyBytesSocketChannel extends NioSocketChannel { +public class CopyBytesSocketChannel extends Netty4NioSocketChannel { private static final int MAX_BYTES_PER_WRITE = StrictMath.toIntExact(ByteSizeValue.parseBytesSizeValue( System.getProperty("es.transport.buffer.size", "1m"), "es.transport.buffer.size").getBytes()); diff --git a/modules/transport-netty4/src/main/java/org/elasticsearch/transport/Netty4NioSocketChannel.java b/modules/transport-netty4/src/main/java/org/elasticsearch/transport/Netty4NioSocketChannel.java new file mode 100644 index 0000000000000..49e8330ee4b98 --- /dev/null +++ b/modules/transport-netty4/src/main/java/org/elasticsearch/transport/Netty4NioSocketChannel.java @@ -0,0 +1,45 @@ +/* + * 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.transport; + +import io.netty.channel.Channel; +import io.netty.channel.socket.nio.NioSocketChannel; + +import java.nio.channels.SocketChannel; + +/** + * Helper class to expose {@link #javaChannel()} method + */ +public class Netty4NioSocketChannel extends NioSocketChannel { + + public Netty4NioSocketChannel() { + super(); + } + + public Netty4NioSocketChannel(Channel parent, SocketChannel socket) { + super(parent, socket); + } + + @Override + public SocketChannel javaChannel() { + return super.javaChannel(); + } + +} diff --git a/modules/transport-netty4/src/main/java/org/elasticsearch/transport/NettyAllocator.java b/modules/transport-netty4/src/main/java/org/elasticsearch/transport/NettyAllocator.java index 0b9cee7024a00..a2f94408e5ebb 100644 --- a/modules/transport-netty4/src/main/java/org/elasticsearch/transport/NettyAllocator.java +++ b/modules/transport-netty4/src/main/java/org/elasticsearch/transport/NettyAllocator.java @@ -68,7 +68,7 @@ public static Class getChannelType() { if (ALLOCATOR instanceof NoDirectBuffers) { return CopyBytesSocketChannel.class; } else { - return NioSocketChannel.class; + return Netty4NioSocketChannel.class; } } diff --git a/modules/transport-netty4/src/main/java/org/elasticsearch/transport/netty4/Netty4Transport.java b/modules/transport-netty4/src/main/java/org/elasticsearch/transport/netty4/Netty4Transport.java index 060f2265ce749..4f24bc67575f7 100644 --- a/modules/transport-netty4/src/main/java/org/elasticsearch/transport/netty4/Netty4Transport.java +++ b/modules/transport-netty4/src/main/java/org/elasticsearch/transport/netty4/Netty4Transport.java @@ -49,12 +49,12 @@ import org.elasticsearch.common.unit.ByteSizeValue; import org.elasticsearch.common.util.PageCacheRecycler; import org.elasticsearch.common.util.concurrent.EsExecutors; -import org.elasticsearch.core.internal.io.IOUtils; import org.elasticsearch.core.internal.net.NetUtils; import org.elasticsearch.indices.breaker.CircuitBreakerService; import org.elasticsearch.threadpool.ThreadPool; -import org.elasticsearch.transport.SharedGroupFactory; +import org.elasticsearch.transport.Netty4NioSocketChannel; import org.elasticsearch.transport.NettyAllocator; +import org.elasticsearch.transport.SharedGroupFactory; import org.elasticsearch.transport.TcpTransport; import org.elasticsearch.transport.TransportSettings; @@ -143,6 +143,7 @@ private Bootstrap createClientBootstrap(SharedGroupFactory.SharedGroup sharedGro bootstrap.group(sharedGroup.getLowLevelGroup()); // NettyAllocator will return the channel type designed to work with the configured allocator + assert Netty4NioSocketChannel.class.isAssignableFrom(NettyAllocator.getChannelType()); bootstrap.channel(NettyAllocator.getChannelType()); bootstrap.option(ChannelOption.ALLOCATOR, NettyAllocator.getAllocator()); @@ -277,7 +278,6 @@ protected Netty4TcpChannel initiateChannel(DiscoveryNode node) throws IOExceptio ExceptionsHelper.maybeDieOnAnotherThread(connectFuture.cause()); throw new IOException(connectFuture.cause()); } - addClosedExceptionLogger(channel); Netty4TcpChannel nettyChannel = new Netty4TcpChannel(channel, false, "default", connectFuture); channel.attr(CHANNEL_KEY).set(nettyChannel); @@ -307,6 +307,11 @@ protected class ClientChannelInitializer extends ChannelInitializer { @Override protected void initChannel(Channel ch) throws Exception { + addClosedExceptionLogger(ch); + assert ch instanceof Netty4NioSocketChannel; + if (ch instanceof Netty4NioSocketChannel) { + NetUtils.setSaneDefaultKeepAliveOptions(((Netty4NioSocketChannel) ch).javaChannel()); + } ch.pipeline().addLast("logging", new ESLoggingHandler()); // using a dot as a prefix means this cannot come from any settings parsed ch.pipeline().addLast("dispatcher", new Netty4MessageChannelHandler(pageCacheRecycler, Netty4Transport.this)); @@ -330,6 +335,10 @@ protected ServerChannelInitializer(String name) { @Override protected void initChannel(Channel ch) throws Exception { addClosedExceptionLogger(ch); + assert ch instanceof Netty4NioSocketChannel; + if (ch instanceof Netty4NioSocketChannel) { + NetUtils.setSaneDefaultKeepAliveOptions(((Netty4NioSocketChannel) ch).javaChannel()); + } Netty4TcpChannel nettyTcpChannel = new Netty4TcpChannel(ch, true, name, ch.newSucceededFuture()); ch.attr(CHANNEL_KEY).set(nettyTcpChannel); ch.pipeline().addLast("logging", new ESLoggingHandler()); diff --git a/modules/transport-netty4/src/test/java/org/elasticsearch/transport/netty4/SimpleNetty4TransportTests.java b/modules/transport-netty4/src/test/java/org/elasticsearch/transport/netty4/SimpleNetty4TransportTests.java index 38b66fc2c1259..9df7cf5ca5eb6 100644 --- a/modules/transport-netty4/src/test/java/org/elasticsearch/transport/netty4/SimpleNetty4TransportTests.java +++ b/modules/transport-netty4/src/test/java/org/elasticsearch/transport/netty4/SimpleNetty4TransportTests.java @@ -21,6 +21,7 @@ import org.elasticsearch.Version; import org.elasticsearch.action.ActionListener; +import org.elasticsearch.bootstrap.JavaVersion; import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.common.io.stream.NamedWriteableRegistry; import org.elasticsearch.common.network.NetworkService; @@ -28,21 +29,33 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.transport.TransportAddress; import org.elasticsearch.common.util.PageCacheRecycler; +import org.elasticsearch.core.internal.io.IOUtils; +import org.elasticsearch.core.internal.net.NetUtils; import org.elasticsearch.indices.breaker.NoneCircuitBreakerService; +import org.elasticsearch.test.transport.MockTransportService; +import org.elasticsearch.test.transport.StubbableTransport; import org.elasticsearch.transport.AbstractSimpleTransportTestCase; import org.elasticsearch.transport.ConnectTransportException; import org.elasticsearch.transport.ConnectionProfile; +import org.elasticsearch.transport.Netty4NioSocketChannel; import org.elasticsearch.transport.SharedGroupFactory; import org.elasticsearch.transport.TcpChannel; +import org.elasticsearch.transport.TcpTransport; +import org.elasticsearch.transport.TestProfiles; import org.elasticsearch.transport.Transport; +import java.io.IOException; import java.net.InetAddress; import java.net.UnknownHostException; +import java.nio.channels.SocketChannel; import java.util.Collections; import static java.util.Collections.emptyMap; import static java.util.Collections.emptySet; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.hasItem; +import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.lessThanOrEqualTo; public class SimpleNetty4TransportTests extends AbstractSimpleTransportTestCase { @@ -75,4 +88,50 @@ public void testConnectException() throws UnknownHostException { assertThat(e.getMessage(), containsString("[127.0.0.1:9876]")); } } + + public void testDefaultKeepAliveSettings() throws IOException { + assumeTrue("setting default keepalive options not supported on this platform", + (IOUtils.LINUX || IOUtils.MAC_OS_X) && + JavaVersion.current().compareTo(JavaVersion.parse("11")) >= 0); + try (MockTransportService serviceC = buildService("TS_C", Version.CURRENT, Settings.EMPTY); + MockTransportService serviceD = buildService("TS_D", Version.CURRENT, Settings.EMPTY)) { + serviceC.start(); + serviceC.acceptIncomingRequests(); + serviceD.start(); + serviceD.acceptIncomingRequests(); + + try (Transport.Connection connection = openConnection(serviceC, serviceD.getLocalDiscoNode(), TestProfiles.LIGHT_PROFILE)) { + assertThat(connection, instanceOf(StubbableTransport.WrappedConnection.class)); + Transport.Connection conn = ((StubbableTransport.WrappedConnection) connection).getConnection(); + assertThat(conn, instanceOf(TcpTransport.NodeChannels.class)); + TcpTransport.NodeChannels nodeChannels = (TcpTransport.NodeChannels) conn; + for (TcpChannel channel : nodeChannels.getChannels()) { + assertFalse(channel.isServerChannel()); + checkDefaultKeepAliveOptions(channel); + } + + assertThat(serviceD.getOriginalTransport(), instanceOf(TcpTransport.class)); + for (TcpChannel channel : getAcceptedChannels((TcpTransport) serviceD.getOriginalTransport())) { + assertTrue(channel.isServerChannel()); + checkDefaultKeepAliveOptions(channel); + } + } + } + } + + private void checkDefaultKeepAliveOptions(TcpChannel channel) throws IOException { + assertThat(channel, instanceOf(Netty4TcpChannel.class)); + Netty4TcpChannel nettyChannel = (Netty4TcpChannel) channel; + assertThat(nettyChannel.getNettyChannel(), instanceOf(Netty4NioSocketChannel.class)); + Netty4NioSocketChannel netty4NioSocketChannel = (Netty4NioSocketChannel) nettyChannel.getNettyChannel(); + SocketChannel socketChannel = netty4NioSocketChannel.javaChannel(); + assertThat(socketChannel.supportedOptions(), hasItem(NetUtils.getTcpKeepIdleSocketOptionOrNull())); + Integer keepIdle = socketChannel.getOption(NetUtils.getTcpKeepIdleSocketOptionOrNull()); + assertNotNull(keepIdle); + assertThat(keepIdle, lessThanOrEqualTo(500)); + assertThat(socketChannel.supportedOptions(), hasItem(NetUtils.getTcpKeepIntervalSocketOptionOrNull())); + Integer keepInterval = socketChannel.getOption(NetUtils.getTcpKeepIntervalSocketOptionOrNull()); + assertNotNull(keepInterval); + assertThat(keepInterval, lessThanOrEqualTo(500)); + } } diff --git a/plugins/transport-nio/src/test/java/org/elasticsearch/transport/nio/SimpleNioTransportTests.java b/plugins/transport-nio/src/test/java/org/elasticsearch/transport/nio/SimpleNioTransportTests.java index 59ef6886326cf..f6215ec72e0ba 100644 --- a/plugins/transport-nio/src/test/java/org/elasticsearch/transport/nio/SimpleNioTransportTests.java +++ b/plugins/transport-nio/src/test/java/org/elasticsearch/transport/nio/SimpleNioTransportTests.java @@ -21,6 +21,7 @@ import org.elasticsearch.Version; import org.elasticsearch.action.ActionListener; +import org.elasticsearch.bootstrap.JavaVersion; import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.common.io.stream.NamedWriteableRegistry; import org.elasticsearch.common.network.NetworkService; @@ -28,22 +29,31 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.transport.TransportAddress; import org.elasticsearch.common.util.MockPageCacheRecycler; +import org.elasticsearch.core.internal.io.IOUtils; +import org.elasticsearch.core.internal.net.NetUtils; import org.elasticsearch.indices.breaker.NoneCircuitBreakerService; +import org.elasticsearch.test.transport.MockTransportService; +import org.elasticsearch.test.transport.StubbableTransport; import org.elasticsearch.transport.AbstractSimpleTransportTestCase; import org.elasticsearch.transport.ConnectTransportException; import org.elasticsearch.transport.ConnectionProfile; import org.elasticsearch.transport.TcpChannel; +import org.elasticsearch.transport.TcpTransport; +import org.elasticsearch.transport.TestProfiles; import org.elasticsearch.transport.Transport; import java.io.IOException; import java.net.InetAddress; import java.net.UnknownHostException; +import java.nio.channels.SocketChannel; import java.util.Collections; import static java.util.Collections.emptyMap; import static java.util.Collections.emptySet; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.hasItem; import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.lessThanOrEqualTo; public class SimpleNioTransportTests extends AbstractSimpleTransportTestCase { @@ -78,4 +88,48 @@ public void testConnectException() throws UnknownHostException { assertThat(cause, instanceOf(IOException.class)); } } + + public void testDefaultKeepAliveSettings() throws IOException { + assumeTrue("setting default keepalive options not supported on this platform", + (IOUtils.LINUX || IOUtils.MAC_OS_X) && + JavaVersion.current().compareTo(JavaVersion.parse("11")) >= 0); + try (MockTransportService serviceC = buildService("TS_C", Version.CURRENT, Settings.EMPTY); + MockTransportService serviceD = buildService("TS_D", Version.CURRENT, Settings.EMPTY)) { + serviceC.start(); + serviceC.acceptIncomingRequests(); + serviceD.start(); + serviceD.acceptIncomingRequests(); + + try (Transport.Connection connection = openConnection(serviceC, serviceD.getLocalDiscoNode(), TestProfiles.LIGHT_PROFILE)) { + assertThat(connection, instanceOf(StubbableTransport.WrappedConnection.class)); + Transport.Connection conn = ((StubbableTransport.WrappedConnection) connection).getConnection(); + assertThat(conn, instanceOf(TcpTransport.NodeChannels.class)); + TcpTransport.NodeChannels nodeChannels = (TcpTransport.NodeChannels) conn; + for (TcpChannel channel : nodeChannels.getChannels()) { + assertFalse(channel.isServerChannel()); + checkDefaultKeepAliveOptions(channel); + } + + assertThat(serviceD.getOriginalTransport(), instanceOf(TcpTransport.class)); + for (TcpChannel channel : getAcceptedChannels((TcpTransport) serviceD.getOriginalTransport())) { + assertTrue(channel.isServerChannel()); + checkDefaultKeepAliveOptions(channel); + } + } + } + } + + private void checkDefaultKeepAliveOptions(TcpChannel channel) throws IOException { + assertThat(channel, instanceOf(NioTcpChannel.class)); + NioTcpChannel nioChannel = (NioTcpChannel) channel; + SocketChannel socketChannel = nioChannel.getRawChannel(); + assertThat(socketChannel.supportedOptions(), hasItem(NetUtils.getTcpKeepIdleSocketOptionOrNull())); + Integer keepIdle = socketChannel.getOption(NetUtils.getTcpKeepIdleSocketOptionOrNull()); + assertNotNull(keepIdle); + assertThat(keepIdle, lessThanOrEqualTo(500)); + assertThat(socketChannel.supportedOptions(), hasItem(NetUtils.getTcpKeepIntervalSocketOptionOrNull())); + Integer keepInterval = socketChannel.getOption(NetUtils.getTcpKeepIntervalSocketOptionOrNull()); + assertNotNull(keepInterval); + assertThat(keepInterval, lessThanOrEqualTo(500)); + } } diff --git a/server/src/main/java/org/elasticsearch/common/network/NetworkService.java b/server/src/main/java/org/elasticsearch/common/network/NetworkService.java index 908a71348a8a4..9de04f71271fe 100644 --- a/server/src/main/java/org/elasticsearch/common/network/NetworkService.java +++ b/server/src/main/java/org/elasticsearch/common/network/NetworkService.java @@ -19,12 +19,9 @@ package org.elasticsearch.common.network; -import org.elasticsearch.bootstrap.JavaVersion; -import org.elasticsearch.common.Booleans; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Setting.Property; import org.elasticsearch.common.unit.ByteSizeValue; -import org.elasticsearch.core.internal.io.IOUtils; import java.io.IOException; import java.net.InetAddress; @@ -53,16 +50,12 @@ public final class NetworkService { Setting.boolSetting("network.tcp.no_delay", true, Property.NodeScope); public static final Setting TCP_KEEP_ALIVE = Setting.boolSetting("network.tcp.keep_alive", true, Property.NodeScope); - public static final boolean SET_EXTRA_KEEP_ALIVE_OPTIONS = - Booleans.parseBoolean(System.getProperty("es.network.tcp.extra_keep_alive_options", "false")) == false && - (IOUtils.LINUX || IOUtils.MAC_OS_X) && - JavaVersion.current().compareTo(JavaVersion.parse("11")) >= 0; public static final Setting TCP_KEEP_IDLE = - Setting.intSetting("network.tcp.keep_idle", SET_EXTRA_KEEP_ALIVE_OPTIONS ? 60 : -1, -1, Property.NodeScope); + Setting.intSetting("network.tcp.keep_idle", -1, -1, Property.NodeScope); public static final Setting TCP_KEEP_INTERVAL = - Setting.intSetting("network.tcp.keep_interval", SET_EXTRA_KEEP_ALIVE_OPTIONS ? 10 : -1, -1, Property.NodeScope); + Setting.intSetting("network.tcp.keep_interval", -1, -1, Property.NodeScope); public static final Setting TCP_KEEP_COUNT = - Setting.intSetting("network.tcp.keep_count", SET_EXTRA_KEEP_ALIVE_OPTIONS ? 3 : -1, -1, Property.NodeScope); + Setting.intSetting("network.tcp.keep_count", -1, -1, Property.NodeScope); public static final Setting TCP_REUSE_ADDRESS = Setting.boolSetting("network.tcp.reuse_address", NetworkUtils.defaultReuseAddress(), Property.NodeScope); public static final Setting TCP_SEND_BUFFER_SIZE = From 33aa5659d91fbb2d18448b8fe61b79816d9425b1 Mon Sep 17 00:00:00 2001 From: Yannick Welsch Date: Mon, 20 Jul 2020 11:06:24 +0200 Subject: [PATCH 07/15] fix tests --- .../test/java/org/elasticsearch/nio/EventHandlerTests.java | 4 +++- .../java/org/elasticsearch/nio/SocketChannelContextTests.java | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/libs/nio/src/test/java/org/elasticsearch/nio/EventHandlerTests.java b/libs/nio/src/test/java/org/elasticsearch/nio/EventHandlerTests.java index 49c78cac3ee84..c3f6fee9c353f 100644 --- a/libs/nio/src/test/java/org/elasticsearch/nio/EventHandlerTests.java +++ b/libs/nio/src/test/java/org/elasticsearch/nio/EventHandlerTests.java @@ -68,7 +68,9 @@ public void setUpHandler() throws IOException { SocketChannel rawChannel = mock(SocketChannel.class); when(rawChannel.finishConnect()).thenReturn(true); NioSocketChannel channel = new NioSocketChannel(rawChannel); - when(rawChannel.socket()).thenReturn(mock(Socket.class)); + Socket socket = mock(Socket.class); + when(rawChannel.socket()).thenReturn(socket); + when(socket.getChannel()).thenReturn(rawChannel); context = new DoNotRegisterSocketContext(channel, selector, channelExceptionHandler, readWriteHandler); channel.setContext(context); handler.handleRegistration(context); diff --git a/libs/nio/src/test/java/org/elasticsearch/nio/SocketChannelContextTests.java b/libs/nio/src/test/java/org/elasticsearch/nio/SocketChannelContextTests.java index d61184bdf5c31..8a44394533a48 100644 --- a/libs/nio/src/test/java/org/elasticsearch/nio/SocketChannelContextTests.java +++ b/libs/nio/src/test/java/org/elasticsearch/nio/SocketChannelContextTests.java @@ -85,6 +85,7 @@ public void setup() throws Exception { }); rawSocket = mock(Socket.class); when(rawChannel.socket()).thenReturn(rawSocket); + when(rawSocket.getChannel()).thenReturn(rawChannel); } public void testIOExceptionSetIfEncountered() throws IOException { From 625ffc0564946bf43e9da28e641f3509081e88c1 Mon Sep 17 00:00:00 2001 From: Yannick Welsch Date: Mon, 20 Jul 2020 12:07:43 +0200 Subject: [PATCH 08/15] oh noes, an unused import --- .../main/java/org/elasticsearch/transport/NettyAllocator.java | 1 - 1 file changed, 1 deletion(-) diff --git a/modules/transport-netty4/src/main/java/org/elasticsearch/transport/NettyAllocator.java b/modules/transport-netty4/src/main/java/org/elasticsearch/transport/NettyAllocator.java index a2f94408e5ebb..5d7edca5c2de7 100644 --- a/modules/transport-netty4/src/main/java/org/elasticsearch/transport/NettyAllocator.java +++ b/modules/transport-netty4/src/main/java/org/elasticsearch/transport/NettyAllocator.java @@ -27,7 +27,6 @@ import io.netty.channel.Channel; import io.netty.channel.ServerChannel; import io.netty.channel.socket.nio.NioServerSocketChannel; -import io.netty.channel.socket.nio.NioSocketChannel; import org.elasticsearch.common.Booleans; import org.elasticsearch.monitor.jvm.JvmInfo; From d840b2d6e75f340b7b0d2e4bd3ccde65d60e0e18 Mon Sep 17 00:00:00 2001 From: Yannick Welsch Date: Wed, 22 Jul 2020 16:41:51 +0200 Subject: [PATCH 09/15] reject values above 5 minutes --- docs/reference/modules/transport.asciidoc | 10 ++++++---- .../elasticsearch/common/network/NetworkService.java | 4 ++-- .../transport/AbstractSimpleTransportTestCase.java | 4 ++-- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/docs/reference/modules/transport.asciidoc b/docs/reference/modules/transport.asciidoc index f11376002c698..e2cd92a9bdc2f 100644 --- a/docs/reference/modules/transport.asciidoc +++ b/docs/reference/modules/transport.asciidoc @@ -89,15 +89,17 @@ example above: starting to send TCP keepalive probes. A value of `-1` means not to set this option at the socket level but to use the system default instead. Only applicable on Linux and Mac, and requires Java 11 or newer. - Defaults to -1. On applicable configurations, this value is set to 5 minutes - if the system default or user-configured setting is higher than that. + Defaults to -1. On applicable configurations, this value is set to + 300 seconds (5 minutes) if the system default or user-configured setting + is higher than that. Values above 300 seconds are rejected for this setting. * `tcp.keep_interval`: Configures the `TCP_KEEPINTVL` option for this socket, which determines the time in seconds between sending TCP keepalive probes. A value of `-1` means not to set this option at the socket level but to use the system default instead. Only applicable on Linux and Mac, and requires Java 11 or newer. - Defaults to -1. On applicable configurations, this value is set to 5 minutes - if the system default or user-configured setting is higher than that. + Defaults to -1. On applicable configurations, this value is set to + 300 seconds (5 minutes) if the system default or user-configured setting + is higher than that. Values above 300 seconds are rejected for this setting. * `tcp.keep_count`: Configures the `TCP_KEEPCNT` option for this socket, which determines the number of unacknowledged TCP keepalive probes that may be sent on a connection before it is dropped. A value of `-1` means not to set diff --git a/server/src/main/java/org/elasticsearch/common/network/NetworkService.java b/server/src/main/java/org/elasticsearch/common/network/NetworkService.java index 9de04f71271fe..210f2a518cd7d 100644 --- a/server/src/main/java/org/elasticsearch/common/network/NetworkService.java +++ b/server/src/main/java/org/elasticsearch/common/network/NetworkService.java @@ -51,9 +51,9 @@ public final class NetworkService { public static final Setting TCP_KEEP_ALIVE = Setting.boolSetting("network.tcp.keep_alive", true, Property.NodeScope); public static final Setting TCP_KEEP_IDLE = - Setting.intSetting("network.tcp.keep_idle", -1, -1, Property.NodeScope); + Setting.intSetting("network.tcp.keep_idle", -1, -1, 300, Property.NodeScope); public static final Setting TCP_KEEP_INTERVAL = - Setting.intSetting("network.tcp.keep_interval", -1, -1, Property.NodeScope); + Setting.intSetting("network.tcp.keep_interval", -1, -1, 300, Property.NodeScope); public static final Setting TCP_KEEP_COUNT = Setting.intSetting("network.tcp.keep_count", -1, -1, Property.NodeScope); public static final Setting TCP_REUSE_ADDRESS = diff --git a/test/framework/src/main/java/org/elasticsearch/transport/AbstractSimpleTransportTestCase.java b/test/framework/src/main/java/org/elasticsearch/transport/AbstractSimpleTransportTestCase.java index 778ce99071d1e..417e1667ddb71 100644 --- a/test/framework/src/main/java/org/elasticsearch/transport/AbstractSimpleTransportTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/transport/AbstractSimpleTransportTestCase.java @@ -147,10 +147,10 @@ public void setUp() throws Exception { connectionSettingsBuilder.put(TransportSettings.TCP_KEEP_ALIVE.getKey(), randomBoolean()); if (randomBoolean()) { - connectionSettingsBuilder.put(TransportSettings.TCP_KEEP_IDLE.getKey(), randomIntBetween(1, 1000)); + connectionSettingsBuilder.put(TransportSettings.TCP_KEEP_IDLE.getKey(), randomIntBetween(1, 300)); } if (randomBoolean()) { - connectionSettingsBuilder.put(TransportSettings.TCP_KEEP_INTERVAL.getKey(), randomIntBetween(1, 1000)); + connectionSettingsBuilder.put(TransportSettings.TCP_KEEP_INTERVAL.getKey(), randomIntBetween(1, 300)); } if (randomBoolean()) { connectionSettingsBuilder.put(TransportSettings.TCP_KEEP_COUNT.getKey(), randomIntBetween(1, 10)); From 785e35fbcab2dd8787906e6709fef66954efbe39 Mon Sep 17 00:00:00 2001 From: Yannick Welsch Date: Mon, 27 Jul 2020 11:40:58 +0200 Subject: [PATCH 10/15] rework docs --- docs/reference/modules/transport.asciidoc | 42 ++++++++++------------- 1 file changed, 18 insertions(+), 24 deletions(-) diff --git a/docs/reference/modules/transport.asciidoc b/docs/reference/modules/transport.asciidoc index e2cd92a9bdc2f..c33a2b7d9e71c 100644 --- a/docs/reference/modules/transport.asciidoc +++ b/docs/reference/modules/transport.asciidoc @@ -79,36 +79,30 @@ this node connects to other nodes in the cluster. The following parameters can be configured on each transport profile, as in the example above: -* `port`: The port to bind to -* `bind_host`: The host to bind -* `publish_host`: The host which is published in informational APIs -* `tcp.no_delay`: Configures the `TCP_NO_DELAY` option for this socket -* `tcp.keep_alive`: Configures the `SO_KEEPALIVE` option for this socket +* `port`: The port to which to bind. +* `bind_host`: The host to which to bind. +* `publish_host`: The host which is published in informational APIs. +* `tcp.no_delay`: Configures the `TCP_NO_DELAY` option for this socket. +* `tcp.keep_alive`: Configures the `SO_KEEPALIVE` option for this socket, which + determines whether it sends TCP keepalive probes. * `tcp.keep_idle`: Configures the `TCP_KEEPIDLE` option for this socket, which determines the time in seconds that a connection must be idle before - starting to send TCP keepalive probes. A value of `-1` means not to set - this option at the socket level but to use the system default instead. - Only applicable on Linux and Mac, and requires Java 11 or newer. - Defaults to -1. On applicable configurations, this value is set to - 300 seconds (5 minutes) if the system default or user-configured setting - is higher than that. Values above 300 seconds are rejected for this setting. + starting to send TCP keepalive probes. Defaults to `-1` which means to use + the smaller of 300 or the system default. May not be greater than 300. Only + applicable on Linux and macOS, and requires Java 11 or newer. * `tcp.keep_interval`: Configures the `TCP_KEEPINTVL` option for this socket, which determines the time in seconds between sending TCP keepalive probes. - A value of `-1` means not to set this option at the socket level but to - use the system default instead. - Only applicable on Linux and Mac, and requires Java 11 or newer. - Defaults to -1. On applicable configurations, this value is set to - 300 seconds (5 minutes) if the system default or user-configured setting - is higher than that. Values above 300 seconds are rejected for this setting. + Defaults to `-1` which means to use the smaller of 300 or the system + default. May not be greater than 300. Only applicable on Linux and macOS, + and requires Java 11 or newer. * `tcp.keep_count`: Configures the `TCP_KEEPCNT` option for this socket, which determines the number of unacknowledged TCP keepalive probes that may be - sent on a connection before it is dropped. A value of `-1` means not to set - this option at the socket level but to use the system default instead. - Only applicable on Linux and Mac, and requires Java 11 or newer. - Defaults to -1. -* `tcp.reuse_address`: Configures the `SO_REUSEADDR` option for this socket -* `tcp.send_buffer_size`: Configures the send buffer size of the socket -* `tcp.receive_buffer_size`: Configures the receive buffer size of the socket + sent on a connection before it is dropped. Defaults to `-1` which means to + use the system default. Only applicable on Linux and macOS, and requires + Java 11 or newer. +* `tcp.reuse_address`: Configures the `SO_REUSEADDR` option for this socket. +* `tcp.send_buffer_size`: Configures the send buffer size of the socket. +* `tcp.receive_buffer_size`: Configures the receive buffer size of the socket. [[long-lived-connections]] ===== Long-lived idle connections From c5c8faf08bcba2043919d2ba7507f548632cb02b Mon Sep 17 00:00:00 2001 From: Yannick Welsch Date: Mon, 27 Jul 2020 11:42:41 +0200 Subject: [PATCH 11/15] ClosedChannelException subclasses IOException --- .../java/org/elasticsearch/core/internal/net/NetUtils.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/libs/core/src/main/java/org/elasticsearch/core/internal/net/NetUtils.java b/libs/core/src/main/java/org/elasticsearch/core/internal/net/NetUtils.java index 4a2bbcca2e45d..2eafdd71a25dd 100644 --- a/libs/core/src/main/java/org/elasticsearch/core/internal/net/NetUtils.java +++ b/libs/core/src/main/java/org/elasticsearch/core/internal/net/NetUtils.java @@ -23,7 +23,6 @@ import java.lang.reflect.Field; import java.net.SocketOption; import java.net.StandardSocketOptions; -import java.nio.channels.ClosedChannelException; import java.nio.channels.NetworkChannel; import java.util.Arrays; @@ -85,7 +84,7 @@ public static void setSaneDefaultKeepAliveOptions(NetworkChannel socketChannel) } } } catch (Exception e) { - assert e instanceof IOException || e instanceof ClosedChannelException : + assert e instanceof IOException : "unexpected exception when setting channel option: " + e.getClass() + ": " + e.getMessage(); } } @@ -100,7 +99,7 @@ private static void setMinValueForSocketOption(NetworkChannel socketChannel, Soc } catch (Exception e) { // Getting an exception here should be ok when concurrently closing the channel // An UnsupportedOperationException or IllegalArgumentException, however, should not happen - assert e instanceof IOException || e instanceof ClosedChannelException : + assert e instanceof IOException : "unexpected exception when setting channel option: " + e.getClass() + ": " + e.getMessage(); } } From c9a582ce7fe7340392f7457a98cbcc1e7c30511a Mon Sep 17 00:00:00 2001 From: Yannick Welsch Date: Mon, 27 Jul 2020 11:45:03 +0200 Subject: [PATCH 12/15] shorter assertion --- .../org/elasticsearch/core/internal/net/NetUtils.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/libs/core/src/main/java/org/elasticsearch/core/internal/net/NetUtils.java b/libs/core/src/main/java/org/elasticsearch/core/internal/net/NetUtils.java index 2eafdd71a25dd..05d3199c63adc 100644 --- a/libs/core/src/main/java/org/elasticsearch/core/internal/net/NetUtils.java +++ b/libs/core/src/main/java/org/elasticsearch/core/internal/net/NetUtils.java @@ -84,8 +84,9 @@ public static void setSaneDefaultKeepAliveOptions(NetworkChannel socketChannel) } } } catch (Exception e) { - assert e instanceof IOException : - "unexpected exception when setting channel option: " + e.getClass() + ": " + e.getMessage(); + // Getting an exception here should be ok when concurrently closing the channel + // An UnsupportedOperationException or IllegalArgumentException, however, should not happen + assert e instanceof IOException : e; } } @@ -99,8 +100,7 @@ private static void setMinValueForSocketOption(NetworkChannel socketChannel, Soc } catch (Exception e) { // Getting an exception here should be ok when concurrently closing the channel // An UnsupportedOperationException or IllegalArgumentException, however, should not happen - assert e instanceof IOException : - "unexpected exception when setting channel option: " + e.getClass() + ": " + e.getMessage(); + assert e instanceof IOException : e; } } } From 1dbe2391e6135ab28686fca959569b227631afe5 Mon Sep 17 00:00:00 2001 From: Yannick Welsch Date: Mon, 27 Jul 2020 11:45:53 +0200 Subject: [PATCH 13/15] Naming --- .../java/org/elasticsearch/core/internal/net/NetUtils.java | 2 +- .../main/java/org/elasticsearch/nio/SocketChannelContext.java | 2 +- .../org/elasticsearch/transport/netty4/Netty4Transport.java | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/libs/core/src/main/java/org/elasticsearch/core/internal/net/NetUtils.java b/libs/core/src/main/java/org/elasticsearch/core/internal/net/NetUtils.java index 05d3199c63adc..67fba54409ce9 100644 --- a/libs/core/src/main/java/org/elasticsearch/core/internal/net/NetUtils.java +++ b/libs/core/src/main/java/org/elasticsearch/core/internal/net/NetUtils.java @@ -70,7 +70,7 @@ private static SocketOption getExtendedSocketOptionOrNull(String fieldNam * Such a high value can result in firewalls eagerly closing these connections. To tell any intermediate devices that * the connection remains alive, we explicitly set these options to 5 minutes if the defaults are higher than that. */ - public static void setSaneDefaultKeepAliveOptions(NetworkChannel socketChannel) { + public static void tryEnsureReasonableKeepAliveConfig(NetworkChannel socketChannel) { assert socketChannel != null; try { if (socketChannel.supportedOptions().contains(StandardSocketOptions.SO_KEEPALIVE)) { diff --git a/libs/nio/src/main/java/org/elasticsearch/nio/SocketChannelContext.java b/libs/nio/src/main/java/org/elasticsearch/nio/SocketChannelContext.java index c640d0fc38bf9..abf968ca8a07f 100644 --- a/libs/nio/src/main/java/org/elasticsearch/nio/SocketChannelContext.java +++ b/libs/nio/src/main/java/org/elasticsearch/nio/SocketChannelContext.java @@ -350,7 +350,7 @@ private void configureSocket(Socket socket, boolean isConnectComplete) throws IO } } } - NetUtils.setSaneDefaultKeepAliveOptions(socket.getChannel()); + NetUtils.tryEnsureReasonableKeepAliveConfig(socket.getChannel()); socket.setTcpNoDelay(socketConfig.tcpNoDelay()); int tcpSendBufferSize = socketConfig.tcpSendBufferSize(); if (tcpSendBufferSize > 0) { diff --git a/modules/transport-netty4/src/main/java/org/elasticsearch/transport/netty4/Netty4Transport.java b/modules/transport-netty4/src/main/java/org/elasticsearch/transport/netty4/Netty4Transport.java index 4f24bc67575f7..d02203a979ecf 100644 --- a/modules/transport-netty4/src/main/java/org/elasticsearch/transport/netty4/Netty4Transport.java +++ b/modules/transport-netty4/src/main/java/org/elasticsearch/transport/netty4/Netty4Transport.java @@ -310,7 +310,7 @@ protected void initChannel(Channel ch) throws Exception { addClosedExceptionLogger(ch); assert ch instanceof Netty4NioSocketChannel; if (ch instanceof Netty4NioSocketChannel) { - NetUtils.setSaneDefaultKeepAliveOptions(((Netty4NioSocketChannel) ch).javaChannel()); + NetUtils.tryEnsureReasonableKeepAliveConfig(((Netty4NioSocketChannel) ch).javaChannel()); } ch.pipeline().addLast("logging", new ESLoggingHandler()); // using a dot as a prefix means this cannot come from any settings parsed @@ -337,7 +337,7 @@ protected void initChannel(Channel ch) throws Exception { addClosedExceptionLogger(ch); assert ch instanceof Netty4NioSocketChannel; if (ch instanceof Netty4NioSocketChannel) { - NetUtils.setSaneDefaultKeepAliveOptions(((Netty4NioSocketChannel) ch).javaChannel()); + NetUtils.tryEnsureReasonableKeepAliveConfig(((Netty4NioSocketChannel) ch).javaChannel()); } Netty4TcpChannel nettyTcpChannel = new Netty4TcpChannel(ch, true, name, ch.newSucceededFuture()); ch.attr(CHANNEL_KEY).set(nettyTcpChannel); From 380abcac93d99b88eb965705a9577406da6e6eb9 Mon Sep 17 00:00:00 2001 From: Yannick Welsch Date: Mon, 27 Jul 2020 11:49:06 +0200 Subject: [PATCH 14/15] The given socket options won't return null --- .../java/org/elasticsearch/core/internal/net/NetUtils.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/libs/core/src/main/java/org/elasticsearch/core/internal/net/NetUtils.java b/libs/core/src/main/java/org/elasticsearch/core/internal/net/NetUtils.java index 67fba54409ce9..9185e39fdab20 100644 --- a/libs/core/src/main/java/org/elasticsearch/core/internal/net/NetUtils.java +++ b/libs/core/src/main/java/org/elasticsearch/core/internal/net/NetUtils.java @@ -75,7 +75,8 @@ public static void tryEnsureReasonableKeepAliveConfig(NetworkChannel socketChann try { if (socketChannel.supportedOptions().contains(StandardSocketOptions.SO_KEEPALIVE)) { final Boolean keepalive = socketChannel.getOption(StandardSocketOptions.SO_KEEPALIVE); - if (keepalive != null && keepalive.booleanValue()) { + assert keepalive != null; + if (keepalive.booleanValue()) { for (SocketOption option : Arrays.asList( NetUtils.getTcpKeepIdleSocketOptionOrNull(), NetUtils.getTcpKeepIntervalSocketOptionOrNull())) { @@ -94,7 +95,8 @@ private static void setMinValueForSocketOption(NetworkChannel socketChannel, Soc if (option != null && socketChannel.supportedOptions().contains(option)) { try { final Integer currentIdleVal = socketChannel.getOption(option); - if (currentIdleVal != null && currentIdleVal.intValue() > minValue) { + assert currentIdleVal != null; + if (currentIdleVal.intValue() > minValue) { socketChannel.setOption(option, minValue); } } catch (Exception e) { From 57f49168fac2837f8b87df55f2efb14558072150 Mon Sep 17 00:00:00 2001 From: Yannick Welsch Date: Mon, 27 Jul 2020 11:52:53 +0200 Subject: [PATCH 15/15] don't be so defensive --- .../elasticsearch/transport/netty4/Netty4Transport.java | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/modules/transport-netty4/src/main/java/org/elasticsearch/transport/netty4/Netty4Transport.java b/modules/transport-netty4/src/main/java/org/elasticsearch/transport/netty4/Netty4Transport.java index d02203a979ecf..60095369bcfae 100644 --- a/modules/transport-netty4/src/main/java/org/elasticsearch/transport/netty4/Netty4Transport.java +++ b/modules/transport-netty4/src/main/java/org/elasticsearch/transport/netty4/Netty4Transport.java @@ -309,9 +309,7 @@ protected class ClientChannelInitializer extends ChannelInitializer { protected void initChannel(Channel ch) throws Exception { addClosedExceptionLogger(ch); assert ch instanceof Netty4NioSocketChannel; - if (ch instanceof Netty4NioSocketChannel) { - NetUtils.tryEnsureReasonableKeepAliveConfig(((Netty4NioSocketChannel) ch).javaChannel()); - } + NetUtils.tryEnsureReasonableKeepAliveConfig(((Netty4NioSocketChannel) ch).javaChannel()); ch.pipeline().addLast("logging", new ESLoggingHandler()); // using a dot as a prefix means this cannot come from any settings parsed ch.pipeline().addLast("dispatcher", new Netty4MessageChannelHandler(pageCacheRecycler, Netty4Transport.this)); @@ -336,9 +334,7 @@ protected ServerChannelInitializer(String name) { protected void initChannel(Channel ch) throws Exception { addClosedExceptionLogger(ch); assert ch instanceof Netty4NioSocketChannel; - if (ch instanceof Netty4NioSocketChannel) { - NetUtils.tryEnsureReasonableKeepAliveConfig(((Netty4NioSocketChannel) ch).javaChannel()); - } + NetUtils.tryEnsureReasonableKeepAliveConfig(((Netty4NioSocketChannel) ch).javaChannel()); Netty4TcpChannel nettyTcpChannel = new Netty4TcpChannel(ch, true, name, ch.newSucceededFuture()); ch.attr(CHANNEL_KEY).set(nettyTcpChannel); ch.pipeline().addLast("logging", new ESLoggingHandler());