Skip to content

Loading…

With unicast discovery, only disconnect from temporary connected nodes #6966

Closed
wants to merge 1 commit into from

4 participants

@kimchy
elastic member

In unicast discovery, we try to reuse existing discovery nodes based on the node address they have. If we find an existing node based on its address, and for some reason its not connected, don't add it to the list of nodes to disconnect from, as that (full) connection is useful down the road
Also, fixed a rogue count down on a latch in case the ping cycle has ended (in practice, it didn't do any harm, since the latch has timed out already by definition, but still cleaner)

@kimchy kimchy added the enhancement label
@martijnvg martijnvg commented on an outdated diff
...search/discovery/zen/ping/unicast/UnicastZenPing.java
@@ -294,7 +299,6 @@ public void run() {
sendPingRequestToNode(sendPingsHandler.id(), timeout, pingRequest, latch, node, nodeToSend);
} else {
// connect took too long, just log it and bail
- latch.countDown();
@martijnvg elastic member

I think we need the latch.countDown(), because then success variable will be set to true and then we don't do latch.countDown() in the finally block?

@kimchy elastic member
kimchy added a note

you are correct!, my bad

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@kimchy kimchy Unicast discovery: only disconnect from temporary connected nodes
In unicast discovery, we try to reuse existing discovery nodes based on the node address they have. If we find an existing node based on its address, and for some reason its not connected, don't add it to the list of nodes to disconnect from, as that (full) connection is useful down the road
closes #6966
807b65b
@martijnvg
elastic member

LGTM

@kimchy kimchy added a commit that closed this pull request
@kimchy kimchy Unicast discovery: only disconnect from temporary connected nodes
In unicast discovery, we try to reuse existing discovery nodes based on the node address they have. If we find an existing node based on its address, and for some reason its not connected, don't add it to the list of nodes to disconnect from, as that (full) connection is useful down the road
closes #6966
50ececb
@kimchy kimchy closed this in 50ececb
@kimchy kimchy added a commit that referenced this pull request
@kimchy kimchy Unicast discovery: only disconnect from temporary connected nodes
In unicast discovery, we try to reuse existing discovery nodes based on the node address they have. If we find an existing node based on its address, and for some reason its not connected, don't add it to the list of nodes to disconnect from, as that (full) connection is useful down the road
closes #6966
31b752d
@kimchy kimchy deleted the kimchy:only_disconnect_from_light_connections branch
@jpountz jpountz removed the review label
@lindstromhenrik lindstromhenrik added a commit to episerver/elasticsearch that referenced this pull request
@kimchy kimchy Unicast discovery: only disconnect from temporary connected nodes
In unicast discovery, we try to reuse existing discovery nodes based on the node address they have. If we find an existing node based on its address, and for some reason its not connected, don't add it to the list of nodes to disconnect from, as that (full) connection is useful down the road
closes #6966
f38eb94
@clintongormley clintongormley changed the title from Unicast discovery: only disconnect from temporary connected nodes to Internal: With unicast discovery, only disconnect from temporary connected nodes
@clintongormley clintongormley changed the title from Internal: With unicast discovery, only disconnect from temporary connected nodes to With unicast discovery, only disconnect from temporary connected nodes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jul 22, 2014
  1. @kimchy

    Unicast discovery: only disconnect from temporary connected nodes

    kimchy committed
    In unicast discovery, we try to reuse existing discovery nodes based on the node address they have. If we find an existing node based on its address, and for some reason its not connected, don't add it to the list of nodes to disconnect from, as that (full) connection is useful down the road
    closes #6966
View
7 src/main/java/org/elasticsearch/discovery/zen/ping/unicast/UnicastZenPing.java
@@ -270,7 +270,12 @@ void sendPings(final TimeValue timeout, @Nullable TimeValue waitTime, final Send
if (sendPingsHandler.isClosed()) {
return;
}
- sendPingsHandler.nodeToDisconnect.add(nodeToSend);
+ // only disconnect from nodes that we will end up creating a light connection to, as they are temporal
+ // if we find on the disco nodes a matching node by address, we are going to restore the connection
+ // anyhow down the line if its not connected...
+ if (!nodeFoundByAddress) {
+ sendPingsHandler.nodeToDisconnect.add(nodeToSend);
+ }
// fork the connection to another thread
sendPingsHandler.executor().execute(new Runnable() {
@Override
Something went wrong with that request. Please try again.