Always check for archiving broken index settings (#41209)
Today we check if an index has broken settings when checking if an index needs to be upgraded. However, it can be the case that an index setting became broken even if an index is already upgraded to the current version if the user removed a plugin (or downgraded from the default distribution to the non-default distribution) while on the same version of Elasticsearch. In this case, some registered settings would go missing and the index would now be broken. Yet, we miss this check and instead of archiving the settings, the index becomes unassigned due to the missing settings. This commit addresses this by checking for broken settings whether or not the index is upgraded.
This commit is contained in:
parent
badb7a22e0
commit
6566979c18
|
@ -88,7 +88,11 @@ public class MetaDataIndexUpgradeService {
|
|||
public IndexMetaData upgradeIndexMetaData(IndexMetaData indexMetaData, Version minimumIndexCompatibilityVersion) {
|
||||
// Throws an exception if there are too-old segments:
|
||||
if (isUpgraded(indexMetaData)) {
|
||||
return indexMetaData;
|
||||
/*
|
||||
* We still need to check for broken index settings since it might be that a user removed a plugin that registers a setting
|
||||
* needed by this index.
|
||||
*/
|
||||
return archiveBrokenIndexSettings(indexMetaData);
|
||||
}
|
||||
checkSupportedVersion(indexMetaData, minimumIndexCompatibilityVersion);
|
||||
IndexMetaData newMetaData = indexMetaData;
|
||||
|
|
|
@ -19,8 +19,6 @@
|
|||
|
||||
package org.elasticsearch.action.admin.indices.create;
|
||||
|
||||
import com.carrotsearch.hppc.cursors.ObjectCursor;
|
||||
import org.elasticsearch.ElasticsearchException;
|
||||
import org.elasticsearch.action.ActionListener;
|
||||
import org.elasticsearch.action.UnavailableShardsException;
|
||||
import org.elasticsearch.action.admin.cluster.state.ClusterStateResponse;
|
||||
|
@ -33,28 +31,18 @@ import org.elasticsearch.cluster.ClusterState;
|
|||
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;
|
||||
import org.elasticsearch.common.xcontent.XContentFactory;
|
||||
import org.elasticsearch.gateway.MetaStateService;
|
||||
import org.elasticsearch.index.IndexNotFoundException;
|
||||
import org.elasticsearch.index.query.RangeQueryBuilder;
|
||||
import org.elasticsearch.test.ESIntegTestCase;
|
||||
import org.elasticsearch.test.ESIntegTestCase.ClusterScope;
|
||||
import org.elasticsearch.test.ESIntegTestCase.Scope;
|
||||
import org.elasticsearch.test.InternalTestCluster;
|
||||
|
||||
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;
|
||||
|
||||
|
@ -63,12 +51,8 @@ import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
|
|||
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
|
||||
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertBlocked;
|
||||
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;
|
||||
import static org.hamcrest.Matchers.lessThanOrEqualTo;
|
||||
import static org.hamcrest.core.IsNull.notNullValue;
|
||||
|
||||
|
@ -396,57 +380,4 @@ public class CreateIndexIT extends ESIntegTestCase {
|
|||
assertEquals("Should have index name in response", "foo", response.index());
|
||||
}
|
||||
|
||||
public void testIndexWithUnknownSetting() throws Exception {
|
||||
final int replicas = internalCluster().numDataNodes() - 1;
|
||||
final Settings settings = Settings.builder().put("index.number_of_shards", 1).put("index.number_of_replicas", replicas).build();
|
||||
client().admin().indices().prepareCreate("test").setSettings(settings).get();
|
||||
ensureGreen("test");
|
||||
final ClusterState state = client().admin().cluster().prepareState().get().getState();
|
||||
|
||||
final Set<String> dataOrMasterNodeNames = new HashSet<>();
|
||||
for (final ObjectCursor<DiscoveryNode> node : state.nodes().getMasterAndDataNodes().values()) {
|
||||
assertTrue(dataOrMasterNodeNames.add(node.value.getName()));
|
||||
}
|
||||
|
||||
final IndexMetaData metaData = state.getMetaData().index("test");
|
||||
internalCluster().fullRestart(new InternalTestCluster.RestartCallback() {
|
||||
@Override
|
||||
public Settings onNodeStopped(String nodeName) throws Exception {
|
||||
if (dataOrMasterNodeNames.contains(nodeName)) {
|
||||
final MetaStateService metaStateService = internalCluster().getInstance(MetaStateService.class, nodeName);
|
||||
final IndexMetaData brokenMetaData =
|
||||
IndexMetaData
|
||||
.builder(metaData)
|
||||
.settings(Settings.builder().put(metaData.getSettings()).put("index.foo", true))
|
||||
.build();
|
||||
// so evil
|
||||
metaStateService.writeIndexAndUpdateManifest("broken metadata", brokenMetaData);
|
||||
}
|
||||
return super.onNodeStopped(nodeName);
|
||||
}
|
||||
});
|
||||
|
||||
// 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 =
|
||||
expectThrows(ElasticsearchException.class, () -> client().admin().indices().prepareOpen("test").get());
|
||||
assertThat(e, hasToString(containsString("Failed to verify index " + metaData.getIndex())));
|
||||
assertNotNull(e.getCause());
|
||||
assertThat(e.getCause(), instanceOf(IllegalArgumentException.class));
|
||||
assertThat(e, hasToString(containsString("unknown setting [index.foo]")));
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
@ -28,12 +28,12 @@ import org.elasticsearch.test.VersionUtils;
|
|||
|
||||
import java.util.Collections;
|
||||
|
||||
import static org.hamcrest.Matchers.equalTo;
|
||||
|
||||
public class MetaDataIndexUpgradeServiceTests extends ESTestCase {
|
||||
|
||||
public void testArchiveBrokenIndexSettings() {
|
||||
MetaDataIndexUpgradeService service = new MetaDataIndexUpgradeService(Settings.EMPTY, xContentRegistry(),
|
||||
new MapperRegistry(Collections.emptyMap(), Collections.emptyMap(), MapperPlugin.NOOP_FIELD_FILTER),
|
||||
IndexScopedSettings.DEFAULT_SCOPED_SETTINGS, Collections.emptyList());
|
||||
MetaDataIndexUpgradeService service = getMetaDataIndexUpgradeService();
|
||||
IndexMetaData src = newIndexMeta("foo", Settings.EMPTY);
|
||||
IndexMetaData indexMetaData = service.archiveBrokenIndexSettings(src);
|
||||
assertSame(indexMetaData, src);
|
||||
|
@ -58,10 +58,20 @@ public class MetaDataIndexUpgradeServiceTests extends ESTestCase {
|
|||
assertSame(indexMetaData, src);
|
||||
}
|
||||
|
||||
public void testAlreadyUpgradedIndexArchivesBrokenIndexSettings() {
|
||||
final MetaDataIndexUpgradeService service = getMetaDataIndexUpgradeService();
|
||||
final IndexMetaData initial = newIndexMeta(
|
||||
"foo",
|
||||
Settings.builder().put(IndexMetaData.SETTING_VERSION_UPGRADED, Version.CURRENT).put("index.refresh_interval", "-200").build());
|
||||
assertTrue(service.isUpgraded(initial));
|
||||
final IndexMetaData after = service.upgradeIndexMetaData(initial, Version.CURRENT.minimumIndexCompatibilityVersion());
|
||||
// the index does not need to be upgraded, but checking that it does should archive any broken settings
|
||||
assertThat(after.getSettings().get("archived.index.refresh_interval"), equalTo("-200"));
|
||||
assertNull(after.getSettings().get("index.refresh_interval"));
|
||||
}
|
||||
|
||||
public void testUpgrade() {
|
||||
MetaDataIndexUpgradeService service = new MetaDataIndexUpgradeService(Settings.EMPTY, xContentRegistry(),
|
||||
new MapperRegistry(Collections.emptyMap(), Collections.emptyMap(), MapperPlugin.NOOP_FIELD_FILTER),
|
||||
IndexScopedSettings.DEFAULT_SCOPED_SETTINGS, Collections.emptyList());
|
||||
MetaDataIndexUpgradeService service = getMetaDataIndexUpgradeService();
|
||||
IndexMetaData src = newIndexMeta("foo", Settings.builder().put("index.refresh_interval", "-200").build());
|
||||
assertFalse(service.isUpgraded(src));
|
||||
src = service.upgradeIndexMetaData(src, Version.CURRENT.minimumIndexCompatibilityVersion());
|
||||
|
@ -72,9 +82,7 @@ public class MetaDataIndexUpgradeServiceTests extends ESTestCase {
|
|||
}
|
||||
|
||||
public void testIsUpgraded() {
|
||||
MetaDataIndexUpgradeService service = new MetaDataIndexUpgradeService(Settings.EMPTY, xContentRegistry(),
|
||||
new MapperRegistry(Collections.emptyMap(), Collections.emptyMap(), MapperPlugin.NOOP_FIELD_FILTER),
|
||||
IndexScopedSettings.DEFAULT_SCOPED_SETTINGS, Collections.emptyList());
|
||||
MetaDataIndexUpgradeService service = getMetaDataIndexUpgradeService();
|
||||
IndexMetaData src = newIndexMeta("foo", Settings.builder().put("index.refresh_interval", "-200").build());
|
||||
assertFalse(service.isUpgraded(src));
|
||||
Version version = VersionUtils.randomVersionBetween(random(), VersionUtils.getFirstVersion(), VersionUtils.getPreviousVersion());
|
||||
|
@ -85,9 +93,7 @@ public class MetaDataIndexUpgradeServiceTests extends ESTestCase {
|
|||
}
|
||||
|
||||
public void testFailUpgrade() {
|
||||
MetaDataIndexUpgradeService service = new MetaDataIndexUpgradeService(Settings.EMPTY, xContentRegistry(),
|
||||
new MapperRegistry(Collections.emptyMap(), Collections.emptyMap(), MapperPlugin.NOOP_FIELD_FILTER),
|
||||
IndexScopedSettings.DEFAULT_SCOPED_SETTINGS, Collections.emptyList());
|
||||
MetaDataIndexUpgradeService service = getMetaDataIndexUpgradeService();
|
||||
Version minCompat = Version.CURRENT.minimumIndexCompatibilityVersion();
|
||||
Version indexUpgraded = VersionUtils.randomVersionBetween(random(), minCompat, VersionUtils.getPreviousVersion(Version.CURRENT));
|
||||
Version indexCreated = Version.fromString((minCompat.major - 1) + "." + randomInt(5) + "." + randomInt(5));
|
||||
|
@ -141,6 +147,15 @@ public class MetaDataIndexUpgradeServiceTests extends ESTestCase {
|
|||
assertEquals(message, "Cannot upgrade index foo");
|
||||
}
|
||||
|
||||
private MetaDataIndexUpgradeService getMetaDataIndexUpgradeService() {
|
||||
return new MetaDataIndexUpgradeService(
|
||||
Settings.EMPTY,
|
||||
xContentRegistry(),
|
||||
new MapperRegistry(Collections.emptyMap(), Collections.emptyMap(), MapperPlugin.NOOP_FIELD_FILTER),
|
||||
IndexScopedSettings.DEFAULT_SCOPED_SETTINGS,
|
||||
Collections.emptyList());
|
||||
}
|
||||
|
||||
public static IndexMetaData newIndexMeta(String name, Settings indexSettings) {
|
||||
Settings build = Settings.builder().put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT)
|
||||
.put(IndexMetaData.SETTING_NUMBER_OF_REPLICAS, 1)
|
||||
|
@ -152,4 +167,5 @@ public class MetaDataIndexUpgradeServiceTests extends ESTestCase {
|
|||
.build();
|
||||
return IndexMetaData.builder(name).settings(build).build();
|
||||
}
|
||||
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue