-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Introduce single-node discovery #23595
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
Conversation
This commit adds a single node discovery type. With this discovery type, a node will elect itself as master and never form a cluster with another node.
|
A follow-up will reveal the true purpose of this. 😉 |
bleskes
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Main change looks good to me. Left some minor comments.
|
|
||
| @Override | ||
| public DiscoveryStats stats() { | ||
| return null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm doubting if this should returns a new DiscoveryStats(null). wdyt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I pushed e10a245.
|
|
||
| }; | ||
| final ClusterStateTaskConfig config = ClusterStateTaskConfig.build(Priority.URGENT); | ||
| clusterService.submitStateUpdateTasks( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
out of curiosity - why didn't you use a simple ClusterStateUpdateTask ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No reasoning other than this is what my fingers did.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it wasn't my fingers. I see why I did this. A ClusterStateUpdateTask can not override runOnlyOnMaster which we need to set to false here. 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh right. I forgot about that. ++
| Stream.concat(pingingRound.getSeedNodes().stream(), nodesFromResponses.stream()) | ||
| .collect(Collectors.toMap(DiscoveryNode::getAddress, Function.identity(), (n1, n2) -> n1)); | ||
|
|
||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: unneeded?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I pushed f235c8b.
| .build(); | ||
| } | ||
|
|
||
| public void testDoesNotPing() throws IOException, InterruptedException, ExecutionException { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit confused by this test - as far as I can tell it doesn't do anything with single node discovery? what are we testing here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A single node with discovery type set to single-node is started, and then we attempt to ping as zen discovery would. The purpose is to test that the node will not join a cluster with zen discovery configured. Does that make sense? Do you have an idea for a better test? Do you think that we need to test that two nodes with single-node configured as the discovery type will not join each other?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I see. So it's more like a "node has a good cloaking device" test. Reading the name I assumed you check that a single-node discovery doesn't ping other nodes (i.e., doesn't join). I think this test is nice (with a new name). Maybe another good test is to start two nodes with the same cluster name and port ranges that would normally cause them to find each other and see that they don't (it's a bit tricky w.r.t fixing ports but I think we can figure something out)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| stack.push(clusterService); | ||
| final SingleNodeDiscovery discovery = | ||
| new SingleNodeDiscovery(Settings.EMPTY, clusterService); | ||
| discovery.startInitialJoin(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doesn't this go async? should we wait via an observer? maybe take the code from node.java ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. I pushed 5caab7d.
|
Thanks @bleskes; I have responded to your feedback. |
bleskes
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. See if you want to bite your teeth on that test suggestion
| .build(); | ||
| } | ||
|
|
||
| public void testDoesNotPing() throws IOException, InterruptedException, ExecutionException { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I see. So it's more like a "node has a good cloaking device" test. Reading the name I assumed you check that a single-node discovery doesn't ping other nodes (i.e., doesn't join). I think this test is nice (with a new name). Maybe another good test is to start two nodes with the same cluster name and port ranges that would normally cause them to find each other and see that they don't (it's a bit tricky w.r.t fixing ports but I think we can figure something out)
|
|
||
| }; | ||
| final ClusterStateTaskConfig config = ClusterStateTaskConfig.build(Priority.URGENT); | ||
| clusterService.submitStateUpdateTasks( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh right. I forgot about that. ++
We need to negate this condition as other discovery types follow the zen logic too, only single-node needs to be excluded.
* master: Clear the interrupt flag before joining Migrate to max line length of 100 Docs: Corrected path to elasticsearch-plugin (elastic#23622) Docs: Add comma to reverse nested agg snippet Fix third-party audit task for Gradle 3.4 (elastic#23612) Adapter action future should restore interrupts Update scripting.asciidoc Unmark reindex as experimental CompletionSuggestionContext#toQuery() should also consider text if prefix/regex missing (elastic#23451) Docs: Specify that byte units use powers of 1024 (elastic#23574) Remove Settings.settingsBuilder (elastic#23575) Change params._source to params['_source'] in example. Fix example in documentation for Painless using _source. (elastic#21322) Remove extra line from license header Fix num docs to be positive in bucket deferring collector test Mapping: Fix NPE with scaled floats stats when field is not indexed (elastic#23528)
* master: Eclipse: move print margin to 100 columns Add support for fragment_length in the unified highlighter (elastic#23431)
* master: (137 commits) CONSOLEify the "using scripts" documentation Adds tests for cardinality and filter aggregations Add Backoff policy to azure repository Revert "Adds tests for cardinality and filter aggregations (elastic#23826)" Adds tests for cardinality and filter aggregations (elastic#23826) mute testDifferentRolesMaintainPathOnRestart Replace custom sort field with SortedSetSortField and SortedNumericSortField when possible (elastic#23827) Prevent nodes from joining if newer indices exist in the cluster (elastic#23843) Synchronized CollapseTopFieldDocs with lucenes relatives (elastic#23854) CONSOLEify analysis docs Adapted search_shards rest test to work with Perl To examine an exception in rest tests, the exception should be caught, not ignored Fixed bad YAML in rest tests Fix BootstrapForTesting blowup Ban Boolean#getBoolean Fix language in some docs CONSOLEify lang-analyzer docs Stricter parsing of remote node attribute Fix cross-cluster remote node gateway attributes FieldCapabilitiesRequest should implements Replaceable since it accepts index patterns ...
* master: Await termination after shutting down executors Fix initialization issue in ElasticsearchException Fix bulk queue size in thread pool docs [DOCS] Remove line about eager loading global ordinals GceDiscoverTests - remove intitial_state_timeout SpecificMasterNodesIT shouldn't use autoMinMasterNodes testRestorePersistentSettings doesn't to mess with discovery settings testDifferentRolesMaintainPathOnRestart shouldn't use auto managing of min master nodes
This commit adds a single node discovery type. With this discovery type, a node will elect itself as master and never form a cluster with another node. Relates #23595
|
Thanks @bleskes. |
For the single node, dev example, the `discovery.type=single-node`[1] is a perfect fit and makes the example shorter and more self explanatory. [1] elastic#23595 [2] elastic#23598
|
I am trying to use elasticsearch version 5.6 and will need test it in a single node type. please suggest the configuration files that will require the change and get started. |
This commit adds a single node discovery type. With this discovery type, a node will elect itself as master and never form a cluster with another node.