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

Add local node to cluster state #6811

Closed
wants to merge 1 commit into from

Conversation

kimchy
Copy link
Member

@kimchy kimchy commented Jul 10, 2014

Today, the tribe node needs the local node so it adds it when it starts, but other APIs would benefit from adding the local node, also, adding the local node should be done in a cleaner manner, where it belongs, which is right after the discovery service starts in the cluster service

Today, the tribe node needs the local node so it adds it when it starts, but other APIs would benefit from adding the local node, also, adding the local node should be done in a cleaner manner, where it belongs, which is right after the discovery service starts in the cluster service
closes elastic#6811
@@ -37,11 +38,28 @@
*/
public class DiscoveryService extends AbstractLifecycleComponent<DiscoveryService> {

private final TimeValue initialStateTimeout;
private static class InitialState implements InitialStateDiscoveryListener {
Copy link
Member

Choose a reason for hiding this comment

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

how about calling this class InitialStateListener instead?

@javanna
Copy link
Member

javanna commented Jul 10, 2014

Left two super minor comments around naming, LGTM otherwise

@javanna javanna removed the review label Jul 10, 2014
@kimchy kimchy closed this in 9ca5e6e Jul 10, 2014
@kimchy kimchy deleted the add_local_node_cluster_state branch July 10, 2014 12:50
kimchy added a commit that referenced this pull request Jul 10, 2014
Today, the tribe node needs the local node so it adds it when it starts, but other APIs would benefit from adding the local node, also, adding the local node should be done in a cleaner manner, where it belongs, which is right after the discovery service starts in the cluster service
closes #6811
@kimchy
Copy link
Member Author

kimchy commented Jul 10, 2014

@javanna cool, renamed and pushed

@clintongormley clintongormley changed the title Add local node to cluster state Internal: Add local node to cluster state Jul 16, 2014
javanna added a commit to javanna/elasticsearch that referenced this pull request Jul 28, 2014
…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 elastic#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 added a commit that referenced this pull request Jul 28, 2014
…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 added a commit that referenced this pull request Jul 28, 2014
…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
@clintongormley clintongormley changed the title Internal: Add local node to cluster state Add local node to cluster state Jun 7, 2015
@clintongormley clintongormley added :Distributed/Distributed A catch all label for anything in the Distributed Area. If you aren't sure, use this one. and removed :Cluster labels Feb 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed/Distributed A catch all label for anything in the Distributed Area. If you aren't sure, use this one. >enhancement v1.3.0 v2.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants