-
Notifications
You must be signed in to change notification settings - Fork 25.6k
[Zen2] Implement basic cluster formation #33668
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
[Zen2] Implement basic cluster formation #33668
Conversation
|
Pinging @elastic/es-distributed |
ywelsch
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.
Just looked at production code so far, follow-up review for tests will follow. Looks great already.
server/src/main/java/org/elasticsearch/cluster/coordination/CoordinationState.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/cluster/coordination/Coordinator.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/cluster/coordination/Coordinator.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/cluster/coordination/Coordinator.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/cluster/coordination/Coordinator.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/cluster/coordination/Coordinator.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/cluster/coordination/Coordinator.java
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/cluster/coordination/JoinHelper.java
Outdated
Show resolved
Hide resolved
| PreVoteCollector(final Settings settings, final TransportService transportService, final Runnable startElection) { | ||
| super(settings); | ||
| state = new Tuple<>(null, preVoteResponse); | ||
| state = new Tuple<>(null, new PreVoteResponse(0, 0, 0)); |
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.
what was the reason for this change?
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.
We are required to keep the PreVoteCollector abreast of changes in the current pre-vote response via the update() method, but we create it before we know what the initial response should be so have to pass in this dummy value to the constructor and call update() after loading the persistent state. On reflection we don't need to set state at all - see 98e7ad6.
server/src/main/java/org/elasticsearch/discovery/PeerFinder.java
Outdated
Show resolved
Hide resolved
ywelsch
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. Good stuff!
|
@elasticmachine test this please |
|
sigh. @elasticmachine retest this please |
|
This job triggered CI during a migration of the master. Kicking off an additional build for you manually... Jenkins, test this please. |
9a6c19d to
af8a335
Compare
|
This has passed all the relevant bits of CI and failed, again, in something unrelated, so we're merging it. |
This PR integrates the following pieces of machinery in the Coordinator:
Together, these things are everything needed to form a cluster. We therefore
also add the start of a test suite that allows us to assert higher-level
properties of the interactions between all these pieces of machinery, with as
little fake behaviour as possible. We assert one such property: "a cluster
successfully forms".