From c484c66f3f7180e97de7a413a2cee80b9baa62b7 Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Mon, 18 Feb 2019 20:20:55 +0100 Subject: [PATCH] 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 --- .../metadata/MetaDataIndexStateService.java | 31 ++++---- .../MetaDataIndexStateServiceTests.java | 72 +++++++++++++------ 2 files changed, 66 insertions(+), 37 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexStateService.java b/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexStateService.java index 3e9143320c53c..ca58b49e51860 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexStateService.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexStateService.java @@ -223,10 +223,6 @@ static ClusterState addIndexClosedBlocks(final Index[] indices, final Map blockedIndices, final Map 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); diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataIndexStateServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataIndexStateServiceTests.java index 4108c542d0cc5..df07982f445ba 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataIndexStateServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataIndexStateServiceTests.java @@ -115,6 +115,57 @@ public void testCloseRoutingTable() { } } + public void testCloseRoutingTableRemovesRoutingTable() { + final Set nonBlockedIndices = new HashSet<>(); + final Map blockedIndices = new HashMap<>(); + final Map 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)); } } }