Skip to content

Commit 89f06d5

Browse files
author
elasticsearchmachine
committed
[CI] Auto commit changes from spotless
1 parent dac102a commit 89f06d5

File tree

6 files changed

+124
-102
lines changed

6 files changed

+124
-102
lines changed

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,11 @@ protected CountDownLatch configureElectionLatchForNewMaster(String newMaster, Li
7777
* @param cleanupTasks The list of cleanup tasks
7878
* @return A latch that will be released when the master acknowledges it's re-election
7979
*/
80-
protected CountDownLatch configureElectionLatchForReElectedMaster(String masterNodeName, long originalTerm, List<Releasable> cleanupTasks) {
80+
protected CountDownLatch configureElectionLatchForReElectedMaster(
81+
String masterNodeName,
82+
long originalTerm,
83+
List<Releasable> cleanupTasks
84+
) {
8185
final var masterKnowsItIsReElectedLatch = new CountDownLatch(1);
8286
ClusterStateApplier newMasterMonitor = event -> {
8387
DiscoveryNode masterNode = event.state().nodes().getMasterNode();

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

Lines changed: 75 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -41,10 +41,7 @@ public class NodeJoiningIT extends MasterElectionTestCase {
4141

4242
@Override
4343
protected Collection<Class<? extends Plugin>> nodePlugins() {
44-
return CollectionUtils.appendToCopyNoNullElements(
45-
super.nodePlugins(),
46-
MockTransportService.TestPlugin.class
47-
);
44+
return CollectionUtils.appendToCopyNoNullElements(super.nodePlugins(), MockTransportService.TestPlugin.class);
4845
}
4946

5047
@Override
@@ -123,11 +120,10 @@ public void testNodeTriesToJoinClusterAndThenDifferentMasterIsElected() {
123120
if (mockTransportService.getLocalNode().getName().equals(newMasterNodeName) == false) {
124121
mockTransportService.addSendBehavior((connection, requestId, action, request, options) -> {
125122
if (
126-
// This disables pre-voting on all nodes except the new master, forcing it to win the election
127-
action.equals(StatefulPreVoteCollector.REQUEST_PRE_VOTE_ACTION_NAME)
123+
// This disables pre-voting on all nodes except the new master, forcing it to win the election
124+
action.equals(StatefulPreVoteCollector.REQUEST_PRE_VOTE_ACTION_NAME)
128125
// This forces the current master node to fail
129-
|| action.equals(PublicationTransportHandler.PUBLISH_STATE_ACTION_NAME)
130-
) {
126+
|| action.equals(PublicationTransportHandler.PUBLISH_STATE_ACTION_NAME)) {
131127
throw new ElasticsearchException("[{}] for [{}] denied", action, connection.getNode());
132128
} else {
133129
connection.sendRequest(requestId, action, request, options);
@@ -179,8 +175,8 @@ public void testNodeTriesToJoinClusterAndThenDifferentMasterIsElected() {
179175

180176
// Tests whether a WARN log is thrown when a node attempts to join a cluster, and then the same master node is re-elected (#126192)
181177
@TestLogging(
182-
reason = "test includes assertions about logging",
183-
value = "org.elasticsearch.cluster.coordination.NodeJoinExecutor:WARN,org.elasticsearch.cluster.coordination.NodeJoinExecutor:INFO,org.elasticsearch.cluster.coordination.MasterService:WARN,org.elasticsearch.cluster.coordination.MasterService:INFO,org.elasticsearch.cluster.coordination.ClusterApplierService:WARN"
178+
reason = "test includes assertions about logging",
179+
value = "org.elasticsearch.cluster.coordination.NodeJoinExecutor:WARN,org.elasticsearch.cluster.coordination.NodeJoinExecutor:INFO,org.elasticsearch.cluster.coordination.MasterService:WARN,org.elasticsearch.cluster.coordination.MasterService:INFO,org.elasticsearch.cluster.coordination.ClusterApplierService:WARN"
184180
)
185181
public void testNodeTriesToJoinClusterAndThenSameMasterIsElected_DoesNotIncludeWarnLog() {
186182
final var cleanupTasks = new ArrayList<Releasable>();
@@ -210,10 +206,18 @@ public void testNodeTriesToJoinClusterAndThenSameMasterIsElected_DoesNotIncludeW
210206
});
211207

212208
// Latch to remove publishing ban to allow re-election
213-
CountDownLatch publishingBanRemovedLatch = removeMockTransportServicePublishBanWhenMasterHasSteppedDown(masterNodeName, masterNodeTransportService, cleanupTasks);
209+
CountDownLatch publishingBanRemovedLatch = removeMockTransportServicePublishBanWhenMasterHasSteppedDown(
210+
masterNodeName,
211+
masterNodeTransportService,
212+
cleanupTasks
213+
);
214214

215215
// A CountDownLatch that only gets decremented when the first master node is re-elected
216-
final var masterKnowsItHasBeenReElectedLatch = configureElectionLatchForReElectedMaster(masterNodeName, originalTerm, cleanupTasks);
216+
final var masterKnowsItHasBeenReElectedLatch = configureElectionLatchForReElectedMaster(
217+
masterNodeName,
218+
originalTerm,
219+
cleanupTasks
220+
);
217221

218222
for (String nodeName : internalCluster().getNodeNames()) {
219223
final var mockTransportService = MockTransportService.getInstance(nodeName);
@@ -280,9 +284,9 @@ public void testNodeTriesToJoinClusterAndThenSameMasterIsElected_DoesNotIncludeW
280284
// Await for N to be in the cluster state of all nodes
281285
for (String nodeName : namesOfAllNodesInOriginalCluster) {
282286
ClusterServiceUtils.awaitClusterState(
283-
logger,
284-
clusterState -> clusterState.nodes().nodeExistsWithName(newNodeName),
285-
internalCluster().clusterService(nodeName)
287+
logger,
288+
clusterState -> clusterState.nodes().nodeExistsWithName(newNodeName),
289+
internalCluster().clusterService(nodeName)
286290
);
287291
}
288292
} catch (Exception e) {
@@ -309,10 +313,8 @@ public void testNodeTriesToJoinClusterAndThenSameMasterIsElected_DoesNotIncludeW
309313
}
310314
}
311315

312-
313-
private List<String> getListOfDataNodeNamesFromCluster(String nodeName){
314-
return internalCluster()
315-
.clusterService(nodeName)
316+
private List<String> getListOfDataNodeNamesFromCluster(String nodeName) {
317+
return internalCluster().clusterService(nodeName)
316318
.state()
317319
.getNodes()
318320
.getDataNodes()
@@ -336,12 +338,7 @@ public void match(LogEvent event) {
336338
return;
337339
}
338340

339-
Pattern pattern = Pattern.compile(
340-
"node-join: \\["
341-
+ expectedNewNodeAsString
342-
+ "] "
343-
+ "with reason \\[joining]"
344-
);
341+
Pattern pattern = Pattern.compile("node-join: \\[" + expectedNewNodeAsString + "] " + "with reason \\[joining]");
345342
Matcher matcher = pattern.matcher(event.getMessage().getFormattedMessage());
346343

347344
if (matcher.find()) {
@@ -370,11 +367,15 @@ public void match(LogEvent event) {
370367
}
371368

372369
Pattern pattern = Pattern.compile(
373-
"node-join\\["
370+
"node-join\\["
374371
+ expectedNewNodeAsString
375372
+ " joining],"
376-
+ " term: " + term + ","
377-
+ " version: " + version + ","
373+
+ " term: "
374+
+ term
375+
+ ","
376+
+ " version: "
377+
+ version
378+
+ ","
378379
+ " delta: added \\{"
379380
+ expectedNewNodeAsString
380381
+ "}"
@@ -407,9 +408,11 @@ public void match(LogEvent event) {
407408
}
408409

409410
Pattern pattern = Pattern.compile(
410-
"failing \\[node-join\\["
411-
+ expectedNewNodeAsString
412-
+ " joining]]: failed to commit cluster state version \\[" + version +"]"
411+
"failing \\[node-join\\["
412+
+ expectedNewNodeAsString
413+
+ " joining]]: failed to commit cluster state version \\["
414+
+ version
415+
+ "]"
413416
);
414417
Matcher matcher = pattern.matcher(event.getMessage().getFormattedMessage());
415418

@@ -440,13 +443,13 @@ public void match(LogEvent event) {
440443

441444
String regexToMatchAnyCharacterExceptClosingBrace = "([^}]+)";
442445
Pattern pattern = Pattern.compile(
443-
"node-join: \\["
444-
+ expectedNewNodeAsString
445-
+ "] "
446-
+ "with reason \\[joining, removed \\["
447-
+ regexToMatchAnyCharacterExceptClosingBrace
448-
+ "] ago with reason \\[disconnected]]; "
449-
+ "for troubleshooting guidance, see https://www.elastic.co/docs/troubleshoot/elasticsearch/troubleshooting-unstable-cluster\\?version=master"
446+
"node-join: \\["
447+
+ expectedNewNodeAsString
448+
+ "] "
449+
+ "with reason \\[joining, removed \\["
450+
+ regexToMatchAnyCharacterExceptClosingBrace
451+
+ "] ago with reason \\[disconnected]]; "
452+
+ "for troubleshooting guidance, see https://www.elastic.co/docs/troubleshoot/elasticsearch/troubleshooting-unstable-cluster\\?version=master"
450453
);
451454
Matcher matcher = pattern.matcher(event.getMessage().getFormattedMessage());
452455

@@ -462,7 +465,10 @@ public void assertMatched() {
462465
});
463466
}
464467

465-
private void addNodeJoinProcessedDuringNewElectionAndClusterStatePublicationExpectation(MockLog mockLog, String expectedNewNodeAsString) {
468+
private void addNodeJoinProcessedDuringNewElectionAndClusterStatePublicationExpectation(
469+
MockLog mockLog,
470+
String expectedNewNodeAsString
471+
) {
466472
mockLog.addExpectation(new MockLog.LoggingExpectation() {
467473
boolean matched = false;
468474

@@ -475,11 +481,7 @@ public void match(LogEvent event) {
475481
return;
476482
}
477483

478-
Pattern pattern = Pattern.compile(
479-
"added \\{"
480-
+ expectedNewNodeAsString
481-
+ "}"
482-
);
484+
Pattern pattern = Pattern.compile("added \\{" + expectedNewNodeAsString + "}");
483485
Matcher matcher = pattern.matcher(event.getMessage().getFormattedMessage());
484486

485487
if (matcher.find()) {
@@ -500,15 +502,31 @@ private String generateNodeDescriptionForNewDiscoveryNode(int numberOfNodesOrigi
500502
String newNodeName = "node_s" + numberOfNodesOriginallyInCluster;
501503
String regexToMatchAnyCharacterExceptClosingBrace = "([^}]+)";
502504

503-
return "\\{" + newNodeName + "}"
504-
+ "\\{" + regexToMatchAnyCharacterExceptClosingBrace + "}"
505-
+ "\\{" + regexToMatchAnyCharacterExceptClosingBrace + "}"
506-
+ "\\{" + newNodeName + "}"
507-
+ "\\{" + masterNode.getHostAddress() + "}"
508-
+ "\\{" + masterNode.getHostAddress() + ":\\d+}"
505+
return "\\{"
506+
+ newNodeName
507+
+ "}"
508+
+ "\\{"
509+
+ regexToMatchAnyCharacterExceptClosingBrace
510+
+ "}"
511+
+ "\\{"
512+
+ regexToMatchAnyCharacterExceptClosingBrace
513+
+ "}"
514+
+ "\\{"
515+
+ newNodeName
516+
+ "}"
517+
+ "\\{"
518+
+ masterNode.getHostAddress()
519+
+ "}"
520+
+ "\\{"
521+
+ masterNode.getHostAddress()
522+
+ ":\\d+}"
509523
+ "\\{d}"
510-
+ "\\{" + masterNode.getVersion() + "}"
511-
+ "\\{" + regexToMatchAnyCharacterExceptClosingBrace + "}";
524+
+ "\\{"
525+
+ masterNode.getVersion()
526+
+ "}"
527+
+ "\\{"
528+
+ regexToMatchAnyCharacterExceptClosingBrace
529+
+ "}";
512530
}
513531

514532
/**
@@ -518,7 +536,11 @@ private String generateNodeDescriptionForNewDiscoveryNode(int numberOfNodesOrigi
518536
* @param mockTransportService The transport service to remove the `addSendBehavior` from
519537
* @param cleanupTasks The list of cleanup tasks
520538
*/
521-
protected CountDownLatch removeMockTransportServicePublishBanWhenMasterHasSteppedDown(String masterNodeName, MockTransportService mockTransportService, List<Releasable> cleanupTasks) {
539+
protected CountDownLatch removeMockTransportServicePublishBanWhenMasterHasSteppedDown(
540+
String masterNodeName,
541+
MockTransportService mockTransportService,
542+
List<Releasable> cleanupTasks
543+
) {
522544
CountDownLatch latch = new CountDownLatch(1);
523545
ClusterStateApplier newMasterMonitor = event -> {
524546
DiscoveryNode masterNode = event.state().nodes().getMasterNode();

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

Lines changed: 41 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -669,48 +669,48 @@ 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-
ActionListener
675-
.runBefore(
676-
joinListener,
677-
() -> {
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, rejected, or did not reach any nodes.
685-
- There is a re-election and M wins. M publishes cluster state (T+1, V+2).
686-
Since it's built from the cluster state on disk, N is still in the cluster.
687-
- Since (T, V+1) failed, N's connection is dropped, even though its inclusion in the cluster may have been committed on a majority of master nodes.
688-
- It can rejoin, but this throws a WARN log since it did not restart.
689-
690-
To mitigate this, we listen for any cluster state update:
691-
1. (T, V+1) is accepted -> NodeConnectionsService now stores an open connection to N. It can be closed.
692-
2. (T, V+1) is rejected -> A new cluster state is published without N in it. It is right to close the connection and retry.
693-
3. The above scenario occurs. We do not close the connection after (T, V+1) fails and keep it open:
694-
3.1 (T+1, V+2) is accepted -> By waiting, we did not close the connection to N unnecessarily
695-
3.2 (T+1, V+2) is rejected -> A new cluster state is published without N in it. Closing is correct here.
696-
*/
697-
logger.info("inside callback, node is is {}, source node is {}", clusterService.state().nodes().getLocalNode().getName(), joinRequest.getSourceNode().getName());
698-
ClusterStateListener listener = new ClusterStateListener() {
699-
@Override
700-
public void clusterChanged(ClusterChangedEvent event) {
701-
logger.info("inside cluster change event, source node is {}, added nodes are {}", joinRequest.getSourceNode().getName(), event.nodesDelta().addedNodes());
702-
// Now it's safe to close the connection
703-
Releasables.close(response);
704-
// Remove this listener to avoid memory leaks
705-
clusterService.removeListener(this);
706-
}
707-
};
672+
validateJoinRequest(joinRequest, ActionListener.runBefore(joinListener, () -> {
673+
/*
674+
This prevents a corner case, explained in #ES-11449, occurring as follows:
675+
- Master M is in term T and has cluster state (T, V).
676+
- Node N tries to join the cluster.
677+
- M proposes cluster state (T, V+1) with N in the cluster.
678+
- M accepts its own proposal and commits it to disk.
679+
- M receives no responses. M doesn't know whether the state was accepted by a majority of nodes, rejected, or did not reach any nodes.
680+
- There is a re-election and M wins. M publishes cluster state (T+1, V+2).
681+
Since it's built from the cluster state on disk, N is still in the cluster.
682+
- Since (T, V+1) failed, N's connection is dropped, even though its inclusion in the cluster may have been committed on a majority of master nodes.
683+
- It can rejoin, but this throws a WARN log since it did not restart.
684+
685+
To mitigate this, we listen for any cluster state update:
686+
1. (T, V+1) is accepted -> NodeConnectionsService now stores an open connection to N. It can be closed.
687+
2. (T, V+1) is rejected -> A new cluster state is published without N in it. It is right to close the connection and retry.
688+
3. The above scenario occurs. We do not close the connection after (T, V+1) fails and keep it open:
689+
3.1 (T+1, V+2) is accepted -> By waiting, we did not close the connection to N unnecessarily
690+
3.2 (T+1, V+2) is rejected -> A new cluster state is published without N in it. Closing is correct here.
691+
*/
692+
logger.info(
693+
"inside callback, node is is {}, source node is {}",
694+
clusterService.state().nodes().getLocalNode().getName(),
695+
joinRequest.getSourceNode().getName()
696+
);
697+
ClusterStateListener listener = new ClusterStateListener() {
698+
@Override
699+
public void clusterChanged(ClusterChangedEvent event) {
700+
logger.info(
701+
"inside cluster change event, source node is {}, added nodes are {}",
702+
joinRequest.getSourceNode().getName(),
703+
event.nodesDelta().addedNodes()
704+
);
705+
// Now it's safe to close the connection
706+
Releasables.close(response);
707+
// Remove this listener to avoid memory leaks
708+
clusterService.removeListener(this);
709+
}
710+
};
708711

709-
clusterService.addListener(listener);
710-
}
711-
)
712-
.delegateFailure((l, ignored) -> processJoinRequest(joinRequest, l))
713-
);
712+
clusterService.addListener(listener);
713+
}).delegateFailure((l, ignored) -> processJoinRequest(joinRequest, l)));
714714
}
715715

716716
@Override

0 commit comments

Comments
 (0)