Do not close bad indices on startup (#39500)

With #17187, we verified IndexService creation during initial state recovery on the master and if the
recovery failed the index was imported as closed, not allocating any shards. This was mainly done to
prevent endless allocation loops and full log files on data-nodes when the indexmetadata contained
broken settings / analyzers. Zen2 loads the cluster state eagerly, and this check currently runs on all
nodes (not only the elected master), which can significantly slow down startup on data nodes.
Furthermore, with replicated closed indices (#33888) on the horizon, importing the index as closed
will no longer not allocate any shards. Fortunately, the original issue for endless allocation loops is
no longer a problem due to #18467, where we limit the retries of failed allocations. The solution here
is therefore to just undo #17187, as it's no longer necessary, and covered by #18467, which will solve
the issue for Zen2 and replicated closed indices as well.
This commit is contained in:
Yannick Welsch 2019-03-01 09:16:53 +01:00
parent b9b46fdec6
commit 1a50af7dd4
6 changed files with 52 additions and 67 deletions

View File

@ -31,8 +31,6 @@ import org.elasticsearch.cluster.node.DiscoveryNode;
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 @@ public class ClusterStateUpdaters {
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());

View File

@ -126,7 +126,6 @@ public class Gateway {
}
ClusterState recoveredState = Function.<ClusterState>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);

View File

@ -137,7 +137,6 @@ public class GatewayMetaState implements ClusterStateApplier, CoordinationState.
.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);
}

View File

@ -34,6 +34,10 @@ import org.elasticsearch.cluster.metadata.IndexMetaData;
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.HashMap;
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.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertBloc
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 class CreateIndexIT extends ESIntegTestCase {
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 =

View File

@ -33,10 +33,8 @@ import org.elasticsearch.common.settings.SettingUpgrader;
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 java.util.stream.Stream;
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.elasticsearch.gateway.GatewayService.STATE_NOT_RECOVERED_BLOCK
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 class ClusterStateUpdatersTests extends ESTestCase {
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);

View File

@ -34,7 +34,11 @@ import org.elasticsearch.cluster.metadata.IndexGraveyard;
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 org.elasticsearch.test.InternalTestCluster.RestartCallback;
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 class GatewayIndexStateIT extends ESIntegTestCase {
}
});
// 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 class GatewayIndexStateIT extends ESIntegTestCase {
}
});
// 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());