Skip to content
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

[Backport][BUG] Add support to clear archived index settings #9508

Merged
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- Fix flaky ResourceAwareTasksTests.testBasicTaskResourceTracking test ([#8993](https://github.com/opensearch-project/OpenSearch/pull/8993))
- Fix memory leak when using Zstd Dictionary ([#9403](https://github.com/opensearch-project/OpenSearch/pull/9403))
- Fix condition to remove index create block ([#9437](https://github.com/opensearch-project/OpenSearch/pull/9437))
- Add support to clear archived index setting ([#9019](https://github.com/opensearch-project/OpenSearch/pull/9019))
andrross marked this conversation as resolved.
Show resolved Hide resolved

### Security

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*/

package org.opensearch.indices.settings;

import org.opensearch.common.settings.Setting;
import org.opensearch.common.settings.Settings;
import org.opensearch.plugins.Plugin;
import org.opensearch.test.InternalTestCluster;
import org.opensearch.test.OpenSearchIntegTestCase;

import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.List;

import static org.hamcrest.Matchers.startsWith;

@OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST, numDataNodes = 0, supportsDedicatedMasters = false)
public class ArchivedIndexSettingsIT extends OpenSearchIntegTestCase {
private volatile boolean installPlugin;

public void testArchiveSettings() throws Exception {
installPlugin = true;
// Set up the cluster with an index containing dummy setting(owned by dummy plugin)
String oldClusterManagerNode = internalCluster().startClusterManagerOnlyNode();
String oldDataNode = internalCluster().startDataOnlyNode();
assertEquals(2, internalCluster().numDataAndClusterManagerNodes());
createIndex("test");
ensureYellow();
// Add a dummy setting
client().admin()
.indices()
.prepareUpdateSettings("test")
.setSettings(Settings.builder().put("index.dummy", "foobar").put("index.dummy2", "foobar"))
.execute()
.actionGet();

// Remove dummy plugin and replace the cluster manager node so that the stale plugin setting moves to "archived".
installPlugin = false;
String newClusterManagerNode = internalCluster().startClusterManagerOnlyNode();
internalCluster().stopRandomNode(InternalTestCluster.nameFilter(oldClusterManagerNode));
internalCluster().restartNode(newClusterManagerNode);

// Verify that archived settings exists.
assertTrue(
client().admin().indices().prepareGetSettings("test").get().getIndexToSettings().get("test").hasValue("archived.index.dummy")
);
assertTrue(
client().admin().indices().prepareGetSettings("test").get().getIndexToSettings().get("test").hasValue("archived.index.dummy2")
);

// Archived setting update should fail on open index.
IllegalArgumentException exception = expectThrows(
IllegalArgumentException.class,
() -> client().admin()
.indices()
.prepareUpdateSettings("test")
.setSettings(Settings.builder().putNull("archived.index.dummy"))
.execute()
.actionGet()
);
assertThat(
exception.getMessage(),
startsWith("Can't update non dynamic settings [[archived.index.dummy]] for open indices [[test")
);

// close the index.
client().admin().indices().prepareClose("test").get();

// Remove archived.index.dummy explicitly.
assertTrue(
client().admin()
.indices()
.prepareUpdateSettings("test")
.setSettings(Settings.builder().putNull("archived.index.dummy"))
.execute()
.actionGet()
.isAcknowledged()
);

// Remove archived.index.dummy2 using wildcard.
assertTrue(
client().admin()
.indices()
.prepareUpdateSettings("test")
.setSettings(Settings.builder().putNull("archived.*"))
.execute()
.actionGet()
.isAcknowledged()
);

// Verify that archived settings are cleaned up successfully.
assertFalse(
client().admin().indices().prepareGetSettings("test").get().getIndexToSettings().get("test").hasValue("archived.index.dummy")
);
assertFalse(
client().admin().indices().prepareGetSettings("test").get().getIndexToSettings().get("test").hasValue("archived.index.dummy2")
);
}

@Override
protected Collection<Class<? extends Plugin>> nodePlugins() {
return installPlugin ? Arrays.asList(DummySettingPlugin.class) : Collections.emptyList();
}

public static class DummySettingPlugin extends Plugin {
public static final Setting<String> DUMMY_SETTING = Setting.simpleString(
"index.dummy",
Setting.Property.IndexScope,
Setting.Property.Dynamic
);
public static final Setting<String> DUMMY_SETTING2 = Setting.simpleString(
"index.dummy2",
Setting.Property.IndexScope,
Setting.Property.Dynamic
);

@Override
public List<Setting<?>> getSettings() {
return Arrays.asList(DUMMY_SETTING, DUMMY_SETTING2);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@
import java.util.Set;

import static org.opensearch.action.support.ContextPreservingActionListener.wrapPreservingContext;
import static org.opensearch.common.settings.AbstractScopedSettings.ARCHIVED_SETTINGS_PREFIX;
import static org.opensearch.index.IndexSettings.same;

/**
Expand Down Expand Up @@ -132,20 +133,25 @@ public void updateSettings(
indexScopedSettings.validate(
normalizedSettings.filter(s -> Regex.isSimpleMatchPattern(s) == false), // don't validate wildcards
false, // don't validate dependencies here we check it below never allow to change the number of shards
false,
true, // Ignore archived setting.
true
); // validate internal or private index settings
for (String key : normalizedSettings.keySet()) {
Setting setting = indexScopedSettings.get(key);
boolean isWildcard = setting == null && Regex.isSimpleMatchPattern(key);
boolean isArchived = key.startsWith(ARCHIVED_SETTINGS_PREFIX);
assert setting != null // we already validated the normalized settings
|| isArchived
|| (isWildcard && normalizedSettings.hasValue(key) == false) : "unknown setting: "
+ key
+ " isWildcard: "
+ isWildcard
+ " hasValue: "
+ normalizedSettings.hasValue(key);
settingsForClosedIndices.copy(key, normalizedSettings);
if (isWildcard || setting.isDynamic()) {
// Only allow dynamic settings and wildcards for open indices. Skip archived settings.
if (isArchived == false && (isWildcard || setting.isDynamic())) {
settingsForOpenIndices.copy(key, normalizedSettings);
} else {
skippedSettings.add(key);
Expand Down Expand Up @@ -305,6 +311,8 @@ public ClusterState execute(ClusterState currentState) {
Settings finalSettings = indexSettings.build();
indexScopedSettings.validate(
finalSettings.filter(k -> indexScopedSettings.isPrivateSetting(k) == false),
true,
false,
true
);
metadataBuilder.put(IndexMetadata.builder(indexMetadata).settings(finalSettings));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ public final class IndexScopedSettings extends AbstractScopedSettings {

public static final Predicate<String> INDEX_SETTINGS_KEY_PREDICATE = (s) -> s.startsWith(IndexMetadata.INDEX_SETTING_PREFIX);

public static final Predicate<String> ARCHIVED_SETTINGS_KEY_PREDICATE = (s) -> s.startsWith(ARCHIVED_SETTINGS_PREFIX);

public static final Set<Setting<?>> BUILT_IN_INDEX_SETTINGS = Collections.unmodifiableSet(
new HashSet<>(
Arrays.asList(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@
import java.util.stream.Collectors;
import java.util.stream.Stream;

import static org.opensearch.common.settings.AbstractScopedSettings.ARCHIVED_SETTINGS_PREFIX;
import static org.opensearch.common.unit.TimeValue.parseTimeValue;
import static org.opensearch.core.common.unit.ByteSizeValue.parseBytesSizeValue;

Expand Down Expand Up @@ -1217,7 +1218,7 @@ public boolean shouldRemoveMissingPlaceholder(String placeholderName) {
}

/**
* Checks that all settings in the builder start with the specified prefix.
* Checks that all settings(except archived settings and wildcards) in the builder start with the specified prefix.
*
* If a setting doesn't start with the prefix, the builder appends the prefix to such setting.
*/
Expand All @@ -1227,7 +1228,7 @@ public Builder normalizePrefix(String prefix) {
while (iterator.hasNext()) {
Map.Entry<String, Object> entry = iterator.next();
String key = entry.getKey();
if (key.startsWith(prefix) == false && key.endsWith("*") == false) {
if (key.startsWith(prefix) == false && key.endsWith("*") == false && key.startsWith(ARCHIVED_SETTINGS_PREFIX) == false) {
replacements.put(prefix + key, entry.getValue());
iterator.remove();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1145,7 +1145,9 @@ public synchronized boolean updateIndexMetadata(IndexMetadata indexMetadata) {
*/
public static boolean same(final Settings left, final Settings right) {
return left.filter(IndexScopedSettings.INDEX_SETTINGS_KEY_PREDICATE)
.equals(right.filter(IndexScopedSettings.INDEX_SETTINGS_KEY_PREDICATE));
.equals(right.filter(IndexScopedSettings.INDEX_SETTINGS_KEY_PREDICATE))
&& left.filter(IndexScopedSettings.ARCHIVED_SETTINGS_KEY_PREDICATE)
.equals(right.filter(IndexScopedSettings.ARCHIVED_SETTINGS_KEY_PREDICATE));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,20 @@ public void testPrefixNormalization() {
assertThat(settings.get("foo.test"), equalTo("test"));
}

public void testPrefixNormalizationArchived() {
Settings settings = Settings.builder().put("archived.foo.bar", "baz").normalizePrefix("foo.").build();

assertThat(settings.size(), equalTo(1));
assertThat(settings.get("foo.archived.foo.bar"), nullValue());
assertThat(settings.get("archived.foo.bar"), equalTo("baz"));

settings = Settings.builder().put("archived.foo.*", "baz").normalizePrefix("foo.").build();

assertThat(settings.size(), equalTo(1));
assertThat(settings.get("foo.archived.foo.*"), nullValue());
assertThat(settings.get("archived.foo.*"), equalTo("baz"));
}

public void testFilteredMap() {
Settings.Builder builder = Settings.builder();
builder.put("a", "a1");
Expand Down
Loading