Browse Source

Allow legacy index settings on legacy indices (#90264)

Add code to ignore unknown index settings on
old indices, to allow rolling-upgrades to work.

Co-authored-by: David Turner <david.turner@elastic.co>
Nikola Grcevski 3 years ago
parent
commit
0652a59155

+ 6 - 0
docs/changelog/90264.yaml

@@ -0,0 +1,6 @@
+pr: 90264
+summary: Allow legacy index settings on legacy indices
+area: Infra/Core
+type: enhancement
+issues:
+ - 84992

+ 114 - 0
qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/UpgradeWithOldIndexSettingsIT.java

@@ -0,0 +1,114 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+package org.elasticsearch.upgrades;
+
+import org.elasticsearch.Version;
+import org.elasticsearch.client.Request;
+import org.elasticsearch.client.Response;
+import org.elasticsearch.client.ResponseException;
+import org.elasticsearch.common.xcontent.support.XContentMapValues;
+
+import java.io.IOException;
+import java.util.Locale;
+import java.util.Map;
+
+import static org.elasticsearch.rest.action.search.RestSearchAction.TOTAL_HITS_AS_INT_PARAM;
+import static org.hamcrest.Matchers.is;
+
+public class UpgradeWithOldIndexSettingsIT extends AbstractRollingTestCase {
+
+    private static final String INDEX_NAME = "test_index_old_settings";
+    private static final String EXPECTED_WARNING = "[index.indexing.slowlog.level] setting was deprecated in Elasticsearch and will "
+        + "be removed in a future release! See the breaking changes documentation for the next major version.";
+
+    private static final String EXPECTED_V8_WARNING = "[index.indexing.slowlog.level] setting was deprecated in the previous Elasticsearch"
+        + " release and is removed in this release.";
+
+    @SuppressWarnings("unchecked")
+    public void testOldIndexSettings() throws Exception {
+        switch (CLUSTER_TYPE) {
+            case OLD -> {
+                Request createTestIndex = new Request("PUT", "/" + INDEX_NAME);
+                createTestIndex.setJsonEntity("{\"settings\": {\"index.indexing.slowlog.level\": \"WARN\"}}");
+                createTestIndex.setOptions(expectWarnings(EXPECTED_WARNING));
+                if (UPGRADE_FROM_VERSION.before(Version.V_8_0_0)) {
+                    // create index with settings no longer valid in 8.0
+                    client().performRequest(createTestIndex);
+                } else {
+                    assertTrue(
+                        expectThrows(ResponseException.class, () -> client().performRequest(createTestIndex)).getMessage()
+                            .contains("unknown setting [index.indexing.slowlog.level]")
+                    );
+
+                    Request createTestIndex1 = new Request("PUT", "/" + INDEX_NAME);
+                    client().performRequest(createTestIndex1);
+                }
+
+                // add some data
+                Request bulk = new Request("POST", "/_bulk");
+                bulk.addParameter("refresh", "true");
+                if (UPGRADE_FROM_VERSION.before(Version.V_8_0_0)) {
+                    bulk.setOptions(expectWarnings(EXPECTED_WARNING));
+                }
+                bulk.setJsonEntity(String.format(Locale.ROOT, """
+                    {"index": {"_index": "%s"}}
+                    {"f1": "v1", "f2": "v2"}
+                    """, INDEX_NAME));
+                client().performRequest(bulk);
+            }
+            case MIXED -> {
+                // add some more data
+                Request bulk = new Request("POST", "/_bulk");
+                bulk.addParameter("refresh", "true");
+                if (UPGRADE_FROM_VERSION.before(Version.V_8_0_0)) {
+                    bulk.setOptions(expectWarnings(EXPECTED_WARNING));
+                }
+                bulk.setJsonEntity(String.format(Locale.ROOT, """
+                    {"index": {"_index": "%s"}}
+                    {"f1": "v3", "f2": "v4"}
+                    """, INDEX_NAME));
+                client().performRequest(bulk);
+            }
+            case UPGRADED -> {
+                if (UPGRADE_FROM_VERSION.before(Version.V_8_0_0)) {
+                    Request createTestIndex = new Request("PUT", "/" + INDEX_NAME + "/_settings");
+                    // update index settings should work
+                    createTestIndex.setJsonEntity("{\"index.indexing.slowlog.level\": \"INFO\"}");
+                    createTestIndex.setOptions(expectWarnings(EXPECTED_V8_WARNING));
+                    client().performRequest(createTestIndex);
+
+                    // ensure we were able to change the setting, despite it having no effect
+                    Request indexSettingsRequest = new Request("GET", "/" + INDEX_NAME + "/_settings");
+                    Map<String, Object> response = entityAsMap(client().performRequest(indexSettingsRequest));
+
+                    var slowLogLevel = (String) (XContentMapValues.extractValue(
+                        INDEX_NAME + ".settings.index.indexing.slowlog.level",
+                        response
+                    ));
+
+                    // check that we can read our old index settings
+                    assertThat(slowLogLevel, is("INFO"));
+                }
+                assertCount(INDEX_NAME, 2);
+            }
+        }
+    }
+
+    private void assertCount(String index, int countAtLeast) throws IOException {
+        Request searchTestIndexRequest = new Request("POST", "/" + index + "/_search");
+        searchTestIndexRequest.addParameter(TOTAL_HITS_AS_INT_PARAM, "true");
+        searchTestIndexRequest.addParameter("filter_path", "hits.total");
+        Response searchTestIndexResponse = client().performRequest(searchTestIndexRequest);
+        Map<String, Object> response = entityAsMap(searchTestIndexResponse);
+
+        var hitsTotal = (Integer) (XContentMapValues.extractValue("hits.total", response));
+
+        assertTrue(hitsTotal >= countAtLeast);
+    }
+}

+ 24 - 0
server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetadata.java

@@ -482,6 +482,30 @@ public class IndexMetadata implements Diffable<IndexMetadata>, ToXContentFragmen
         Setting.Property.Final
     );
 
