Skip to content

Commit ee7eac8

Browse files
Tim-BrooksTim-Brooks
authored
MockTcpTransport to connect asynchronously (#28203)
The method `initiateChannel` on `TcpTransport` is explicit in that channels can be connect asynchronously. All production implementations do connect asynchronously. Only the blocking `MockTcpTransport` connects in a synchronous manner. This avoids testing some of the blocking code in `TcpTransport` that waits on connections to complete. Additionally, it requires a more extensive method signature than required for other transports. This commit modifies the `MockTcpTransport` to make these connections asynchronously on a different thread. Additionally, it simplifies that `initiateChannel` method signature.
1 parent 190f1e1 commit ee7eac8

File tree

6 files changed

+46
-56
lines changed

6 files changed

+46
-56
lines changed

‎modules/transport-netty4/src/main/java/org/elasticsearch/transport/netty4/Netty4Transport.java‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@
4040
import org.elasticsearch.ElasticsearchException;
4141
import org.elasticsearch.ExceptionsHelper;
4242
import org.elasticsearch.action.ActionListener;
43-
import org.elasticsearch.cluster.node.DiscoveryNode;
4443
import org.elasticsearch.common.SuppressForbidden;
4544
import org.elasticsearch.common.collect.Tuple;
4645
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
@@ -51,12 +50,10 @@
5150
import org.elasticsearch.common.settings.Settings;
5251
import org.elasticsearch.common.unit.ByteSizeUnit;
5352
import org.elasticsearch.common.unit.ByteSizeValue;
54-
import org.elasticsearch.common.unit.TimeValue;
5553
import org.elasticsearch.common.util.BigArrays;
5654
import org.elasticsearch.common.util.concurrent.EsExecutors;
5755
import org.elasticsearch.indices.breaker.CircuitBreakerService;
5856
import org.elasticsearch.threadpool.ThreadPool;
59-
import org.elasticsearch.transport.TcpChannel;
6057
import org.elasticsearch.transport.TcpTransport;
6158
import org.elasticsearch.transport.TransportRequestOptions;
6259

@@ -239,9 +236,8 @@ protected final void exceptionCaught(ChannelHandlerContext ctx, Throwable cause)
239236
}
240237

