Remove index routing table of closed indices in mixed versions clusters (#38955)

This pull request removes the legacy way of closing indices (aka "direct close") 
in mixed versions clusters, since this backward compatibility logic is not
 required anymore on master/8.0.0.

It also changes the closing logic so that routing tables of closed indices are
 removed when the cluster contains a node in version < 8.0.

Relates #33888

Author: tlrx

server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexStateService.java

@@ -223,10 +223,6 @@ static ClusterState addIndexClosedBlocks(final Index[] indices, final Map<Index,
         // Check if index closing conflicts with any running snapshots
         SnapshotsService.checkIndexClosing(currentState, indicesToClose);
 
-        // If the cluster is in a mixed version that does not support the shard close action,
-        // we use the previous way to close indices and directly close them without sanity checks
-        final boolean useDirectClose = currentState.nodes().getMinNodeVersion().before(Version.V_6_7_0);
-
         final ClusterBlocks.Builder blocks = ClusterBlocks.builder().blocks(currentState.blocks());
         final RoutingTable.Builder routingTable = RoutingTable.builder(currentState.routingTable());
 
@@ -244,19 +240,11 @@ static ClusterState addIndexClosedBlocks(final Index[] indices, final Map<Index,
                     }
                 }
             }
-            if (useDirectClose) {
-                logger.debug("closing index {} directly", index);
-                metadata.put(IndexMetaData.builder(indexToClose).state(IndexMetaData.State.CLOSE)); // increment version?
-                blocks.removeIndexBlockWithId(index.getName(), INDEX_CLOSED_BLOCK_ID);
-                routingTable.remove(index.getName());
-                indexBlock = INDEX_CLOSED_BLOCK;
-            } else {
-                if (indexBlock == null) {
-                    // Create a new index closed block
-                    indexBlock = createIndexClosingBlock();
-                }
-                assert Strings.hasLength(indexBlock.uuid()) : "Closing block should have a UUID";
+            if (indexBlock == null) {
+                // Create a new index closed block
+                indexBlock = createIndexClosingBlock();
             }
+            assert Strings.hasLength(indexBlock.uuid()) : "Closing block should have a UUID";
             blocks.addIndexBlock(index.getName(), indexBlock);
             blockedIndices.put(index, indexBlock);
         }
