Skip to content

Commit

Permalink
Remove index routing table of closed indices in mixed versions cluste…
Browse files Browse the repository at this point in the history 
…rs (elastic#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 elastic#33888
  • Loading branch information
@tlrx
tlrx committed Mar 1, 2019
1 parent c56d9bc commit b28acaa
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 37 deletions.
31 changes: 14 additions & 17 deletions server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexStateService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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());

Expand All @@ -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);
}
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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);
Expand Down
72 changes: 52 additions & 20 deletions server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataIndexStateServiceTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
{
Expand Down Expand Up @@ -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();
Expand All @@ -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));
}
}
}
Expand Down

0 comments on commit b28acaa

Please sign in to comment.