Browse Source

Permit removal of archived index settings (#86107)

Today you cannot remove archived index settings by applying a setting
update `{"archived.*":null}` because `IndexSettings#same` incorrectly
treats such an update as a no-op. This commit fixes that.
David Turner 3 years ago
parent
commit
e70dc48220

+ 5 - 0
docs/changelog/86107.yaml

@@ -0,0 +1,5 @@
+pr: 86107
+summary: Permit removal of archived index settings
+area: Infra/Settings
+type: bug
+issues: []

+ 4 - 10
docs/reference/upgrade/archived-settings.asciidoc

@@ -66,20 +66,14 @@ GET */_settings?flat_settings=true&filter_path=**.settings.archived*
 ----
 // TEST[s/^/PUT my-index\n/]
 
-Removing an index's archived index settings requires a <<docs-reindex,reindex>>.
-Reindexing can be resource-intensive and  time-consuming. Before you start, test
-the reindex with a subset of the data to estimate your time requirements.
+To remove any archived index settings, use the following
+<<indices-update-settings,indices update settings>> request.
 
 [source,console]
 ----
-POST _reindex
+PUT /my-index/_settings
 {
-  "source": {
-    "index": "my-index"
-  },
-  "dest": {
-    "index": "reindexed-my-index"
-  }
+  "archived.*": null
 }
 ----
 // TEST[s/^/PUT my-index\n/]

+ 55 - 0
server/src/internalClusterTest/java/org/elasticsearch/index/IndexSettingsIT.java

@@ -0,0 +1,55 @@
+/*
+ * 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 org.elasticsearch.common.settings.Setting;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.util.CollectionUtils;
+import org.elasticsearch.plugins.Plugin;
+import org.elasticsearch.test.ESIntegTestCase;
+
+import java.util.Collection;
+import java.util.List;
+
+import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
+import static org.hamcrest.Matchers.equalTo;
+
+public class IndexSettingsIT extends ESIntegTestCase {
+
+    public static final Setting<Boolean> TEST_SETTING = Setting.boolSetting("index.test_setting", false, Setting.Property.IndexScope);
+
+    public static class TestPlugin extends Plugin {
+        @Override
+        public List<Setting<?>> getSettings() {
+            return registerSetting ? List.of(TEST_SETTING) : List.of();
+        }
+    }
+
+    private static volatile boolean registerSetting = true;
+
+    @Override
+    protected Collection<Class<? extends Plugin>> nodePlugins() {
+        return CollectionUtils.appendToCopy(super.nodePlugins(), TestPlugin.class);
+    }
+
+    public void testCanRemoveArchivedSettings() throws Exception {
+        createIndex("test", Settings.builder().put(TEST_SETTING.getKey(), "true").build());
+        registerSetting = false;
+        try {
+            internalCluster().fullRestart();
+
+            final var indicesClient = client().admin().indices();
+            assertThat(indicesClient.prepareGetSettings("test").get().getSetting("test", "archived.index.test_setting"), equalTo("true"));
+            assertAcked(indicesClient.prepareUpdateSettings("test").setSettings(Settings.builder().putNull("archived.*")));
+            assertNull(indicesClient.prepareGetSettings("test").get().getSetting("test", "archived.index.test_setting"));
+        } finally {
+            registerSetting = true;
+        }
+    }
+}

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

@@ -15,6 +15,7 @@ import org.elasticsearch.Version;
 import org.elasticsearch.cluster.metadata.IndexMetadata;
 import org.elasticsearch.cluster.routing.IndexRouting;
 import org.elasticsearch.common.logging.Loggers;
+import org.elasticsearch.common.settings.AbstractScopedSettings;
 import org.elasticsearch.common.settings.IndexScopedSettings;
 import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Setting.Property;
@@ -983,7 +984,9 @@ public final class IndexSettings {
         if (left.equals(right)) {
             return true;
         }
-        return left.getByPrefix(IndexMetadata.INDEX_SETTING_PREFIX).equals(right.getByPrefix(IndexMetadata.INDEX_SETTING_PREFIX));
+        return left.getByPrefix(IndexMetadata.INDEX_SETTING_PREFIX).equals(right.getByPrefix(IndexMetadata.INDEX_SETTING_PREFIX))
+            && left.getByPrefix(AbstractScopedSettings.ARCHIVED_SETTINGS_PREFIX)
+                .equals(right.getByPrefix(AbstractScopedSettings.ARCHIVED_SETTINGS_PREFIX));
     }
 
     /**

+ 43 - 0
server/src/test/java/org/elasticsearch/index/IndexSettingsTests.java

@@ -749,4 +749,47 @@ public class IndexSettingsTests extends ESTestCase {
         IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> new IndexSettings(metadata, Settings.EMPTY));
         assertThat(e.getMessage(), Matchers.containsString("index.time_series.end_time must be larger than index.time_series.start_time"));
     }
+
+    public void testSame() {
+        final var indexSettingKey = "index.example.setting";
+        final var archivedSettingKey = "archived.example.setting";
+        final var otherSettingKey = "other.example.setting";
+
+        final var builder = Settings.builder();
+        if (randomBoolean()) {
+            builder.put(indexSettingKey, randomAlphaOfLength(10));
+        }
+        if (randomBoolean()) {
+            builder.put(archivedSettingKey, randomAlphaOfLength(10));
+        }
+        if (randomBoolean()) {
+            builder.put(otherSettingKey, randomAlphaOfLength(10));
+        }
+        final var settings = builder.build();
+        assertTrue(IndexSettings.same(settings, Settings.builder().put(settings).build()));
+
+        final var differentIndexSettingBuilder = Settings.builder().put(settings);
+        if (settings.hasValue(indexSettingKey) && randomBoolean()) {
+            differentIndexSettingBuilder.putNull(indexSettingKey);
+        } else {
+            differentIndexSettingBuilder.put(indexSettingKey, randomAlphaOfLength(11));
+        }
+        assertFalse(IndexSettings.same(settings, differentIndexSettingBuilder.build()));
+
+        final var differentArchivedSettingBuilder = Settings.builder().put(settings);
+        if (settings.hasValue(archivedSettingKey) && randomBoolean()) {
+            differentArchivedSettingBuilder.putNull(archivedSettingKey);
+        } else {
+            differentArchivedSettingBuilder.put(archivedSettingKey, randomAlphaOfLength(11));
+        }
+        assertFalse(IndexSettings.same(settings, differentArchivedSettingBuilder.build()));
+
+        final var differentOtherSettingBuilder = Settings.builder().put(settings);
+        if (settings.hasValue(otherSettingKey) && randomBoolean()) {
+            differentOtherSettingBuilder.putNull(otherSettingKey);
+        } else {
+            differentOtherSettingBuilder.put(otherSettingKey, randomAlphaOfLength(11));
+        }
+        assertTrue(IndexSettings.same(settings, differentOtherSettingBuilder.build()));
+    }
 }