@@ -384,6 +372,11 @@ private void sendVerifyShardBeforeCloseRequest(final IndexShardRoutingTable shar
     static ClusterState closeRoutingTable(final ClusterState currentState,
                                           final Map<Index, ClusterBlock> blockedIndices,
                                           final Map<Index, AcknowledgedResponse> results) {
+
+        // Remove the index routing table of closed indices if the cluster is in a mixed version
+        // that does not support the replication of closed indices
+        final boolean removeRoutingTable = currentState.nodes().getMinNodeVersion().before(Version.V_8_0_0);
+
         final MetaData.Builder metadata = MetaData.builder(currentState.metaData());
         final ClusterBlocks.Builder blocks = ClusterBlocks.builder().blocks(currentState.blocks());
         final RoutingTable.Builder routingTable = RoutingTable.builder(currentState.routingTable());
@@ -413,7 +406,11 @@ static ClusterState closeRoutingTable(final ClusterState currentState,
                 metadata.put(IndexMetaData.builder(indexMetaData).state(IndexMetaData.State.CLOSE));
                 blocks.removeIndexBlockWithId(index.getName(), INDEX_CLOSED_BLOCK_ID);
                 blocks.addIndexBlock(index.getName(), INDEX_CLOSED_BLOCK);
-                routingTable.addAsFromOpenToClose(metadata.getSafe(index));
+                if (removeRoutingTable) {
+                    routingTable.remove(index.getName());
+                } else {
+                    routingTable.addAsFromOpenToClose(metadata.getSafe(index));
+                }
                 closedIndices.add(index.getName());
             } catch (final IndexNotFoundException e) {
                 logger.debug("index {} has been deleted since it was blocked before closing, ignoring", index);

server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataIndexStateServiceTests.java

@@ -115,6 +115,57 @@ public void testCloseRoutingTable() {
         }
     }
 
+    public void testCloseRoutingTableRemovesRoutingTable() {
+        final Set<Index> nonBlockedIndices = new HashSet<>();
+        final Map<Index, ClusterBlock> blockedIndices = new HashMap<>();
+        final Map<Index, AcknowledgedResponse> results = new HashMap<>();
+        final ClusterBlock closingBlock = MetaDataIndexStateService.createIndexClosingBlock();
+
+        ClusterState state = ClusterState.builder(new ClusterName("testCloseRoutingTableRemovesRoutingTable")).build();
+        for (int i = 0; i < randomIntBetween(1, 25); i++) {
+            final String indexName = "index-" + i;
+
+            if (randomBoolean()) {
+                state = addOpenedIndex(indexName, randomIntBetween(1, 5), randomIntBetween(0, 5), state);
+                nonBlockedIndices.add(state.metaData().index(indexName).getIndex());
+            } else {
+                state = addBlockedIndex(indexName, randomIntBetween(1, 5), randomIntBetween(0, 5), state, closingBlock);
+                blockedIndices.put(state.metaData().index(indexName).getIndex(), closingBlock);
+                results.put(state.metaData().index(indexName).getIndex(), new AcknowledgedResponse(randomBoolean()));
+            }
+        }
+
+        state = ClusterState.builder(state)
+            .nodes(DiscoveryNodes.builder(state.nodes())
+                .add(new DiscoveryNode("old_node", buildNewFakeTransportAddress(), emptyMap(),
+                    new HashSet<>(Arrays.asList(DiscoveryNode.Role.values())), Version.V_7_1_0))
+                .add(new DiscoveryNode("new_node", buildNewFakeTransportAddress(), emptyMap(),
+                    new HashSet<>(Arrays.asList(DiscoveryNode.Role.values())), Version.V_8_0_0)))
+            .build();
+
+        state = MetaDataIndexStateService.closeRoutingTable(state, blockedIndices, results);
+        assertThat(state.metaData().indices().size(), equalTo(nonBlockedIndices.size() + blockedIndices.size()));
+
+        for (Index nonBlockedIndex : nonBlockedIndices) {
+            assertIsOpened(nonBlockedIndex.getName(), state);
+            assertThat(state.blocks().hasIndexBlockWithId(nonBlockedIndex.getName(), INDEX_CLOSED_BLOCK_ID), is(false));
+        }
+        for (Index blockedIndex : blockedIndices.keySet()) {
+            if (results.get(blockedIndex).isAcknowledged()) {
+                assertThat(state.metaData().index(blockedIndex).getState(), is(IndexMetaData.State.CLOSE));
+                assertThat(state.blocks().hasIndexBlock(blockedIndex.getName(), MetaDataIndexStateService.INDEX_CLOSED_BLOCK), is(true));
+                assertThat("Index must have only 1 block with [id=" + MetaDataIndexStateService.INDEX_CLOSED_BLOCK_ID + "]",
+                    state.blocks().indices().getOrDefault(blockedIndex.getName(), emptySet()).stream()
+                        .filter(clusterBlock -> clusterBlock.id() == MetaDataIndexStateService.INDEX_CLOSED_BLOCK_ID).count(), equalTo(1L));
+                assertThat("Index routing table should have been removed when closing the index on mixed cluster version",
+                    state.routingTable().index(blockedIndex), nullValue());
+            } else {
+                assertIsOpened(blockedIndex.getName(), state);
+                assertThat(state.blocks().hasIndexBlock(blockedIndex.getName(), closingBlock), is(true));
+            }
+        }
+    }
+
     public void testAddIndexClosedBlocks() {
         final ClusterState initialState = ClusterState.builder(new ClusterName("testAddIndexClosedBlocks")).build();
         {
@@ -191,14 +242,6 @@ public void testAddIndexClosedBlocks() {
             ClusterState state = addOpenedIndex("index-1", randomIntBetween(1, 3), randomIntBetween(0, 3), initialState);
             state = addOpenedIndex("index-2", randomIntBetween(1, 3), randomIntBetween(0, 3), state);
             state = addOpenedIndex("index-3", randomIntBetween(1, 3), randomIntBetween(0, 3), state);
-            final boolean mixedVersions = randomBoolean();
-            if (mixedVersions) {
-                state = ClusterState.builder(state)
-                    .nodes(DiscoveryNodes.builder(state.nodes())
-                        .add(new DiscoveryNode("old_node", buildNewFakeTransportAddress(), emptyMap(),
-                             new HashSet<>(Arrays.asList(DiscoveryNode.Role.values())), Version.V_6_0_0)))
-                    .build();
-            }
 
             Index index1 = state.metaData().index("index-1").getIndex();
             Index index2 = state.metaData().index("index-2").getIndex();
@@ -210,18 +253,7 @@ public void testAddIndexClosedBlocks() {
 
             for (Index index : indices) {
                 assertTrue(blockedIndices.containsKey(index));
-                if (mixedVersions) {
-                    assertThat(updatedState.metaData().index(index).getState(), is(IndexMetaData.State.CLOSE));
-                    assertTrue(updatedState.blocks().hasIndexBlock(index.getName(), MetaDataIndexStateService.INDEX_CLOSED_BLOCK));
-                    assertThat("Index " + index + " must have only 1 block with id=" + MetaDataIndexStateService.INDEX_CLOSED_BLOCK_ID,
-                        updatedState.blocks().indices().getOrDefault(index.getName(), emptySet()).stream().filter(clusterBlock ->
-                            clusterBlock.id() == MetaDataIndexStateService.INDEX_CLOSED_BLOCK_ID).count(), equalTo(1L));
-
-                    final IndexRoutingTable indexRoutingTable = updatedState.routingTable().index(index);
-                    assertThat(indexRoutingTable, nullValue());
-                } else {
-                    assertHasBlock(index.getName(), updatedState, blockedIndices.get(index));
-                }
+                assertHasBlock(index.getName(), updatedState, blockedIndices.get(index));
             }
         }
     }