Remove `TcpChannel#setSoLinger` method (#35924)

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. We still will set SO_LINGER on the
`MockNioTransport`. However we can do this manually.
This commit is contained in:
Tim Brooks 2018-11-27 09:08:14 -07:00 committed by GitHub
parent b6ed6ef189
commit cc1fa799c8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 12 additions and 75 deletions

View File

@ -20,13 +20,8 @@
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 java.io.IOException;
import org.elasticsearch.ExceptionsHelper;
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.common.Nullable;
@ -97,17 +92,6 @@ public class Netty4TcpChannel implements TcpChannel {
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();

View File

@ -24,8 +24,6 @@ import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.nio.NioSocketChannel;
import org.elasticsearch.transport.TcpChannel;
import java.io.IOException;
import java.net.StandardSocketOptions;
import java.nio.channels.SocketChannel;
public class NioTcpChannel extends NioSocketChannel implements TcpChannel {
@ -41,13 +39,6 @@ public class NioTcpChannel extends NioSocketChannel implements TcpChannel {
getContext().sendMessage(BytesReference.toByteBuffers(reference), ActionListener.toBiConsumer(listener));
}
@Override
public void setSoLinger(int value) throws IOException {
if (isOpen()) {
getRawChannel().setOption(StandardSocketOptions.SO_LINGER, value);
}
}
@Override
public String getProfile() {
return profile;

View File

@ -23,7 +23,6 @@ import org.elasticsearch.action.ActionListener;
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.
*

View File

@ -350,24 +350,6 @@ public abstract class TcpTransport extends AbstractLifecycleComponent implements
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 {

View File

@ -283,10 +283,6 @@ public class TcpTransportTests extends ESTestCase {
public void addConnectListener(ActionListener<Void> listener) {
}
@Override
public void setSoLinger(int value) throws IOException {
}
@Override
public boolean isOpen() {
return false;

View File

@ -399,13 +399,6 @@ public class MockTcpTransport extends TcpTransport {
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();

View File

@ -51,6 +51,7 @@ import org.elasticsearch.transport.TcpTransport;
import org.elasticsearch.transport.TransportRequestOptions;
import java.io.IOException;
import java.io.UncheckedIOException;
import java.net.InetSocketAddress;
import java.net.StandardSocketOptions;
import java.nio.ByteBuffer;
@ -193,7 +194,17 @@ public class MockNioTransport extends TcpTransport {
BytesChannelContext context = new BytesChannelContext(nioChannel, selector, (e) -> exceptionCaught(nioChannel, e),
readWriteHandler, new InboundChannelBuffer(pageSupplier));
nioChannel.setContext(context);
nioChannel.setSoLinger(0);
nioChannel.addConnectListener((v, e) -> {
if (e == null) {
if (channel.isConnected()) {
try {
channel.setOption(StandardSocketOptions.SO_LINGER, 0);
} catch (IOException ex) {
throw new UncheckedIOException(new IOException());
}
}
}
});
return nioChannel;
}
@ -280,14 +291,6 @@ public class MockNioTransport extends TcpTransport {
addConnectListener(ActionListener.toBiConsumer(listener));
}
@Override
public void setSoLinger(int value) throws IOException {
SocketChannel rawChannel = getRawChannel();
if (rawChannel.isConnected()) {
rawChannel.setOption(StandardSocketOptions.SO_LINGER, value);
}
}
@Override
public void sendMessage(BytesReference reference, ActionListener<Void> listener) {
getContext().sendMessage(BytesReference.toByteBuffers(reference), ActionListener.toBiConsumer(listener));

View File

@ -40,7 +40,6 @@ 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 java.io.IOException;
import java.net.InetAddress;
@ -110,8 +109,6 @@ public class SimpleMockNioTransportTests extends AbstractSimpleTransportTestCase
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)
.build();
ClusterSettings clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS);