diff --git a/server/src/main/java/org/elasticsearch/gateway/ClusterStateUpdaters.java b/server/src/main/java/org/elasticsearch/gateway/ClusterStateUpdaters.java index 056919ab1c4be..74eb3e08f002f 100644 --- a/server/src/main/java/org/elasticsearch/gateway/ClusterStateUpdaters.java +++ b/server/src/main/java/org/elasticsearch/gateway/ClusterStateUpdaters.java @@ -31,8 +31,6 @@ import org.elasticsearch.cluster.node.DiscoveryNodes; import org.elasticsearch.cluster.routing.RoutingTable; import org.elasticsearch.common.settings.ClusterSettings; -import org.elasticsearch.index.Index; -import org.elasticsearch.indices.IndicesService; import java.util.Map; @@ -92,26 +90,6 @@ static ClusterState recoverClusterBlocks(final ClusterState state) { return ClusterState.builder(state).blocks(blocks).build(); } - static ClusterState closeBadIndices(final ClusterState clusterState, final IndicesService indicesService) { - final MetaData.Builder builder = MetaData.builder(clusterState.metaData()).removeAllIndices(); - - for (IndexMetaData metaData : clusterState.metaData()) { - try { - if (metaData.getState() == IndexMetaData.State.OPEN) { - // verify that we can actually create this index - if not we recover it as closed with lots of warn logs - indicesService.verifyIndexMetadata(metaData, metaData); - } - } catch (final Exception e) { - final Index electedIndex = metaData.getIndex(); - logger.warn(() -> new ParameterizedMessage("recovering index {} failed - recovering as closed", electedIndex), e); - metaData = IndexMetaData.builder(metaData).state(IndexMetaData.State.CLOSE).build(); - } - builder.put(metaData, false); - } - - return ClusterState.builder(clusterState).metaData(builder).build(); - } - static ClusterState updateRoutingTable(final ClusterState state) { // initialize all index routing tables as empty final RoutingTable.Builder routingTableBuilder = RoutingTable.builder(state.routingTable()); diff --git a/server/src/main/java/org/elasticsearch/gateway/Gateway.java b/server/src/main/java/org/elasticsearch/gateway/Gateway.java index cffb672f0cfda..317bf63a4a651 100644 --- a/server/src/main/java/org/elasticsearch/gateway/Gateway.java +++ b/server/src/main/java/org/elasticsearch/gateway/Gateway.java @@ -126,7 +126,6 @@ public void performStateRecovery(final GatewayStateRecoveredListener listener) t } ClusterState recoveredState = Function.identity() .andThen(state -> ClusterStateUpdaters.upgradeAndArchiveUnknownOrInvalidSettings(state, clusterService.getClusterSettings())) - .andThen(state -> ClusterStateUpdaters.closeBadIndices(state, indicesService)) .apply(ClusterState.builder(clusterService.getClusterName()).metaData(metaDataBuilder).build()); listener.onSuccess(recoveredState); diff --git a/server/src/main/java/org/elasticsearch/gateway/GatewayMetaState.java b/server/src/main/java/org/elasticsearch/gateway/GatewayMetaState.java index f14d86c7602bf..bd6fd908d49ab 100644 --- a/server/src/main/java/org/elasticsearch/gateway/GatewayMetaState.java +++ b/server/src/main/java/org/elasticsearch/gateway/GatewayMetaState.java @@ -137,7 +137,6 @@ public void applyClusterStateUpdaters() { .andThen(ClusterStateUpdaters::addStateNotRecoveredBlock) .andThen(state -> ClusterStateUpdaters.setLocalNode(state, transportService.getLocalNode())) .andThen(state -> ClusterStateUpdaters.upgradeAndArchiveUnknownOrInvalidSettings(state, clusterService.getClusterSettings())) - .andThen(state -> ClusterStateUpdaters.closeBadIndices(state, indicesService)) .andThen(ClusterStateUpdaters::recoverClusterBlocks) .apply(previousClusterState); } diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/create/CreateIndexIT.java b/server/src/test/java/org/elasticsearch/action/admin/indices/create/CreateIndexIT.java index 05da57cc5da45..f23dbaa8ea413 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/create/CreateIndexIT.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/create/CreateIndexIT.java @@ -34,6 +34,10 @@ import org.elasticsearch.cluster.metadata.MappingMetaData; import org.elasticsearch.cluster.metadata.MetaData; import org.elasticsearch.cluster.node.DiscoveryNode; +import org.elasticsearch.cluster.routing.IndexRoutingTable; +import org.elasticsearch.cluster.routing.IndexShardRoutingTable; +import org.elasticsearch.cluster.routing.RoutingTable; +import org.elasticsearch.cluster.routing.UnassignedInfo; import org.elasticsearch.common.collect.ImmutableOpenMap; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.TimeValue; @@ -50,6 +54,7 @@ import java.util.HashSet; import java.util.Set; import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.BiFunction; @@ -60,6 +65,7 @@ import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.greaterThanOrEqualTo; import static org.hamcrest.Matchers.hasToString; import static org.hamcrest.Matchers.instanceOf; @@ -419,11 +425,20 @@ public Settings onNodeStopped(String nodeName) throws Exception { return super.onNodeStopped(nodeName); } }); - ensureGreen(metaData.getIndex().getName()); // we have to wait for the index to show up in the metadata or we will fail in a race - final ClusterState stateAfterRestart = client().admin().cluster().prepareState().get().getState(); - // the index should not be open after we restart and recover the broken index metadata - assertThat(stateAfterRestart.getMetaData().index(metaData.getIndex()).getState(), equalTo(IndexMetaData.State.CLOSE)); + // check that the cluster does not keep reallocating shards + assertBusy(() -> { + final RoutingTable routingTable = client().admin().cluster().prepareState().get().getState().routingTable(); + final IndexRoutingTable indexRoutingTable = routingTable.index("test"); + assertNotNull(indexRoutingTable); + for (IndexShardRoutingTable shardRoutingTable : indexRoutingTable) { + assertTrue(shardRoutingTable.primaryShard().unassigned()); + assertEquals(UnassignedInfo.AllocationStatus.DECIDERS_NO, + shardRoutingTable.primaryShard().unassignedInfo().getLastAllocationStatus()); + assertThat(shardRoutingTable.primaryShard().unassignedInfo().getNumFailedAllocations(), greaterThan(0)); + } + }, 60, TimeUnit.SECONDS); + client().admin().indices().prepareClose("test").get(); // try to open the index final ElasticsearchException e = diff --git a/server/src/test/java/org/elasticsearch/gateway/ClusterStateUpdatersTests.java b/server/src/test/java/org/elasticsearch/gateway/ClusterStateUpdatersTests.java index cae33db90a6bc..999d80586fea4 100644 --- a/server/src/test/java/org/elasticsearch/gateway/ClusterStateUpdatersTests.java +++ b/server/src/test/java/org/elasticsearch/gateway/ClusterStateUpdatersTests.java @@ -33,10 +33,8 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.set.Sets; import org.elasticsearch.index.Index; -import org.elasticsearch.indices.IndicesService; import org.elasticsearch.test.ESTestCase; -import java.io.IOException; import java.util.Arrays; import java.util.Collections; import java.util.Set; @@ -47,7 +45,6 @@ import static org.elasticsearch.cluster.metadata.MetaData.CLUSTER_READ_ONLY_BLOCK; import static org.elasticsearch.gateway.ClusterStateUpdaters.addStateNotRecoveredBlock; -import static org.elasticsearch.gateway.ClusterStateUpdaters.closeBadIndices; import static org.elasticsearch.gateway.ClusterStateUpdaters.hideStateIfNotRecovered; import static org.elasticsearch.gateway.ClusterStateUpdaters.mixCurrentStateAndRecoveredState; import static org.elasticsearch.gateway.ClusterStateUpdaters.recoverClusterBlocks; @@ -59,8 +56,6 @@ import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; -import static org.mockito.Mockito.doThrow; -import static org.mockito.Mockito.mock; public class ClusterStateUpdatersTests extends ESTestCase { @@ -201,32 +196,6 @@ public void testAddStateNotRecoveredBlock() { assertTrue(newState.blocks().hasGlobalBlock(STATE_NOT_RECOVERED_BLOCK)); } - public void testCloseBadIndices() throws IOException { - final IndicesService indicesService = mock(IndicesService.class); - final IndexMetaData good = createIndexMetaData("good", Settings.EMPTY); - final IndexMetaData bad = createIndexMetaData("bad", Settings.EMPTY); - final IndexMetaData ugly = IndexMetaData.builder(createIndexMetaData("ugly", Settings.EMPTY)) - .state(IndexMetaData.State.CLOSE) - .build(); - - final ClusterState initialState = ClusterState - .builder(ClusterState.EMPTY_STATE) - .metaData(MetaData.builder() - .put(good, false) - .put(bad, false) - .put(ugly, false) - .build()) - .build(); - - doThrow(new RuntimeException("test")).when(indicesService).verifyIndexMetadata(bad, bad); - doThrow(new AssertionError("ugly index is already closed")).when(indicesService).verifyIndexMetadata(ugly, ugly); - - final ClusterState newState = closeBadIndices(initialState, indicesService); - assertThat(newState.metaData().index(good.getIndex()).getState(), equalTo(IndexMetaData.State.OPEN)); - assertThat(newState.metaData().index(bad.getIndex()).getState(), equalTo(IndexMetaData.State.CLOSE)); - assertThat(newState.metaData().index(ugly.getIndex()).getState(), equalTo(IndexMetaData.State.CLOSE)); - } - public void testUpdateRoutingTable() { final int numOfShards = randomIntBetween(1, 10); diff --git a/server/src/test/java/org/elasticsearch/gateway/GatewayIndexStateIT.java b/server/src/test/java/org/elasticsearch/gateway/GatewayIndexStateIT.java index ebdae985a39c7..272ac31658229 100644 --- a/server/src/test/java/org/elasticsearch/gateway/GatewayIndexStateIT.java +++ b/server/src/test/java/org/elasticsearch/gateway/GatewayIndexStateIT.java @@ -34,7 +34,11 @@ import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.cluster.metadata.MappingMetaData; import org.elasticsearch.cluster.metadata.MetaData; +import org.elasticsearch.cluster.routing.IndexRoutingTable; +import org.elasticsearch.cluster.routing.IndexShardRoutingTable; +import org.elasticsearch.cluster.routing.RoutingTable; import org.elasticsearch.cluster.routing.ShardRoutingState; +import org.elasticsearch.cluster.routing.UnassignedInfo; import org.elasticsearch.common.Priority; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentFactory; @@ -51,12 +55,14 @@ import java.io.IOException; import java.util.List; +import java.util.concurrent.TimeUnit; import static org.elasticsearch.action.support.WriteRequest.RefreshPolicy.IMMEDIATE; import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.nullValue; @ClusterScope(scope = Scope.TEST, numDataNodes = 0) @@ -369,9 +375,20 @@ public Settings onNodeStopped(String nodeName) throws Exception { } }); - // ensureGreen(closedIndex) waits for the index to show up in the metadata - // this is crucial otherwise the state call below might not contain the index yet - ensureGreen(metaData.getIndex().getName()); + // check that the cluster does not keep reallocating shards + assertBusy(() -> { + final RoutingTable routingTable = client().admin().cluster().prepareState().get().getState().routingTable(); + final IndexRoutingTable indexRoutingTable = routingTable.index("test"); + assertNotNull(indexRoutingTable); + for (IndexShardRoutingTable shardRoutingTable : indexRoutingTable) { + assertTrue(shardRoutingTable.primaryShard().unassigned()); + assertEquals(UnassignedInfo.AllocationStatus.DECIDERS_NO, + shardRoutingTable.primaryShard().unassignedInfo().getLastAllocationStatus()); + assertThat(shardRoutingTable.primaryShard().unassignedInfo().getNumFailedAllocations(), greaterThan(0)); + } + }, 60, TimeUnit.SECONDS); + client().admin().indices().prepareClose("test").get(); + state = client().admin().cluster().prepareState().get().getState(); assertEquals(IndexMetaData.State.CLOSE, state.getMetaData().index(metaData.getIndex()).getState()); assertEquals("classic", state.getMetaData().index(metaData.getIndex()).getSettings().get("archived.index.similarity.BM25.type")); @@ -432,11 +449,19 @@ public Settings onNodeStopped(String nodeName) throws Exception { } }); - // ensureGreen(closedIndex) waits for the index to show up in the metadata - // this is crucial otherwise the state call below might not contain the index yet - ensureGreen(metaData.getIndex().getName()); - state = client().admin().cluster().prepareState().get().getState(); - assertEquals(IndexMetaData.State.CLOSE, state.getMetaData().index(metaData.getIndex()).getState()); + // check that the cluster does not keep reallocating shards + assertBusy(() -> { + final RoutingTable routingTable = client().admin().cluster().prepareState().get().getState().routingTable(); + final IndexRoutingTable indexRoutingTable = routingTable.index("test"); + assertNotNull(indexRoutingTable); + for (IndexShardRoutingTable shardRoutingTable : indexRoutingTable) { + assertTrue(shardRoutingTable.primaryShard().unassigned()); + assertEquals(UnassignedInfo.AllocationStatus.DECIDERS_NO, + shardRoutingTable.primaryShard().unassignedInfo().getLastAllocationStatus()); + assertThat(shardRoutingTable.primaryShard().unassignedInfo().getNumFailedAllocations(), greaterThan(0)); + } + }, 60, TimeUnit.SECONDS); + client().admin().indices().prepareClose("test").get(); // try to open it with the broken setting - fail again! ElasticsearchException ex = expectThrows(ElasticsearchException.class, () -> client().admin().indices().prepareOpen("test").get());