+    /**
+     * Legacy index setting, kept for 7.x BWC compatibility. This setting has no effect in 8.x. Do not use.
+     * TODO: Remove in 9.0
+     */
+    @Deprecated
+    public static final Setting<String> INDEX_ROLLUP_SOURCE_UUID = Setting.simpleString(
+        "index.rollup.source.uuid",
+        Property.IndexScope,
+        Property.PrivateIndex,
+        Property.IndexSettingDeprecatedInV7AndRemovedInV8
+    );
+
+    /**
+     * Legacy index setting, kept for 7.x BWC compatibility. This setting has no effect in 8.x. Do not use.
+     * TODO: Remove in 9.0
+     */
+    @Deprecated
+    public static final Setting<String> INDEX_ROLLUP_SOURCE_NAME = Setting.simpleString(
+        "index.rollup.source.name",
+        Property.IndexScope,
+        Property.PrivateIndex,
+        Property.IndexSettingDeprecatedInV7AndRemovedInV8
+    );
+
     public static final String KEY_IN_SYNC_ALLOCATIONS = "in_sync_allocations";
     static final String KEY_VERSION = "version";
     static final String KEY_MAPPING_VERSION = "mapping_version";

+ 3 - 0
server/src/main/java/org/elasticsearch/cluster/metadata/MetadataUpdateSettingsService.java

