Reset node-left generation on election (elastic#99100)

Node-left generations are only comparable within a master term and must start again from zero in the next term.
easyice · Sep 1, 2023 · 2566dc3 · 2566dc3
1 parent ebe4fe9
commit 2566dc3
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 0 deletions.
diff --git a/server/src/main/java/org/elasticsearch/cluster/coordination/NodeJoinExecutor.java b/server/src/main/java/org/elasticsearch/cluster/coordination/NodeJoinExecutor.java
@@ -267,6 +267,7 @@ protected ClusterState.Builder becomeMasterAndTrimConflictingNodes(
         DiscoveryNodes.Builder nodesBuilder = DiscoveryNodes.builder(currentNodes);
         Map<String, TransportVersion> transportVersions = new HashMap<>(getTransportVersions(currentState));
         nodesBuilder.masterNodeId(currentState.nodes().getLocalNodeId());
+        nodesBuilder.resetNodeLeftGeneration();
 
         for (final var taskContext : taskContexts) {
             for (final var joiningNode : taskContext.getTask().nodes()) {

diff --git a/server/src/main/java/org/elasticsearch/cluster/node/DiscoveryNodes.java b/server/src/main/java/org/elasticsearch/cluster/node/DiscoveryNodes.java
@@ -722,6 +722,7 @@ public static class Builder {
         private final long oldNodeLeftGeneration;
         @Nullable // if not specified
         private Long nodeLeftGeneration;
+        private boolean resetNodeLeftGeneration;
 
         public Builder() {
             nodes = new HashMap<>();
@@ -857,7 +858,10 @@ public DiscoveryNodes build() {
             } else if (this.nodeLeftGeneration != null) {
                 // only happens during deserialization
                 assert removedNode == false;
+                assert resetNodeLeftGeneration == false;
                 newNodeLeftGeneration = nodeLeftGeneration;
+            } else if (resetNodeLeftGeneration) {
+                newNodeLeftGeneration = 0L;
             } else if (removedNode) {
                 newNodeLeftGeneration = oldNodeLeftGeneration + 1L;
             } else {
@@ -909,6 +913,11 @@ void nodeLeftGeneration(long nodeLeftGeneration) {
             assert this.nodeLeftGeneration == null : nodeLeftGeneration + " vs " + this.nodeLeftGeneration;
             this.nodeLeftGeneration = nodeLeftGeneration;
         }
+
+        public void resetNodeLeftGeneration() {
+            assert this.resetNodeLeftGeneration == false;
+            this.resetNodeLeftGeneration = true;
+        }
     }
 
     private static Map<String, DiscoveryNode> filteredNodes(Map<String, DiscoveryNode> nodes, Predicate<DiscoveryNode> predicate) {

diff --git a/server/src/test/java/org/elasticsearch/cluster/coordination/NodeJoinExecutorTests.java b/server/src/test/java/org/elasticsearch/cluster/coordination/NodeJoinExecutorTests.java
@@ -57,6 +57,7 @@
 import static org.elasticsearch.test.VersionUtils.randomVersionBetween;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.greaterThan;
 import static org.hamcrest.Matchers.hasSize;
 import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.is;
@@ -699,6 +700,39 @@ public void testPerNodeLogging() {
         }
     }
 
+    public void testResetsNodeLeftGenerationOnNewTerm() throws Exception {
+        final AllocationService allocationService = createAllocationService();
+        when(allocationService.adaptAutoExpandReplicas(any())).then(invocationOnMock -> invocationOnMock.getArguments()[0]);
+        final RerouteService rerouteService = (reason, priority, listener) -> listener.onResponse(null);
+
+        final NodeJoinExecutor executor = new NodeJoinExecutor(allocationService, rerouteService);
+
+        final long term = randomLongBetween(0, Long.MAX_VALUE - 1);
+        final DiscoveryNode masterNode = DiscoveryNodeUtils.create(UUIDs.base64UUID());
+        final DiscoveryNode otherNode = DiscoveryNodeUtils.create(UUIDs.base64UUID());
+        final ClusterState clusterState = ClusterState.builder(ClusterName.DEFAULT)
+            .metadata(Metadata.builder().coordinationMetadata(CoordinationMetadata.builder().term(term).build()))
+            .nodes(DiscoveryNodes.builder().add(masterNode).localNodeId(masterNode.getId()).add(otherNode).remove(otherNode))
+            .build();
+
+        assertEquals(term, clusterState.term());
+        assertEquals(1L, clusterState.nodes().getNodeLeftGeneration());
+
+        final var resultingState = ClusterStateTaskExecutorUtils.executeAndAssertSuccessful(
+            clusterState,
+            executor,
+            List.of(
+                JoinTask.completingElection(
+                    Stream.of(new JoinTask.NodeJoinTask(otherNode, TransportVersion.current(), TEST_REASON, NOT_COMPLETED_LISTENER)),
+                    randomLongBetween(term + 1, Long.MAX_VALUE)
+                )
+            )
+        );
+
+        assertThat(resultingState.term(), greaterThan(term));
+        assertEquals(0L, resultingState.nodes().getNodeLeftGeneration());
+    }
+
     private DesiredNodeWithStatus createActualizedDesiredNode() {
         return new DesiredNodeWithStatus(randomDesiredNode(), DesiredNodeWithStatus.Status.ACTUALIZED);
     }