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.
elastic · Nov 27, 2018 · f2deb9a · f2deb9a
1 parent 5c5914b
commit f2deb9a
Show file tree

Hide file tree

Showing 5 changed files with 0 additions and 52 deletions.
diff --git a/...s/transport-netty4/src/main/java/org/elasticsearch/transport/netty4/Netty4TcpChannel.java b/...s/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<Void> 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
@@ -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
@@ -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
@@ -273,10 +273,6 @@ public void addCloseListener(ActionListener<Void> listener) {
         public void addConnectListener(ActionListener<Void> 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
@@ -399,13 +399,6 @@ public void addConnectListener(ActionListener<Void> 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();