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

Projects
None yet
3 participants
@kimchy
Copy link
Member

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

Add local node to cluster state
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 kimchy added review labels Jul 10, 2014

@@ -37,11 +38,28 @@
*/
public class DiscoveryService extends AbstractLifecycleComponent<DiscoveryService> {

private final TimeValue initialStateTimeout;
private static class InitialState implements InitialStateDiscoveryListener {

This comment has been minimized.

Copy link
@javanna

javanna Jul 10, 2014

Member

how about calling this class InitialStateListener instead?

private volatile boolean initialStateReceived;
private final TimeValue initialStateTimeout;
private final Discovery discovery;
private InitialState initialState;

This comment has been minimized.

Copy link
@javanna

javanna Jul 10, 2014

Member

and calling this variable initialStateListener?

@javanna

This comment has been minimized.

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 kimchy:add_local_node_cluster_state branch Jul 10, 2014

kimchy added a commit that referenced this pull request Jul 10, 2014

Add local node to cluster state
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

This comment has been minimized.

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

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 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

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 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

@clintongormley clintongormley changed the title Internal: Add local node to cluster state Add local node to cluster state Jun 7, 2015

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.