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));
  }
  }
  }