Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix RemoteConnectionManager size() method #52823

Conversation

Tim-Brooks
Copy link
Contributor

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 #52029.

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.
@Tim-Brooks Tim-Brooks added >bug :Distributed/Network Http and internode communication implementations v8.0.0 v7.7.0 v7.6.1 labels Feb 26, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed (:Distributed/Network)

Copy link
Member

@original-brownbear original-brownbear left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Tim-Brooks Tim-Brooks merged commit c5ed349 into elastic:master Feb 26, 2020
Tim-Brooks added a commit that referenced this pull request Mar 4, 2020
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 #52029.
@jimczi
Copy link
Contributor

jimczi commented Mar 18, 2020

@tbrooks8 is the backport pending label still needed ? Looks like this pr was backported to 7.x

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Distributed/Network Http and internode communication implementations v7.7.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CI] RemoteClusterClientTests testEnsureWeReconnect failing with NoSuchRemoteClusterException
6 participants