Skip to content

Commit bfd29fb

Browse files
authored
Prevent connection races in testEnsureWeReconnect (elastic#56847)
Currently it is possible that a sniff connection round is occurring as we enter another test loop in testEnsureWeReconnect. The problem is that once we enter another loop, closing the connection manually can cause this pre-existing connection round to fail. This round failing can fail the test. This commit fixes the issue by ensuring that there are no in-progress connections before entering another loop.
1 parent 832dd83 commit bfd29fb

File tree

1 file changed

+17
-27
lines changed

1 file changed

+17
-27
lines changed

server/src/test/java/org/elasticsearch/transport/RemoteClusterClientTests.java

+17-27
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020

2121
import org.elasticsearch.Version;
2222
import org.elasticsearch.action.admin.cluster.state.ClusterStateResponse;
23+
import org.elasticsearch.action.support.PlainActionFuture;
2324
import org.elasticsearch.client.Client;
2425
import org.elasticsearch.cluster.ClusterName;
2526
import org.elasticsearch.cluster.node.DiscoveryNode;
@@ -31,7 +32,6 @@
3132
import org.elasticsearch.threadpool.ThreadPool;
3233

3334
import java.util.Collections;
34-
import java.util.concurrent.Semaphore;
3535
import java.util.concurrent.TimeUnit;
3636

3737
import static org.elasticsearch.transport.RemoteClusterConnectionTests.startTransport;
@@ -86,37 +86,27 @@ public void testEnsureWeReconnect() throws Exception {
8686
.put("cluster.remote.test.seeds",
8787
remoteNode.getAddress().getAddress() + ":" + remoteNode.getAddress().getPort()).build();
8888
try (MockTransportService service = MockTransportService.createNewService(localSettings, Version.CURRENT, threadPool, null)) {
89-
Semaphore semaphore = new Semaphore(1);
9089
service.start();
91-
service.getRemoteClusterService().getConnections().forEach(con -> {
92-
con.getConnectionManager().addListener(new TransportConnectionListener() {
93-
@Override
94-
public void onNodeDisconnected(DiscoveryNode node, Transport.Connection connection) {
95-
if (remoteNode.equals(node)) {
96-
semaphore.release();
97-
}
98-
}
99-
});
100-
});
10190
// this test is not perfect since we might reconnect concurrently but it will fail most of the time if we don't have
10291
// the right calls in place in the RemoteAwareClient
10392
service.acceptIncomingRequests();
93+
RemoteClusterService remoteClusterService = service.getRemoteClusterService();
94+
assertBusy(() -> assertTrue(remoteClusterService.isRemoteNodeConnected("test", remoteNode)));
10495
for (int i = 0; i < 10; i++) {
105-
semaphore.acquire();
106-
try {
107-
service.getRemoteClusterService().getConnections().forEach(con -> {
108-
con.getConnectionManager().disconnectFromNode(remoteNode);
109-
});
110-
semaphore.acquire();
111-
RemoteClusterService remoteClusterService = service.getRemoteClusterService();
112-
Client client = remoteClusterService.getRemoteClusterClient(threadPool, "test");
113-
ClusterStateResponse clusterStateResponse = client.admin().cluster().prepareState().execute().get();
114-
assertNotNull(clusterStateResponse);
115-
assertEquals("foo_bar_cluster", clusterStateResponse.getState().getClusterName().value());
116-
assertTrue(remoteClusterService.isRemoteNodeConnected("test", remoteNode));
117-
} finally {
118-
semaphore.release();
119-
}
96+
RemoteClusterConnection remoteClusterConnection = remoteClusterService.getRemoteClusterConnection("test");
97+
assertBusy(remoteClusterConnection::assertNoRunningConnections);
98+
ConnectionManager connectionManager = remoteClusterConnection.getConnectionManager();
99+
Transport.Connection connection = connectionManager.getConnection(remoteNode);
100+
PlainActionFuture<Void> closeFuture = PlainActionFuture.newFuture();
101+
connection.addCloseListener(closeFuture);
102+
connectionManager.disconnectFromNode(remoteNode);
103+
closeFuture.get();
104+
105+
Client client = remoteClusterService.getRemoteClusterClient(threadPool, "test");
106+
ClusterStateResponse clusterStateResponse = client.admin().cluster().prepareState().execute().get();
107+
assertNotNull(clusterStateResponse);
108+
assertEquals("foo_bar_cluster", clusterStateResponse.getState().getClusterName().value());
109+
assertTrue(remoteClusterConnection.isNodeConnected(remoteNode));
120110
}
121111
}
122112
}

0 commit comments

Comments
 (0)