Skip to content

Allow legacy index settings on legacy indices #90264

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 22 commits into from
Nov 2, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions docs/changelog/90264.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pr: 90264
summary: Allow legacy index settings on legacy indices
area: Infra/Core
type: enhancement
issues:
- 84992
Original file line number Diff line number Diff line change
@@ -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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

out of curiosity.
Any particular reason not to use yml test for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's just my comfort level with writing yml tests, but that definitely means I should add one :). I'll add a yml test too!


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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,30 @@ public Iterator<Setting<?>> settings() {
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";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ ClusterState execute(ClusterState currentState) {
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 {
Expand Down Expand Up @@ -310,6 +311,8 @@ public static void updateIndexSettings(
) {
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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,8 @@ public synchronized <T> void addSettingsUpdateConsumer(Setting<T> setting, Consu
addSettingsUpdateConsumer(setting, consumer, (s) -> {});
}

protected void validateDeprecatedAndRemovedSettingV7(Settings settings, Setting<?> setting) {}

/**
* Validates that all settings are registered and valid.
*
Expand Down Expand Up @@ -562,6 +564,9 @@ void validate(final String key, final Settings settings, final boolean validateV
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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -225,4 +235,16 @@ public boolean isPrivateSetting(String key) {
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() + "]");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,13 @@ public enum Property {
/**
* 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;
Expand All @@ -151,6 +157,11 @@ public enum Property {
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,
Expand Down Expand Up @@ -181,12 +192,13 @@ private Setting(
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;
}
Expand Down Expand Up @@ -409,13 +421,19 @@ public boolean hasIndexScope() {
* 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
Expand Down Expand Up @@ -599,6 +617,13 @@ void checkDeprecation(Settings settings) {
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);
}
Expand Down
14 changes: 14 additions & 0 deletions server/src/main/java/org/elasticsearch/index/IndexSettings.java
Original file line number Diff line number Diff line change
Expand Up @@ -550,6 +550,20 @@ public Iterator<Setting<?>> settings() {
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;
Expand Down
14 changes: 14 additions & 0 deletions server/src/main/java/org/elasticsearch/index/IndexingSlowLog.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
);
Expand Down
14 changes: 14 additions & 0 deletions server/src/main/java/org/elasticsearch/index/SearchSlowLog.java
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Loading