Skip to content

Commit

Permalink
[BUG] Add support to clear archived index settings (opensearch-projec…
Browse files Browse the repository at this point in the history
…t#9019)

* Add support to clear archived index setting

Signed-off-by: Ankit Kala <[email protected]>
  • Loading branch information
ankitkala authored Aug 22, 2023
1 parent dd75a22 commit ebdffbb
Show file tree
Hide file tree
Showing 7 changed files with 161 additions and 4 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
### Fixed
- Fix 'org.apache.hc.core5.http.ParseException: Invalid protocol version' under JDK 16+ ([#4827](https://github.com/opensearch-project/OpenSearch/pull/4827))
- Fix compression support for h2c protocol ([#4944](https://github.com/opensearch-project/OpenSearch/pull/4944))
- Add support to clear archived index setting ([#9019](https://github.com/opensearch-project/OpenSearch/pull/9019))

### 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 @@ -135,20 +136,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 @@ -308,6 +314,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
4 changes: 3 additions & 1 deletion server/src/main/java/org/opensearch/index/IndexSettings.java
Original file line number Diff line number Diff line change
Expand Up @@ -1147,7 +1147,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 @@ -301,6 +301,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

0 comments on commit ebdffbb

Please sign in to comment.