@@ -175,6 +175,7 @@ public class MetadataUpdateSettingsService {
                 Index index = request.indices()[i];
                 actualIndices[i] = index.getName();
                 final IndexMetadata metadata = currentState.metadata().getIndexSafe(index);
+
                 if (metadata.getState() == IndexMetadata.State.OPEN) {
                     openIndices.add(index);
                 } else {
@@ -310,6 +311,8 @@ public class MetadataUpdateSettingsService {
     ) {
         for (Index index : indices) {
             IndexMetadata indexMetadata = metadataBuilder.getSafe(index);
+            // We validate the settings for removed deprecated settings, since we have the indexMetadata now.
+            indexScopedSettings.validate(indexMetadata.getSettings(), true, true, true);
             Settings.Builder indexSettings = Settings.builder().put(indexMetadata.getSettings());
             if (settingUpdater.apply(index, indexSettings)) {
                 if (preserveExisting) {

+ 5 - 0
server/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java

@@ -434,6 +434,8 @@ public abstract class AbstractScopedSettings {
         addSettingsUpdateConsumer(setting, consumer, (s) -> {});
     }
 
+    protected void validateDeprecatedAndRemovedSettingV7(Settings settings, Setting<?> setting) {}
+
     /**
      * Validates that all settings are registered and valid.
      *
@@ -562,6 +564,9 @@ public abstract class AbstractScopedSettings {
             if (setting.hasComplexMatcher()) {
                 setting = setting.getConcreteSetting(key);
             }
+            if (setting.isDeprecatedAndRemoved()) {
+                validateDeprecatedAndRemovedSettingV7(settings, setting);
+            }
             if (validateValue && settingsDependencies.isEmpty() == false) {
                 for (final Setting.SettingDependency settingDependency : settingsDependencies) {
                     final Setting<?> dependency = settingDependency.getSetting();

+ 23 - 1
server/src/main/java/org/elasticsearch/common/settings/IndexScopedSettings.java

@@ -7,6 +7,7 @@
  */
 package org.elasticsearch.common.settings;
 
+import org.elasticsearch.Version;
 import org.elasticsearch.cluster.metadata.IndexMetadata;
 import org.elasticsearch.cluster.metadata.MetadataIndexStateService;
 import org.elasticsearch.cluster.routing.UnassignedInfo;
@@ -180,7 +181,16 @@ public final class IndexScopedSettings extends AbstractScopedSettings {
         IndexSettings.MODE,
         IndexMetadata.INDEX_ROUTING_PATH,
         IndexSettings.TIME_SERIES_START_TIME,
-        IndexSettings.TIME_SERIES_END_TIME
+        IndexSettings.TIME_SERIES_END_TIME,
+
+        // Legacy index settings we must keep around for BWC from 7.x
+        EngineConfig.INDEX_OPTIMIZE_AUTO_GENERATED_IDS,
+        IndexMetadata.INDEX_ROLLUP_SOURCE_NAME,
+        IndexMetadata.INDEX_ROLLUP_SOURCE_UUID,
+        IndexSettings.MAX_ADJACENCY_MATRIX_FILTERS_SETTING,
+        IndexingSlowLog.INDEX_INDEXING_SLOWLOG_LEVEL_SETTING,
+        SearchSlowLog.INDEX_SEARCH_SLOWLOG_LEVEL,
+        Store.FORCE_RAM_TERM_DICT
     );
 
     public static final IndexScopedSettings DEFAULT_SCOPED_SETTINGS = new IndexScopedSettings(Settings.EMPTY, BUILT_IN_INDEX_SETTINGS);
@@ -225,4 +235,16 @@ public final class IndexScopedSettings extends AbstractScopedSettings {
                 return IndexMetadata.INDEX_ROUTING_INITIAL_RECOVERY_GROUP_SETTING.getRawKey().match(key);
         }
     }
+
+    @Override
+    protected void validateDeprecatedAndRemovedSettingV7(Settings settings, Setting<?> setting) {
+        Version indexVersion = IndexMetadata.SETTING_INDEX_VERSION_CREATED.get(settings);
+        // At various stages in settings verification we will perform validation without having the
+        // IndexMetadata at hand, in which case the setting version will be empty. We don't want to
+        // error out on those validations, we will check with the creation version present at index
+        // creation time, as well as on index update settings.
+        if (indexVersion.equals(Version.V_EMPTY) == false && indexVersion.major != Version.V_7_0_0.major) {
+            throw new IllegalArgumentException("unknown setting [" + setting.getKey() + "]");
+        }
+    }
 }

+ 29 - 4
server/src/main/java/org/elasticsearch/common/settings/Setting.java

@@ -139,7 +139,13 @@ public class Setting<T> implements ToXContentObject {
         /**
          * Indicates an index-level setting that is privately managed. Such a setting can not even be set on index creation.
          */
-        PrivateIndex
+        PrivateIndex,
+
+        /**
+         * Indicates that this index-level setting was deprecated in {@link Version#V_7_17_0} and is
+         * forbidden in indices created from {@link Version#V_8_0_0} onwards.
+         */
+        IndexSettingDeprecatedInV7AndRemovedInV8
     }
 
     private final Key key;
@@ -151,6 +157,11 @@ public class Setting<T> implements ToXContentObject {
     private final EnumSet<Property> properties;
 
     private static final EnumSet<Property> EMPTY_PROPERTIES = EnumSet.noneOf(Property.class);
+    private static final EnumSet<Property> DEPRECATED_PROPERTIES = EnumSet.of(
+        Property.Deprecated,
+        Property.DeprecatedWarning,
+        Property.IndexSettingDeprecatedInV7AndRemovedInV8
+    );
 
     private Setting(
         Key key,
@@ -181,12 +192,13 @@ public class Setting<T> implements ToXContentObject {
             if (propertiesAsSet.contains(Property.Dynamic) && propertiesAsSet.contains(Property.OperatorDynamic)) {
                 throw new IllegalArgumentException("setting [" + key + "] cannot be both dynamic and operator dynamic");
             }
-            if (propertiesAsSet.contains(Property.Deprecated) && propertiesAsSet.contains(Property.DeprecatedWarning)) {
-                throw new IllegalArgumentException("setting [" + key + "] cannot be deprecated at both critical and warning levels");
+            if (propertiesAsSet.stream().filter(DEPRECATED_PROPERTIES::contains).count() > 1) {
+                throw new IllegalArgumentException("setting [" + key + "] must be at most one of [" + DEPRECATED_PROPERTIES + "]");
             }
             checkPropertyRequiresIndexScope(propertiesAsSet, Property.NotCopyableOnResize);
             checkPropertyRequiresIndexScope(propertiesAsSet, Property.InternalIndex);
             checkPropertyRequiresIndexScope(propertiesAsSet, Property.PrivateIndex);
+            checkPropertyRequiresIndexScope(propertiesAsSet, Property.IndexSettingDeprecatedInV7AndRemovedInV8);
             checkPropertyRequiresNodeScope(propertiesAsSet);
             this.properties = propertiesAsSet;
         }
@@ -409,13 +421,19 @@ public class Setting<T> implements ToXContentObject {
      * Returns <code>true</code> if this setting is deprecated, otherwise <code>false</code>
      */
     private boolean isDeprecated() {
-        return properties.contains(Property.Deprecated) || properties.contains(Property.DeprecatedWarning);
+        return properties.contains(Property.Deprecated)
+            || properties.contains(Property.DeprecatedWarning)
+            || properties.contains(Property.IndexSettingDeprecatedInV7AndRemovedInV8);
     }
 
     private boolean isDeprecatedWarningOnly() {
         return properties.contains(Property.DeprecatedWarning);
     }
 
+    public boolean isDeprecatedAndRemoved() {
+        return properties.contains(Property.IndexSettingDeprecatedInV7AndRemovedInV8);
+    }
+
     /**
      * Returns <code>true</code> iff this setting is a group setting. Group settings represent a set of settings rather than a single value.
      * The key, see {@link #getKey()}, in contrast to non-group settings is a prefix like {@code cluster.store.} that matches all settings
@@ -599,6 +617,13 @@ public class Setting<T> implements ToXContentObject {
             String message = "[{}] setting was deprecated in Elasticsearch and will be removed in a future release.";
             if (this.isDeprecatedWarningOnly()) {
                 Settings.DeprecationLoggerHolder.deprecationLogger.warn(DeprecationCategory.SETTINGS, key, message, key);
+            } else if (this.isDeprecatedAndRemoved()) {
+                Settings.DeprecationLoggerHolder.deprecationLogger.critical(
+                    DeprecationCategory.SETTINGS,
+                    key,
+                    "[{}] setting was deprecated in the previous Elasticsearch release and is removed in this release.",
+                    key
+                );
             } else {
                 Settings.DeprecationLoggerHolder.deprecationLogger.critical(DeprecationCategory.SETTINGS, key, message, key);
             }

+ 14 - 0
server/src/main/java/org/elasticsearch/index/IndexSettings.java

@@ -550,6 +550,20 @@ public final class IndexSettings {
         Property.Final
     );
 
+    /**
+     * Legacy index setting, kept for 7.x BWC compatibility. This setting has no effect in 8.x. Do not use.
+     * TODO: Remove in 9.0
+     */
+    @Deprecated
+    public static final Setting<Integer> MAX_ADJACENCY_MATRIX_FILTERS_SETTING = Setting.intSetting(
+        "index.max_adjacency_matrix_filters",
+        100,
+        2,
+        Property.Dynamic,
+        Property.IndexScope,
+        Property.IndexSettingDeprecatedInV7AndRemovedInV8
+    );
+
     private final Index index;
     private final Version version;
     private final Logger logger;

+ 14 - 0
server/src/main/java/org/elasticsearch/index/IndexingSlowLog.java

@@ -69,6 +69,20 @@ public final class IndexingSlowLog implements IndexingOperationListener {
         Property.IndexScope
     );
 
+    /**
+     * Legacy index setting, kept for 7.x BWC compatibility. This setting has no effect in 8.x. Do not use.
+     * TODO: Remove in 9.0
+     */
+    @Deprecated
+    public static final Setting<SlowLogLevel> INDEX_INDEXING_SLOWLOG_LEVEL_SETTING = new Setting<>(
+        INDEX_INDEXING_SLOWLOG_PREFIX + ".level",
+        SlowLogLevel.TRACE.name(),
+        SlowLogLevel::parse,
+        Property.Dynamic,
+        Property.IndexScope,
+        Property.IndexSettingDeprecatedInV7AndRemovedInV8
+    );
+
     private final Logger indexLogger;
     private final Index index;
 

+ 1 - 1
server/src/main/java/org/elasticsearch/index/MergePolicyConfig.java

@@ -143,7 +143,7 @@ public final class MergePolicyConfig {
         "index.merge.policy.max_merge_at_once_explicit",
         30,
         2,
-        Property.Deprecated,
+        Property.Deprecated, // When removing in 9.0 follow the approach of IndexSettingDeprecatedInV7AndRemovedInV8
         Property.Dynamic,
         Property.IndexScope
     );

+ 14 - 0
server/src/main/java/org/elasticsearch/index/SearchSlowLog.java

@@ -102,6 +102,20 @@ public final class SearchSlowLog implements SearchOperationListener {
         Property.IndexScope
     );
 
+    /**
+     * Legacy index setting, kept for 7.x BWC compatibility. This setting has no effect in 8.x. Do not use.
+     * TODO: Remove in 9.0
+     */
+    @Deprecated
+    public static final Setting<SlowLogLevel> INDEX_SEARCH_SLOWLOG_LEVEL = new Setting<>(
+        INDEX_SEARCH_SLOWLOG_PREFIX + ".level",
+        SlowLogLevel.TRACE.name(),
+        SlowLogLevel::parse,
+        Property.Dynamic,
+        Property.IndexScope,
+        Property.IndexSettingDeprecatedInV7AndRemovedInV8
+    );
+
     private static final ToXContent.Params FORMAT_PARAMS = new ToXContent.MapParams(Collections.singletonMap("pretty", "false"));
 
     public SearchSlowLog(IndexSettings indexSettings) {

+ 38 - 0
server/src/main/java/org/elasticsearch/index/SlowLogLevel.java

@@ -0,0 +1,38 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+package org.elasticsearch.index;
+
+import java.util.Locale;
+
+/**
+ * Legacy enum class for index settings, kept for 7.x BWC compatibility. Do not use.
+ * TODO: Remove in 9.0
+ */
+@Deprecated
+public enum SlowLogLevel {
+    WARN(3), // most specific - little logging
+    INFO(2),
+    DEBUG(1),
+    TRACE(0); // least specific - lots of logging
+
+    private final int specificity;
+
+    SlowLogLevel(int specificity) {
+        this.specificity = specificity;
+    }
+
+    public static SlowLogLevel parse(String level) {
+        return valueOf(level.toUpperCase(Locale.ROOT));
+    }
+
+    boolean isLevelEnabledFor(SlowLogLevel levelToBeUsed) {
+        // example: this.info(2) tries to log with levelToBeUsed.warn(3) - should allow
+        return this.specificity <= levelToBeUsed.specificity;
+    }
+}

+ 13 - 0
server/src/main/java/org/elasticsearch/index/engine/EngineConfig.java

@@ -108,6 +108,19 @@ public final class EngineConfig {
         }
     }, Property.IndexScope, Property.NodeScope);
 
+    /**
+     * Legacy index setting, kept for 7.x BWC compatibility. This setting has no effect in 8.x. Do not use.
+     * TODO: Remove in 9.0
+     */
+    @Deprecated
+    public static final Setting<Boolean> INDEX_OPTIMIZE_AUTO_GENERATED_IDS = Setting.boolSetting(
+        "index.optimize_auto_generated_id",
+        true,
+        Property.IndexScope,
+        Property.Dynamic,
+        Property.IndexSettingDeprecatedInV7AndRemovedInV8
+    );
+
     private final TranslogConfig translogConfig;
 
     private final LongSupplier relativeTimeInNanosSupplier;

+ 13 - 0
server/src/main/java/org/elasticsearch/index/store/Store.java

@@ -121,6 +121,19 @@ import static org.elasticsearch.index.engine.Engine.ES_VERSION;
  * </pre>
  */
 public class Store extends AbstractIndexShardComponent implements Closeable, RefCounted {
+
+    /**
+     * Legacy index setting, kept for 7.x BWC compatibility. This setting has no effect in 8.x. Do not use.
+     * TODO: Remove in 9.0
+     */
+    @Deprecated
+    public static final Setting<Boolean> FORCE_RAM_TERM_DICT = Setting.boolSetting(
+        "index.force_memory_term_dictionary",
+        false,
+        Property.IndexScope,
+        Property.IndexSettingDeprecatedInV7AndRemovedInV8
+    );
+
     static final String CODEC = "store";
     static final int CORRUPTED_MARKER_CODEC_VERSION = 2;
     // public is for test purposes

+ 9 - 1
server/src/test/java/org/elasticsearch/common/settings/SettingTests.java

@@ -1416,9 +1416,17 @@ public class SettingTests extends ESTestCase {
     }
 
     public void testDeprecationPropertyValidation() {
-        final IllegalArgumentException e = expectThrows(
+        expectThrows(
             IllegalArgumentException.class,
             () -> Setting.boolSetting("a.bool.setting", true, Property.Deprecated, Property.DeprecatedWarning)
         );
+        expectThrows(
+            IllegalArgumentException.class,
+            () -> Setting.boolSetting("a.bool.setting", true, Property.Deprecated, Property.IndexSettingDeprecatedInV7AndRemovedInV8)
+        );
+        expectThrows(
+            IllegalArgumentException.class,
+            () -> Setting.boolSetting("a.bool.setting", true, Property.DeprecatedWarning, Property.IndexSettingDeprecatedInV7AndRemovedInV8)
+        );
     }
 }

+ 2 - 1
x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/action/TransportResumeFollowActionTests.java

@@ -329,7 +329,8 @@ public class TransportResumeFollowActionTests extends ESTestCase {
         replicatedSettings.add(IndexSettings.TIME_SERIES_END_TIME);
 
         for (Setting<?> setting : IndexScopedSettings.BUILT_IN_INDEX_SETTINGS) {
-            if (setting.isDynamic()) {
+            // removed settings have no effect, they are only there for BWC
+            if (setting.isDynamic() && setting.isDeprecatedAndRemoved() == false) {
                 boolean notReplicated = TransportResumeFollowAction.NON_REPLICATED_SETTINGS.contains(setting);
                 boolean replicated = replicatedSettings.contains(setting);
                 assertThat(