Skip to content

Commit 34e0e32

Browse files
committed
Fix RemoteConnectionManager size() method
Currently the remote connection manager will delegate the size() call to the underlying cluster connection manager. This introduces the possibility that call will return 1 before the nodeConnection method has been triggered to add the connection to the remote connection list. This can cause issues, as the ensureConnected method checks the connection managers size and executes synchronously if the size is > 0. This leads to a potential cluster not connected exception while we are still waiting for the connection opened callback to be triggered. This commit fixes this issue by using the remote connection manager's size to report the connection manager's size. Fixes elastic#52029.
1 parent 4e003fe commit 34e0e32

File tree

4 files changed

+7
-5
lines changed

4 files changed

+7
-5
lines changed

server/src/main/java/org/elasticsearch/transport/RemoteClusterAwareClient.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ final class RemoteClusterAwareClient extends AbstractClient {
4545
@Override
4646
protected <Request extends ActionRequest, Response extends ActionResponse>
4747
void doExecute(ActionType<Response> action, Request request, ActionListener<Response> listener) {
48-
remoteClusterService.ensureConnected(clusterAlias, ActionListener.wrap(res -> {
48+
remoteClusterService.ensureConnected(clusterAlias, ActionListener.wrap(v -> {
4949
Transport.Connection connection;
5050
if (request instanceof RemoteClusterAwareRequest) {
5151
DiscoveryNode preferredTargetNode = ((RemoteClusterAwareRequest) request).getPreferredTargetNode();

server/src/main/java/org/elasticsearch/transport/RemoteConnectionManager.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,11 @@ public Transport.Connection getAnyRemoteConnection() {
110110

111111
@Override
112112
public int size() {
113-
return delegate.size();
113+
// Although we use a delegate instance, we report the connection manager size based on the
114+
// RemoteConnectionManager's knowledge of the connections. This is because there is a brief window
115+
// in between the time when the connection is added to the delegate map, and the time when
116+
// nodeConnected is called.
117+
return this.connections.size();
114118
}
115119

116120
@Override

server/src/main/java/org/elasticsearch/transport/SniffConnectionStrategy.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -349,8 +349,7 @@ public void onResponse(Void aVoid) {
349349

350350
@Override
351351
public void onFailure(Exception e) {
352-
if (e instanceof ConnectTransportException ||
353-
e instanceof IllegalStateException) {
352+
if (e instanceof ConnectTransportException || e instanceof IllegalStateException) {
354353
// ISE if we fail the handshake with an version incompatible node
355354
// fair enough we can't connect just move on
356355
logger.debug(() -> new ParameterizedMessage("failed to connect to node {}", node), e);

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,6 @@ public void testConnectAndExecuteRequest() throws Exception {
7474
}
7575
}
7676

77-
@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/52029")
7877
public void testEnsureWeReconnect() throws Exception {
7978
Settings remoteSettings = Settings.builder().put(ClusterName.CLUSTER_NAME_SETTING.getKey(), "foo_bar_cluster").build();
8079
try (MockTransportService remoteTransport = startTransport("remote_node", Collections.emptyList(), Version.CURRENT, threadPool,

0 commit comments

Comments
 (0)