241238
@Override
242-
protected NettyTcpChannel initiateChannel(DiscoveryNode node, TimeValue connectTimeout, ActionListener<Void> listener)
243-
throws IOException {
244-
ChannelFuture channelFuture = bootstrap.connect(node.getAddress().address());
239+
protected NettyTcpChannel initiateChannel(InetSocketAddress address, ActionListener<Void> listener) throws IOException {
240+
ChannelFuture channelFuture = bootstrap.connect(address);
245241
Channel channel = channelFuture.channel();
246242
if (channel == null) {
247243
Netty4Utils.maybeDie(channelFuture.cause());

‎plugins/transport-nio/src/main/java/org/elasticsearch/transport/nio/NioTransport.java‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,12 @@
2121

2222
import org.elasticsearch.ElasticsearchException;
2323
import org.elasticsearch.action.ActionListener;
24-
import org.elasticsearch.cluster.node.DiscoveryNode;
2524
import org.elasticsearch.common.bytes.BytesReference;
2625
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
2726
import org.elasticsearch.common.network.NetworkService;
2827
import org.elasticsearch.common.recycler.Recycler;
2928
import org.elasticsearch.common.settings.Setting;
3029
import org.elasticsearch.common.settings.Settings;
31-
import org.elasticsearch.common.unit.TimeValue;
3230
import org.elasticsearch.common.util.BigArrays;
3331
import org.elasticsearch.common.util.PageCacheRecycler;
3432
import org.elasticsearch.common.util.concurrent.EsExecutors;
@@ -93,9 +91,8 @@ protected TcpNioServerSocketChannel bind(String name, InetSocketAddress address)
9391
}
9492

9593
@Override
96-
protected TcpNioSocketChannel initiateChannel(DiscoveryNode node, TimeValue connectTimeout, ActionListener<Void> connectListener)
97-
throws IOException {
98-
TcpNioSocketChannel channel = nioGroup.openChannel(node.getAddress().address(), clientChannelFactory);
94+
protected TcpNioSocketChannel initiateChannel(InetSocketAddress address, ActionListener<Void> connectListener) throws IOException {
95+
TcpNioSocketChannel channel = nioGroup.openChannel(address, clientChannelFactory);
9996
channel.addConnectListener(ActionListener.toBiConsumer(connectListener));
10097
return channel;
10198
}

‎server/src/main/java/org/elasticsearch/transport/TcpTransport.java‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -604,7 +604,7 @@ public final NodeChannels openConnection(DiscoveryNode node, ConnectionProfile c
604604
try {
605605
PlainActionFuture<Void> connectFuture = PlainActionFuture.newFuture();
606606
connectionFutures.add(connectFuture);
607-
TcpChannel channel = initiateChannel(node, connectionProfile.getConnectTimeout(), connectFuture);
607+
TcpChannel channel = initiateChannel(node.getAddress().address(), connectFuture);
608608
logger.trace(() -> new ParameterizedMessage("Tcp transport client channel opened: {}", channel));
609609
channels.add(channel);
610610
} catch (Exception e) {
@@ -1057,17 +1057,14 @@ protected void serverAcceptedChannel(TcpChannel channel) {
10571057
protected abstract TcpChannel bind(String name, InetSocketAddress address) throws IOException;
10581058

10591059
/**
1060-
* Initiate a single tcp socket channel to a node. Implementations do not have to observe the connectTimeout.
1061-
* It is provided for synchronous connection implementations.
1060+
* Initiate a single tcp socket channel.
10621061
*
1063-
* @param node the node
1064-
* @param connectTimeout the connection timeout
1065-
* @param connectListener listener to be called when connection complete
1062+
* @param address address for the initiated connection
1063+
* @param connectListener listener to be called when connection complete
10661064
* @return the pending connection
10671065
* @throws IOException if an I/O exception occurs while opening the channel
10681066
*/
1069-
protected abstract TcpChannel initiateChannel(DiscoveryNode node, TimeValue connectTimeout, ActionListener<Void> connectListener)
1070-
throws IOException;
1067+
protected abstract TcpChannel initiateChannel(InetSocketAddress address, ActionListener<Void> connectListener) throws IOException;
10711068

10721069
/**
10731070
* Called to tear down internal resources

‎server/src/test/java/org/elasticsearch/transport/TcpTransportTests.java‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
import org.elasticsearch.Version;
2323
import org.elasticsearch.action.ActionListener;
2424
import org.elasticsearch.cluster.node.DiscoveryNode;
25-
import org.elasticsearch.common.bytes.BytesArray;
2625
import org.elasticsearch.common.bytes.BytesReference;
2726
import org.elasticsearch.common.compress.CompressorFactory;
2827
import org.elasticsearch.common.io.stream.BytesStreamOutput;
@@ -41,15 +40,13 @@
4140
import java.io.IOException;
4241
import java.io.StreamCorruptedException;
4342
import java.net.InetSocketAddress;
44-
import java.nio.ByteBuffer;
4543
import java.util.ArrayList;
4644
import java.util.concurrent.TimeUnit;
4745
import java.util.concurrent.atomic.AtomicReference;
4846

4947
import static org.hamcrest.Matchers.equalTo;
5048
import static org.hamcrest.core.IsInstanceOf.instanceOf;
5149
import static org.mockito.Mockito.verify;
52-
import static org.mockito.Mockito.verifyZeroInteractions;
5350

5451
/** Unit tests for {@link TcpTransport} */
5552
public class TcpTransportTests extends ESTestCase {
@@ -193,8 +190,7 @@ protected FakeChannel bind(String name, InetSocketAddress address) throws IOExce
193190
}
194191

195192
@Override
196-
protected FakeChannel initiateChannel(DiscoveryNode node, TimeValue connectTimeout, ActionListener<Void> connectListener)
197-
throws IOException {
193+
protected FakeChannel initiateChannel(InetSocketAddress address, ActionListener<Void> connectListener) throws IOException {
198194
return new FakeChannel(messageCaptor);
199195
}
200196

‎test/framework/src/main/java/org/elasticsearch/transport/MockTcpTransport.java‎

Lines changed: 34 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
import org.apache.lucene.util.IOUtils;
2222
import org.elasticsearch.Version;
2323
import org.elasticsearch.action.ActionListener;
24-
import org.elasticsearch.cluster.node.DiscoveryNode;
2524
import org.elasticsearch.common.bytes.BytesReference;
2625
import org.elasticsearch.common.io.stream.BytesStreamOutput;
2726
import org.elasticsearch.common.io.stream.InputStreamStreamInput;
@@ -30,7 +29,6 @@
3029
import org.elasticsearch.common.network.NetworkService;
3130
import org.elasticsearch.common.settings.Settings;
3231
import org.elasticsearch.common.unit.ByteSizeValue;
33-
import org.elasticsearch.common.unit.TimeValue;
3432
import org.elasticsearch.common.util.BigArrays;
3533
import org.elasticsearch.common.util.CancellableThreads;
3634
import org.elasticsearch.common.util.concurrent.AbstractRunnable;
@@ -49,7 +47,6 @@
4947
import java.net.ServerSocket;
5048
import java.net.Socket;
5149
import java.net.SocketException;
52-
import java.net.SocketTimeoutException;
5350
import java.util.Collections;
5451
import java.util.HashSet;
5552
import java.util.Set;
@@ -61,7 +58,6 @@
6158
import java.util.concurrent.Executors;
6259
import java.util.concurrent.TimeUnit;
6360
import java.util.concurrent.atomic.AtomicBoolean;
64-
import java.util.function.Consumer;
6561

6662
/**
6763
* This is a socket based blocking TcpTransport implementation that is used for tests
@@ -164,28 +160,32 @@ private void readMessage(MockChannel mockChannel, StreamInput input) throws IOEx
164160
}
165161

166162
@Override
167-
protected MockChannel initiateChannel(DiscoveryNode node, TimeValue connectTimeout, ActionListener<Void> connectListener)
168-
throws IOException {
169-
InetSocketAddress address = node.getAddress().address();
163+
protected MockChannel initiateChannel(InetSocketAddress address, ActionListener<Void> connectListener) throws IOException {
170164
final MockSocket socket = new MockSocket();
165+
final MockChannel channel = new MockChannel(socket, address, "none");
166+
171167
boolean success = false;
172168
try {
173169
configureSocket(socket);
174-
try {
175-
socket.connect(address, Math.toIntExact(connectTimeout.millis()));
176-
} catch (SocketTimeoutException ex) {
177-
throw new ConnectTransportException(node, "connect_timeout[" + connectTimeout + "]", ex);
178-
}
179-
MockChannel channel = new MockChannel(socket, address, "none", (c) -> {});
180-
channel.loopRead(executor);
181170
success = true;
182-
connectListener.onResponse(null);
183-
return channel;
184171
} finally {
185172
if (success == false) {
186173
IOUtils.close(socket);
187174
}
175+
188176
}
177+
178+
executor.submit(() -> {
179+
try {
180+
socket.connect(address);
181+
channel.loopRead(executor);
182+
connectListener.onResponse(null);
183+
} catch (Exception ex) {
184+
connectListener.onFailure(ex);
185+
}
186+
});
187+
188+
return channel;
189189
}
190190

191191
@Override
@@ -218,7 +218,6 @@ public final class MockChannel implements Closeable, TcpChannel {
218218
private final Socket activeChannel;
219219
private final String profile;
220220
private final CancellableThreads cancellableThreads = new CancellableThreads();
221-
private final Closeable onClose;
222221
private final CompletableFuture<Void> closeFuture = new CompletableFuture<>();
223222

224223
/**
@@ -227,14 +226,12 @@ public final class MockChannel implements Closeable, TcpChannel {
227226
* @param socket The client socket. Mut not be null.
228227
* @param localAddress Address associated with the corresponding local server socket. Must not be null.
229228
* @param profile The associated profile name.
230-
* @param onClose Callback to execute when this channel is closed.
231229
*/
232-
public MockChannel(Socket socket, InetSocketAddress localAddress, String profile, Consumer<MockChannel> onClose) {
230+
public MockChannel(Socket socket, InetSocketAddress localAddress, String profile) {
233231
this.localAddress = localAddress;
234232
this.activeChannel = socket;
235233
this.serverSocket = null;
236234
this.profile = profile;
237-
this.onClose = () -> onClose.accept(this);
238235
synchronized (openChannels) {
239236
openChannels.add(this);
240237
}
@@ -246,12 +243,11 @@ public MockChannel(Socket socket, InetSocketAddress localAddress, String profile
246243
* @param serverSocket The associated server socket. Must not be null.
247244
* @param profile The associated profile name.
248245
*/
249-
public MockChannel(ServerSocket serverSocket, String profile) {
246+
MockChannel(ServerSocket serverSocket, String profile) {
250247
this.localAddress = (InetSocketAddress) serverSocket.getLocalSocketAddress();
251248
this.serverSocket = serverSocket;
252249
this.profile = profile;
253250
this.activeChannel = null;
254-
this.onClose = null;
255251
synchronized (openChannels) {
256252
openChannels.add(this);
257253
}
@@ -266,8 +262,19 @@ public void accept(Executor executor) throws IOException {
266262
synchronized (this) {
267263
if (isOpen.get()) {
268264
incomingChannel = new MockChannel(incomingSocket,
269-
new InetSocketAddress(incomingSocket.getLocalAddress(), incomingSocket.getPort()), profile,
270-
workerChannels::remove);
265+
new InetSocketAddress(incomingSocket.getLocalAddress(), incomingSocket.getPort()), profile);
266+
MockChannel finalIncomingChannel = incomingChannel;
267+
incomingChannel.addCloseListener(new ActionListener<Void>() {
268+
@Override
269+
public void onResponse(Void aVoid) {
270+
workerChannels.remove(finalIncomingChannel);
271+
}
272+
273+
@Override
274+
public void onFailure(Exception e) {
275+
workerChannels.remove(finalIncomingChannel);
276+
}
277+
});
271278
serverAcceptedChannel(incomingChannel);
272279
//establish a happens-before edge between closing and accepting a new connection
273280
workerChannels.add(incomingChannel);
@@ -287,7 +294,7 @@ public void accept(Executor executor) throws IOException {
287294
}
288295
}
289296

290-
public void loopRead(Executor executor) {
297+
void loopRead(Executor executor) {
291298
executor.execute(new AbstractRunnable() {
292299
@Override
293300
public void onFailure(Exception e) {
@@ -312,7 +319,7 @@ protected void doRun() throws Exception {
312319
});
313320
}
314321

315-
public synchronized void close0() throws IOException {
322+
synchronized void close0() throws IOException {
316323
// establish a happens-before edge between closing and accepting a new connection
317324
// we have to sync this entire block to ensure that our openChannels checks work correctly.
318325
// The close block below will close all worker channels but if one of the worker channels runs into an exception
@@ -325,7 +332,7 @@ public synchronized void close0() throws IOException {
325332
removedChannel = openChannels.remove(this);
326333
}
327334
IOUtils.close(serverSocket, activeChannel, () -> IOUtils.close(workerChannels),
328-
() -> cancellableThreads.cancel("channel closed"), onClose);
335+
() -> cancellableThreads.cancel("channel closed"));
329336
assert removedChannel: "Channel was not removed or removed twice?";
330337
}
331338
}

‎test/framework/src/main/java/org/elasticsearch/transport/nio/MockNioTransport.java‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,11 @@
2121

2222
import org.elasticsearch.ElasticsearchException;
2323
import org.elasticsearch.action.ActionListener;
24-
import org.elasticsearch.cluster.node.DiscoveryNode;
2524
import org.elasticsearch.common.bytes.BytesReference;
2625
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
2726
import org.elasticsearch.common.network.NetworkService;
2827
import org.elasticsearch.common.recycler.Recycler;
2928
import org.elasticsearch.common.settings.Settings;
30-
import org.elasticsearch.common.unit.TimeValue;
3129
import org.elasticsearch.common.util.BigArrays;
3230
import org.elasticsearch.common.util.PageCacheRecycler;
3331
import org.elasticsearch.indices.breaker.CircuitBreakerService;
@@ -83,9 +81,8 @@ protected MockServerChannel bind(String name, InetSocketAddress address) throws
8381
}
8482

8583
@Override
86-
protected MockSocketChannel initiateChannel(DiscoveryNode node, TimeValue connectTimeout, ActionListener<Void> connectListener)
87-
throws IOException {
88-
MockSocketChannel channel = nioGroup.openChannel(node.getAddress().address(), clientChannelFactory);
84+
protected MockSocketChannel initiateChannel(InetSocketAddress address, ActionListener<Void> connectListener) throws IOException {
85+
MockSocketChannel channel = nioGroup.openChannel(address, clientChannelFactory);
8986
channel.addConnectListener(ActionListener.toBiConsumer(connectListener));
9087
return channel;
9188
}

0 commit comments

Comments
 (0)