From f2deb9a58db97c5ad0d9e432216b68d33901f336 Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Tue, 27 Nov 2018 13:37:03 -0700 Subject: [PATCH] Remove `TcpChannel#setSoLinger` method (#35924) (#35952) This commit removes the dedicated `setSoLinger` method. This simplifies the `TcpChannel` interface. This method has very little effect as the SO_LINGER is not set prior to the channels being closed in the abstract transport test case. --- .../transport/netty4/Netty4TcpChannel.java | 14 -------------- .../elasticsearch/transport/TcpChannel.java | 9 --------- .../elasticsearch/transport/TcpTransport.java | 18 ------------------ .../transport/TcpTransportTests.java | 4 ---- .../transport/MockTcpTransport.java | 7 ------- 5 files changed, 52 deletions(-) diff --git a/modules/transport-netty4/src/main/java/org/elasticsearch/transport/netty4/Netty4TcpChannel.java b/modules/transport-netty4/src/main/java/org/elasticsearch/transport/netty4/Netty4TcpChannel.java index 7080607b6853b..30a83eeed7efd 100644 --- a/modules/transport-netty4/src/main/java/org/elasticsearch/transport/netty4/Netty4TcpChannel.java +++ b/modules/transport-netty4/src/main/java/org/elasticsearch/transport/netty4/Netty4TcpChannel.java @@ -20,9 +20,7 @@ package org.elasticsearch.transport.netty4; import io.netty.channel.Channel; -import io.netty.channel.ChannelException; import io.netty.channel.ChannelFuture; -import io.netty.channel.ChannelOption; import io.netty.channel.ChannelPromise; import org.elasticsearch.ExceptionsHelper; import org.elasticsearch.action.ActionListener; @@ -32,7 +30,6 @@ import org.elasticsearch.transport.TcpChannel; import org.elasticsearch.transport.TransportException; -import java.io.IOException; import java.net.InetSocketAddress; public class Netty4TcpChannel implements TcpChannel { @@ -95,17 +92,6 @@ public void addConnectListener(ActionListener listener) { connectContext.addListener(ActionListener.toBiConsumer(listener)); } - @Override - public void setSoLinger(int value) throws IOException { - if (channel.isOpen()) { - try { - channel.config().setOption(ChannelOption.SO_LINGER, value); - } catch (ChannelException e) { - throw new IOException(e); - } - } - } - @Override public boolean isOpen() { return channel.isOpen(); diff --git a/server/src/main/java/org/elasticsearch/transport/TcpChannel.java b/server/src/main/java/org/elasticsearch/transport/TcpChannel.java index f4d265389d3d4..cfab9af06d54f 100644 --- a/server/src/main/java/org/elasticsearch/transport/TcpChannel.java +++ b/server/src/main/java/org/elasticsearch/transport/TcpChannel.java @@ -23,7 +23,6 @@ import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.network.CloseableChannel; -import java.io.IOException; import java.net.InetSocketAddress; @@ -39,14 +38,6 @@ public interface TcpChannel extends CloseableChannel { */ String getProfile(); - /** - * This sets the low level socket option {@link java.net.StandardSocketOptions} SO_LINGER on a channel. - * - * @param value to set for SO_LINGER - * @throws IOException that can be throw by the low level socket implementation - */ - void setSoLinger(int value) throws IOException; - /** * Returns the local address for this channel. * diff --git a/server/src/main/java/org/elasticsearch/transport/TcpTransport.java b/server/src/main/java/org/elasticsearch/transport/TcpTransport.java index a72251fae5a06..ef83498e7e5aa 100644 --- a/server/src/main/java/org/elasticsearch/transport/TcpTransport.java +++ b/server/src/main/java/org/elasticsearch/transport/TcpTransport.java @@ -348,24 +348,6 @@ protected void innerOnFailure(Exception e) { public void close() { if (isClosing.compareAndSet(false, true)) { try { - if (lifecycle.stopped()) { - /* We set SO_LINGER timeout to 0 to ensure that when we shutdown the node we don't - * have a gazillion connections sitting in TIME_WAIT to free up resources quickly. - * This is really the only part where we close the connection from the server side - * otherwise the client (node) initiates the TCP closing sequence which doesn't cause - * these issues. Setting this by default from the beginning can have unexpected - * side-effects an should be avoided, our protocol is designed in a way that clients - * close connection which is how it should be*/ - - channels.forEach(c -> { - try { - c.setSoLinger(0); - } catch (IOException e) { - logger.warn(new ParameterizedMessage("unexpected exception when setting SO_LINGER on channel {}", c), e); - } - }); - } - boolean block = lifecycle.stopped() && Transports.isTransportThread(Thread.currentThread()) == false; CloseableChannel.closeChannels(channels, block); } finally { diff --git a/server/src/test/java/org/elasticsearch/transport/TcpTransportTests.java b/server/src/test/java/org/elasticsearch/transport/TcpTransportTests.java index d1f8817750559..7ec5ebd10a54e 100644 --- a/server/src/test/java/org/elasticsearch/transport/TcpTransportTests.java +++ b/server/src/test/java/org/elasticsearch/transport/TcpTransportTests.java @@ -273,10 +273,6 @@ public void addCloseListener(ActionListener listener) { public void addConnectListener(ActionListener listener) { } - @Override - public void setSoLinger(int value) throws IOException { - } - @Override public boolean isOpen() { return false; diff --git a/test/framework/src/main/java/org/elasticsearch/transport/MockTcpTransport.java b/test/framework/src/main/java/org/elasticsearch/transport/MockTcpTransport.java index 11b16b67651aa..48358960c3f3a 100644 --- a/test/framework/src/main/java/org/elasticsearch/transport/MockTcpTransport.java +++ b/test/framework/src/main/java/org/elasticsearch/transport/MockTcpTransport.java @@ -399,13 +399,6 @@ public void addConnectListener(ActionListener listener) { connectFuture.addListener(ActionListener.toBiConsumer(listener)); } - @Override - public void setSoLinger(int value) throws IOException { - if (activeChannel != null && activeChannel.isClosed() == false) { - activeChannel.setSoLinger(true, value); - } - } - @Override public boolean isOpen() { return isOpen.get();