From a8ec2917207b5fa747e378a638aa53060631d002 Mon Sep 17 00:00:00 2001 From: Mayya Sharipova Date: Mon, 8 Jan 2018 16:58:03 -0500 Subject: [PATCH] Discontinue archiving broken cluster settings Currently unknown or invalid cluster settings get archived. For a better user experience, we stop archving broken cluster settings. Instead, we will fail to recover the cluster state. The solution for users in an upgrade case would be to rollback to the previous version, address the settings that would be unknown or invalid the next major version, and then proceed with the upgrade. Closes #28026 --- .../migration/migrate_7_0/cluster.asciidoc | 6 +++ .../settings/AbstractScopedSettings.java | 38 +++++++++++++++++++ .../org/elasticsearch/gateway/Gateway.java | 22 +++++------ .../gateway/GatewayIndexStateIT.java | 23 ++++------- 4 files changed, 62 insertions(+), 27 deletions(-) diff --git a/docs/reference/migration/migrate_7_0/cluster.asciidoc b/docs/reference/migration/migrate_7_0/cluster.asciidoc index e9584074d73d2..2df055192b7e6 100644 --- a/docs/reference/migration/migrate_7_0/cluster.asciidoc +++ b/docs/reference/migration/migrate_7_0/cluster.asciidoc @@ -14,3 +14,9 @@ primary shards of the opened index to be allocated. ==== Shard preferences `_primary`, `_primary_first`, `_replica`, and `_replica_first` are removed These shard preferences are removed in favour of the `_prefer_nodes` and `_only_nodes` preferences. + +==== Discontinue archiving broken cluster settings +We will no longer archive unknown or invalid cluster settings (prepending "archived." to a broken setting's name). +Instead, we will fail to recover a cluster state with broken cluster settings. +The solution for users in an upgrade case would be to rollback to the previous version, +address the settings that would be unknown or invalid in the next major version, and then proceed with the upgrade. diff --git a/server/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java b/server/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java index e8bb946c8a795..fd98222a59536 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java +++ b/server/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java @@ -681,6 +681,44 @@ public Settings archiveUnknownOrInvalidSettings( } } + + /** + * Checks invalid or unknown settings. Any setting that is not recognized or fails validation + * will be processed by consumers. + * An exception will be thrown if any invalid or unknown setting is found. + * + * @param settings the {@link Settings} instance to scan for unknown or invalid settings + * @param unknownConsumer callback on unknown settings (consumer receives unknown key and its + * associated value) + * @param invalidConsumer callback on invalid settings (consumer receives invalid key, its + * associated value and an exception) + */ + public void checkUnknownOrInvalidSettings( + final Settings settings, + final Consumer> unknownConsumer, + final BiConsumer, IllegalArgumentException> invalidConsumer) { + List failedKeys = new ArrayList<>(); + for (String key : settings.keySet()) { + try { + Setting setting = get(key); + if (setting != null) { + setting.get(settings); + } else { + if (!isPrivateSetting(key)) { + failedKeys.add(key); + unknownConsumer.accept(new Entry(key, settings)); + } + } + } catch (IllegalArgumentException ex) { + failedKeys.add(key); + invalidConsumer.accept(new Entry(key, settings), ex); + } + } + if (failedKeys.size() > 0) { + throw new IllegalStateException("Invalid or unknown settings: " + String.join(", ", failedKeys)); + } + } + private static final class Entry implements Map.Entry { private final String key; diff --git a/server/src/main/java/org/elasticsearch/gateway/Gateway.java b/server/src/main/java/org/elasticsearch/gateway/Gateway.java index ae8f5a85def44..01f5b095ef3a6 100644 --- a/server/src/main/java/org/elasticsearch/gateway/Gateway.java +++ b/server/src/main/java/org/elasticsearch/gateway/Gateway.java @@ -137,27 +137,25 @@ public void performStateRecovery(final GatewayStateRecoveredListener listener) t } } final ClusterSettings clusterSettings = clusterService.getClusterSettings(); - metaDataBuilder.persistentSettings( - clusterSettings.archiveUnknownOrInvalidSettings( - metaDataBuilder.persistentSettings(), - e -> logUnknownSetting("persistent", e), - (e, ex) -> logInvalidSetting("persistent", e, ex))); - metaDataBuilder.transientSettings( - clusterSettings.archiveUnknownOrInvalidSettings( - metaDataBuilder.transientSettings(), - e -> logUnknownSetting("transient", e), - (e, ex) -> logInvalidSetting("transient", e, ex))); + clusterSettings.checkUnknownOrInvalidSettings( + metaDataBuilder.persistentSettings(), + e -> logUnknownSetting("persistent", e), + (e, ex) -> logInvalidSetting("persistent", e, ex)); + clusterSettings.checkUnknownOrInvalidSettings( + metaDataBuilder.transientSettings(), + e -> logUnknownSetting("transient", e), + (e, ex) -> logInvalidSetting("transient", e, ex)); ClusterState.Builder builder = clusterService.newClusterStateBuilder(); builder.metaData(metaDataBuilder); listener.onSuccess(builder.build()); } private void logUnknownSetting(String settingType, Map.Entry e) { - logger.warn("ignoring unknown {} setting: [{}] with value [{}]; archiving", settingType, e.getKey(), e.getValue()); + logger.warn("unknown {} setting: [{}] with value [{}]", settingType, e.getKey(), e.getValue()); } private void logInvalidSetting(String settingType, Map.Entry e, IllegalArgumentException ex) { - logger.warn(() -> new ParameterizedMessage("ignoring invalid {} setting: [{}] with value [{}]; archiving", + logger.warn(() -> new ParameterizedMessage("invalid {} setting: [{}] with value [{}]", settingType, e.getKey(), e.getValue()), ex); } diff --git a/server/src/test/java/org/elasticsearch/gateway/GatewayIndexStateIT.java b/server/src/test/java/org/elasticsearch/gateway/GatewayIndexStateIT.java index aeadcf30e3678..67896516f0616 100644 --- a/server/src/test/java/org/elasticsearch/gateway/GatewayIndexStateIT.java +++ b/server/src/test/java/org/elasticsearch/gateway/GatewayIndexStateIT.java @@ -49,6 +49,8 @@ import org.elasticsearch.test.ESIntegTestCase.ClusterScope; import org.elasticsearch.test.ESIntegTestCase.Scope; import org.elasticsearch.test.InternalTestCluster.RestartCallback; +import org.junit.Rule; +import org.junit.rules.ExpectedException; import java.io.IOException; import java.util.List; @@ -64,6 +66,8 @@ @ClusterScope(scope = Scope.TEST, numDataNodes = 0) public class GatewayIndexStateIT extends ESIntegTestCase { + @Rule + public ExpectedException expectedException = ExpectedException.none(); private final Logger logger = Loggers.getLogger(GatewayIndexStateIT.class); public void testMappingMetaDataParsed() throws Exception { @@ -479,7 +483,7 @@ public void testRecoverMissingAnalyzer() throws Exception { assertThat(ex.getCause().getMessage(), containsString("analyzer [test] not found for field [field1]")); } - public void testArchiveBrokenClusterSettings() throws Exception { + public void testFailBrokenClusterSettings() throws Exception { logger.info("--> starting one node"); internalCluster().startNode(); client().prepareIndex("test", "type1", "1").setSource("field1", "value1").setRefreshPolicy(IMMEDIATE).get(); @@ -502,20 +506,9 @@ public void testArchiveBrokenClusterSettings() throws Exception { .put(ElectMasterService.DISCOVERY_ZEN_MINIMUM_MASTER_NODES_SETTING.getKey(), "broken").build()).build(); MetaData.FORMAT.write(brokenMeta, nodeEnv.nodeDataPaths()); } - internalCluster().fullRestart(); - ensureYellow("test"); // wait for state recovery - state = client().admin().cluster().prepareState().get().getState(); - assertEquals("true", state.metaData().persistentSettings().get("archived.this.is.unknown")); - assertEquals("broken", state.metaData().persistentSettings().get("archived." - + ElectMasterService.DISCOVERY_ZEN_MINIMUM_MASTER_NODES_SETTING.getKey())); - // delete these settings - client().admin().cluster().prepareUpdateSettings().setPersistentSettings(Settings.builder().putNull("archived.*")).get(); - - state = client().admin().cluster().prepareState().get().getState(); - assertNull(state.metaData().persistentSettings().get("archived.this.is.unknown")); - assertNull(state.metaData().persistentSettings().get("archived." - + ElectMasterService.DISCOVERY_ZEN_MINIMUM_MASTER_NODES_SETTING.getKey())); - assertHitCount(client().prepareSearch().setQuery(matchAllQuery()).get(), 1L); + expectedException.expect(ElasticsearchException.class); + internalCluster().fullRestart(); } + }