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

Transport client: Don't add listed nodes to connected nodes list in sniff mode #7067

Conversation

Projects
None yet
4 participants
@javanna
Copy link
Member

commented Jul 28, 2014

This commit effectively reverts e1aa91d , as it is not needed anymore to add the original listed nodes. The cluster state local call made will in fact always return at least the local node (see #6811).

There were a couple of downsides caused by putting the original listed nodes among the connected nodes:

  1. in the following retries, they weren't seen as listed nodes anymore, thus the light connect wasn't used
  2. among the connected nodes some were "bad" duplicates as they are already there and don't contain all needed info for each node. This was causing serialization problems for instance given that the node version was missing on the DiscoveryNode object (or minCompatibilityVersion after #6894).

(As a side note, the fact that nodes were appearing twice in the list was hiding #6829 in sniff mode, as more nodes than expected were in the list and then retried)

Next step is to enable transport client sniff mode in our tests, already in the work on a public branch: https://github.com/elasticsearch/elasticsearch/tree/enhancement/test-enable-transport-client-sniff .

Transport client: don't add listed nodes to connected nodes list in s…
…niff mode

This commit effectively reverts e1aa91d , as it is not needed anymore to add the original listed nodes. The cluster state local call made will in fact always return at least the local node (see #6811).

There were a couple of downsides caused by putting the original listed nodes among the connected nodes:
1) in the following retries, they weren't seen as listed nodes anymore, thus the light connect wasn't used
2) among the connected nodes some were "bad" duplicates as they are already there and don't contain all needed info for each node. This was causing serialization problems for instance given that the node version was missing on the `DiscoveryNode` object.

@javanna javanna added bug labels Jul 28, 2014

@javanna javanna self-assigned this Jul 28, 2014

@kimchy

This comment has been minimized.

Copy link
Member

commented Jul 28, 2014

LGTM

javanna added a commit that referenced this pull request Jul 28, 2014

Transport client: don't add listed nodes to connected nodes list in s…
…niff mode

This commit effectively reverts e1aa91d , as it is not needed anymore to add the original listed nodes. The cluster state local call made will in fact always return at least the local node (see #6811).

There were a couple of downsides caused by putting the original listed nodes among the connected nodes:
1) in the following retries, they weren't seen as listed nodes anymore, thus the light connect wasn't used
2) among the connected nodes some were "bad" duplicates as they are already there and don't contain all needed info for each node. This was causing serialization problems for instance given that the node version was missing on the `DiscoveryNode` object.

Closes #7067

@javanna javanna closed this in 91c4824 Jul 28, 2014

@jpountz jpountz removed the review label Jul 29, 2014

@clintongormley clintongormley changed the title Transport client: don't add listed nodes to connected nodes list in sniff mode Transport client: Don't add listed nodes to connected nodes list in sniff mode Sep 8, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.