From 6566979c189dc111ea0128469771aafb3c3c556e Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Wed, 17 Apr 2019 06:59:29 -0400 Subject: [PATCH] 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. --- .../metadata/MetaDataIndexUpgradeService.java | 6 +- .../admin/indices/create/CreateIndexIT.java | 69 ------------------- .../MetaDataIndexUpgradeServiceTests.java | 40 +++++++---- 3 files changed, 33 insertions(+), 82 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexUpgradeService.java b/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexUpgradeService.java index 6bc9104000f..d3520da6702 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexUpgradeService.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexUpgradeService.java @@ -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; 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 f23dbaa8ea4..27e3ffefd63 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 @@ -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 dataOrMasterNodeNames = new HashSet<>(); - for (final ObjectCursor 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]"))); - } - } diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataIndexUpgradeServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataIndexUpgradeServiceTests.java index c1e341fd5bc..50166bd42b3 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataIndexUpgradeServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataIndexUpgradeServiceTests.java @@ -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(); } + }