Skip to content

Commit f7cbcbd

Browse files
author
elasticsearchmachine
committed
[CI] Auto commit changes from spotless
1 parent d47915b commit f7cbcbd

File tree

2 files changed

+71
-74
lines changed

2 files changed

+71
-74
lines changed

server/src/internalClusterTest/java/org/elasticsearch/cluster/coordination/NodeJoiningIT.java

Lines changed: 26 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,11 @@ public void testNodeJoinsCluster() {
6363
ensureSufficientMasterEligibleNodes();
6464
String masterNodeName = internalCluster().getMasterName();
6565
int numberOfNodesOriginallyInCluster = internalCluster().clusterService(masterNodeName).state().getNodes().size();
66-
int numberOfMasterNodesOriginallyInCluster = internalCluster().clusterService(masterNodeName).state().nodes().getMasterNodes().size();
66+
int numberOfMasterNodesOriginallyInCluster = internalCluster().clusterService(masterNodeName)
67+
.state()
68+
.nodes()
69+
.getMasterNodes()
70+
.size();
6771
List<String> namesOfDataNodesInOriginalCluster = getListOfDataNodeNamesFromCluster(masterNodeName);
6872

6973
// Attempt to add new node
@@ -97,7 +101,11 @@ public void testNodeTriesToJoinClusterAndThenDifferentMasterIsElected() {
97101
final var newMasterNodeName = ensureSufficientMasterEligibleNodes();
98102
String originalMasterNodeName = internalCluster().getMasterName();
99103
int numberOfNodesOriginallyInCluster = internalCluster().clusterService(originalMasterNodeName).state().getNodes().size();
100-
int numberOfMasterNodesOriginallyInCluster = internalCluster().clusterService(originalMasterNodeName).state().nodes().getMasterNodes().size();
104+
int numberOfMasterNodesOriginallyInCluster = internalCluster().clusterService(originalMasterNodeName)
105+
.state()
106+
.nodes()
107+
.getMasterNodes()
108+
.size();
101109
List<String> namesOfDataNodesInOriginalCluster = getListOfDataNodeNamesFromCluster(originalMasterNodeName);
102110

103111
// Sets MockTransportService behaviour
@@ -133,13 +141,10 @@ public void testNodeTriesToJoinClusterAndThenDifferentMasterIsElected() {
133141

134142
// Wait until the old master has acknowledged the new master's election
135143
ClusterService originalMasterClusterService = internalCluster().getInstance(ClusterService.class, originalMasterNodeName);
136-
ClusterServiceUtils.addTemporaryStateListener(
137-
originalMasterClusterService,
138-
clusterState -> {
139-
DiscoveryNode currentMasterNode = clusterState.nodes().getMasterNode();
140-
return currentMasterNode != null && currentMasterNode.getName().equals(newMasterNodeName);
141-
}
142-
);
144+
ClusterServiceUtils.addTemporaryStateListener(originalMasterClusterService, clusterState -> {
145+
DiscoveryNode currentMasterNode = clusterState.nodes().getMasterNode();
146+
return currentMasterNode != null && currentMasterNode.getName().equals(newMasterNodeName);
147+
});
143148
assertNotEquals(originalMasterNodeName, internalCluster().getMasterName());
144149
logger.info("New master is elected");
145150

@@ -184,7 +189,11 @@ public void testNodeTriesToJoinClusterAndThenSameMasterIsElected() {
184189

185190
long originalTerm = internalCluster().clusterService(masterNodeName).state().coordinationMetadata().term();
186191
int numberOfNodesOriginallyInCluster = internalCluster().clusterService(masterNodeName).state().getNodes().size();
187-
int numberOfMasterNodesOriginallyInCluster = internalCluster().clusterService(masterNodeName).state().nodes().getMasterNodes().size();
192+
int numberOfMasterNodesOriginallyInCluster = internalCluster().clusterService(masterNodeName)
193+
.state()
194+
.nodes()
195+
.getMasterNodes()
196+
.size();
188197
List<String> namesOfDataNodesInOriginalCluster = getListOfDataNodeNamesFromCluster(masterNodeName);
189198
String[] namesOfAllNodesInOriginalCluster = internalCluster().getNodeNames();
190199

@@ -250,14 +259,11 @@ public void testNodeTriesToJoinClusterAndThenSameMasterIsElected() {
250259

251260
// Wait until the master acknowledges its re-election. The master is only re-elected once it's publishing ban is lifted
252261
ClusterService masterClusterService = internalCluster().getInstance(ClusterService.class, masterNodeName);
253-
ClusterServiceUtils.addTemporaryStateListener(
254-
masterClusterService,
255-
clusterState -> {
256-
DiscoveryNode currentMasterNode = clusterState.nodes().getMasterNode();
257-
long currentTerm = clusterState.coordinationMetadata().term();
258-
return currentMasterNode != null && currentMasterNode.getName().equals(masterNodeName) && currentTerm > originalTerm;
259-
}
260-
);
262+
ClusterServiceUtils.addTemporaryStateListener(masterClusterService, clusterState -> {
263+
DiscoveryNode currentMasterNode = clusterState.nodes().getMasterNode();
264+
long currentTerm = clusterState.coordinationMetadata().term();
265+
return currentMasterNode != null && currentMasterNode.getName().equals(masterNodeName) && currentTerm > originalTerm;
266+
});
261267
assertEquals(masterNodeName, internalCluster().getMasterName());
262268
logger.info("Master has been re-elected");
263269

@@ -266,7 +272,7 @@ public void testNodeTriesToJoinClusterAndThenSameMasterIsElected() {
266272
for (String nodeName : namesOfAllNodesInOriginalCluster) {
267273
ClusterServiceUtils.awaitClusterState(
268274
logger,
269-
clusterState -> nodeExistsWithName(clusterState.nodes(),newNodeName),
275+
clusterState -> nodeExistsWithName(clusterState.nodes(), newNodeName),
270276
internalCluster().clusterService(nodeName)
271277
);
272278
}
@@ -416,7 +422,7 @@ protected CountDownLatch removeMockTransportServicePublishBanWhenMasterHasSteppe
416422
*/
417423
protected static String ensureSufficientMasterEligibleNodes() {
418424
final var votingConfigSizeListener = ClusterServiceUtils.addTemporaryStateListener(
419-
cs -> 3 <= cs.coordinationMetadata().getLastCommittedConfiguration().getNodeIds().size()
425+
cs -> 3 <= cs.coordinationMetadata().getLastCommittedConfiguration().getNodeIds().size()
420426
);
421427

422428
try {

server/src/main/java/org/elasticsearch/cluster/coordination/Coordinator.java

Lines changed: 45 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -669,65 +669,56 @@ private void handleJoinRequest(JoinRequest joinRequest, ActionListener<Void> joi
669669
transportService.connectToNode(joinRequest.getSourceNode(), new ActionListener<>() {
670670
@Override
671671
public void onResponse(Releasable response) {
672-
validateJoinRequest(
673-
joinRequest,
674-
new ActionListener<>() {
675-
@Override
676-
public void onResponse(Void unused) {
677-
processJoinRequest(
678-
joinRequest,
679-
ActionListener.runBefore(
680-
joinListener,
681-
() -> Releasables.close(response)
682-
)
683-
);
684-
}
672+
validateJoinRequest(joinRequest, new ActionListener<>() {
673+
@Override
674+
public void onResponse(Void unused) {
675+
processJoinRequest(joinRequest, ActionListener.runBefore(joinListener, () -> Releasables.close(response)));
676+
}
685677

686-
/*
687-
This prevents a corner case, explained in #ES-11449, occurring as follows:
688-
- Master M is in term T and has cluster state (T, V).
689-
- Node N tries to join the cluster.
690-
- M proposes cluster state (T, V+1) with N in the cluster.
691-
- M accepts its own proposal and commits it to disk.
692-
- M receives no responses. M doesn't know whether the state was accepted by a majority of nodes,
693-
rejected, or did not reach any nodes.
694-
- There is a re-election and M wins. M publishes cluster state (T+1, V+2).
695-
Since it's built from the cluster state on disk, N is still in the cluster.
696-
- Since (T, V+1) failed, a FailedToCommitClusterStateException is thrown and N's connection is dropped,
697-
even though its inclusion in the cluster may have been committed on a majority of master nodes.
698-
- It can rejoin, but this throws a WARN log since it did not restart.
699-
700-
To mitigate this, we optionally listen for the next committed cluster state update:
701-
1. (T, V+1) is accepted -> NodeConnectionsService now stores an open connection to N.
702-
The connection can be closed as soon as the node has joined. This is handled by onResponse above.
703-
2. (T, V+1) is rejected -> A new cluster state is published without N in it
704-
It is right to close the connection and retry. This is handled by onResponse above.
705-
3. The above scenario occurs, and a FailedToCommitClusterStateException is thrown for state (T, V+1).
706-
Now, we keep the connection open until the next committed cluster state, rather than disconnecting:
707-
3.1 (T+1, V+2) is accepted -> By waiting, we did not close the connection to N unnecessarily
708-
3.2 (T+1, V+2) is rejected -> A new cluster state is published without N in it. Closing is correct here.
709-
*/
710-
@Override
711-
public void onFailure(Exception e) {
712-
if (e instanceof FailedToCommitClusterStateException) {
713-
ClusterStateListener clusterStateListener = new ClusterStateListener() {
714-
@Override
715-
public void clusterChanged(ClusterChangedEvent event) {
716-
// Keep the connection open until the next committed state
717-
if (event.state().nodes().getMasterNode() != null) {
718-
Releasables.close(response);
719-
// Remove this listener to avoid memory leaks
720-
clusterService.removeListener(this);
721-
}
678+
/*
679+
This prevents a corner case, explained in #ES-11449, occurring as follows:
680+
- Master M is in term T and has cluster state (T, V).
681+
- Node N tries to join the cluster.
682+
- M proposes cluster state (T, V+1) with N in the cluster.
683+
- M accepts its own proposal and commits it to disk.
684+
- M receives no responses. M doesn't know whether the state was accepted by a majority of nodes,
685+
rejected, or did not reach any nodes.
686+
- There is a re-election and M wins. M publishes cluster state (T+1, V+2).
687+
Since it's built from the cluster state on disk, N is still in the cluster.
688+
- Since (T, V+1) failed, a FailedToCommitClusterStateException is thrown and N's connection is dropped,
689+
even though its inclusion in the cluster may have been committed on a majority of master nodes.
690+
- It can rejoin, but this throws a WARN log since it did not restart.
691+
692+
To mitigate this, we optionally listen for the next committed cluster state update:
693+
1. (T, V+1) is accepted -> NodeConnectionsService now stores an open connection to N.
694+
The connection can be closed as soon as the node has joined. This is handled by onResponse above.
695+
2. (T, V+1) is rejected -> A new cluster state is published without N in it
696+
It is right to close the connection and retry. This is handled by onResponse above.
697+
3. The above scenario occurs, and a FailedToCommitClusterStateException is thrown for state (T, V+1).
698+
Now, we keep the connection open until the next committed cluster state, rather than disconnecting:
699+
3.1 (T+1, V+2) is accepted -> By waiting, we did not close the connection to N unnecessarily
700+
3.2 (T+1, V+2) is rejected -> A new cluster state is published without N in it. Closing is correct here.
701+
*/
702+
@Override
703+
public void onFailure(Exception e) {
704+
if (e instanceof FailedToCommitClusterStateException) {
705+
ClusterStateListener clusterStateListener = new ClusterStateListener() {
706+
@Override
707+
public void clusterChanged(ClusterChangedEvent event) {
708+
// Keep the connection open until the next committed state
709+
if (event.state().nodes().getMasterNode() != null) {
710+
Releasables.close(response);
711+
// Remove this listener to avoid memory leaks
712+
clusterService.removeListener(this);
722713
}
723-
};
714+
}
715+
};
724716

725-
clusterService.addListener(clusterStateListener);
726-
}
727-
joinListener.onFailure(e);
717+
clusterService.addListener(clusterStateListener);
728718
}
719+
joinListener.onFailure(e);
729720
}
730-
);
721+
});
731722
}
732723

733724
@Override

0 commit comments

Comments
 (0)