Skip archive settings on upgrade (elastic#139297)

We are removing archiving and validation of index settings during startup.
We are doing so because, the work is redundant, as during a rolling upgrade, the Master node will send the updated metadata.
In the case of a full cluster restart, the index settings won't be archived, but the cluster will still be healthy; validation is still present when opening the index.

Author: mamazzol

server/src/internalClusterTest/java/org/elasticsearch/gateway/GatewayIndexStateIT.java

@@ -389,8 +389,6 @@ public void testRecoverBrokenIndexMetadata() throws Exception {
                 Settings.builder()
                     .put(metadata.getSettings())
                     .put(IndexMetadata.SETTING_VERSION_CREATED, IndexVersions.MINIMUM_COMPATIBLE)
-                    // this is invalid but should be archived
-                    .put("index.similarity.BM25.type", "boolean")
                     // this one is not validated ahead of time and breaks allocation
                     .put("index.analysis.filter.myCollator.type", "icu_collation")
             );
@@ -415,10 +413,6 @@ public void testRecoverBrokenIndexMetadata() throws Exception {
 
         state = clusterAdmin().prepareState(TEST_REQUEST_TIMEOUT).get().getState();
         assertEquals(IndexMetadata.State.CLOSE, state.getMetadata().getProject().index(metadata.getIndex()).getState());
-        assertEquals(
-            "boolean",
-            state.getMetadata().getProject().index(metadata.getIndex()).getSettings().get("archived.index.similarity.BM25.type")
-        );
         // try to open it with the broken setting - fail again!
         ElasticsearchException ex = expectThrows(ElasticsearchException.class, indicesAdmin().prepareOpen("test"));
         assertEquals(ex.getMessage(), "Failed to verify index " + metadata.getIndex());

server/src/internalClusterTest/java/org/elasticsearch/index/IndexSettingsIT.java

@@ -84,16 +84,33 @@ public IndexMetadataVerifier(
     }
 
     /**
-     * Checks that the index can be upgraded to the current version of the master node.
+     * Checks that the index can be upgraded to the current version as provided.
      *
      * <p>
      * If the index does not need upgrade it returns the index metadata unchanged, otherwise it returns a modified index metadata. If index
-     * cannot be updated the method throws an exception.
+     * cannot be updated the method throws an exception. Also archives or deletes unsupported settings.
      */
     public IndexMetadata verifyIndexMetadata(
         IndexMetadata indexMetadata,
         IndexVersion minimumIndexCompatibilityVersion,
         IndexVersion minimumReadOnlyIndexCompatibilityVersion
+    ) {
+        return verifyIndexMetadata(indexMetadata, minimumIndexCompatibilityVersion, minimumReadOnlyIndexCompatibilityVersion, true);
+    }
+
+    /**
+     * Checks that the index can be upgraded to the current version as provided.
+     *
+     * <p>
+     * If the index does not need upgrade it returns the index metadata unchanged, otherwise it returns a modified index metadata. If index
+     * cannot be updated the method throws an exception. If {@code archiveOrDeleteBrokenIndexSettings} is {@code true},
+     * unsupported settings will be either archived or deleted.
+     */
+    public IndexMetadata verifyIndexMetadata(
+        IndexMetadata indexMetadata,
+        IndexVersion minimumIndexCompatibilityVersion,
+        IndexVersion minimumReadOnlyIndexCompatibilityVersion,
+        boolean archiveOrDeleteBrokenIndexSettings
     ) {
         checkSupportedVersion(indexMetadata, minimumIndexCompatibilityVersion, minimumReadOnlyIndexCompatibilityVersion);
 
@@ -103,10 +120,12 @@ public IndexMetadata verifyIndexMetadata(
         // invalid settings, since they are now removed the FilterAllocationDecider treats them as
         // regular attribute filters, and shards cannot be allocated.
         newMetadata = removeTierFiltering(newMetadata);
-        // Next we have to run this otherwise if we try to create IndexSettings
-        // with broken settings it would fail in checkMappingsCompatibility
-        newMetadata = archiveOrDeleteBrokenIndexSettings(newMetadata);
-        checkMappingsCompatibility(newMetadata);
+        if (archiveOrDeleteBrokenIndexSettings) {
+            // Next we have to run this otherwise if we try to create IndexSettings
+            // with broken settings it would fail in checkMappingsCompatibility
+            newMetadata = archiveOrDeleteBrokenIndexSettings(newMetadata);
+            checkMappingsCompatibility(newMetadata);
+        }
         return newMetadata;
     }
 
@@ -315,6 +334,7 @@ IndexMetadata archiveOrDeleteBrokenIndexSettings(IndexMetadata indexMetadata) {
         final Settings newSettings;
 
         if (indexMetadata.isSystem()) {
+            // TODO This is also skipped if the upgrade/archive step is skipped. To be discussed
             newSettings = indexScopedSettings.deleteUnknownOrInvalidSettings(
                 settings,
                 e -> logger.warn(

server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetadataVerifier.java

@@ -318,7 +318,8 @@ private static ProjectMetadata upgradeProjectMetadata(
             IndexMetadata newMetadata = indexMetadataVerifier.verifyIndexMetadata(
                 indexMetadata,
                 IndexVersions.MINIMUM_COMPATIBLE,
-                IndexVersions.MINIMUM_READONLY_COMPATIBLE
+                IndexVersions.MINIMUM_READONLY_COMPATIBLE,
+                false
             );
             changed |= indexMetadata != newMetadata;
             upgradedMetadata.put(newMetadata, false);

server/src/main/java/org/elasticsearch/gateway/GatewayMetaState.java

@@ -294,6 +294,27 @@ public void testReadOnlyVersionCompatibility() {
         }
     }
 
+    public void testSkippingArchiveOrDeletePreservesBrokenSettings() {
+        IndexMetadataVerifier service = getIndexMetadataVerifier();
+        IndexMetadata src = newIndexMeta(
+            "foo",
+            Settings.builder()
+                .put(IndexMetadata.SETTING_VERSION_CREATED, IndexVersion.current())
+                .put("index.refresh_interval", "-200")
+                .build()
+        );
+
+        IndexMetadata result = service.verifyIndexMetadata(
+            src,
+            IndexVersions.MINIMUM_COMPATIBLE,
+            IndexVersions.MINIMUM_READONLY_COMPATIBLE,
+            false
+        );
+
+        assertEquals("-200", result.getSettings().get("index.refresh_interval"));
+        assertNull(result.getSettings().get("archived.index.refresh_interval"));
+    }
+
     private IndexMetadataVerifier getIndexMetadataVerifier() {
         return new IndexMetadataVerifier(
             Settings.EMPTY,

server/src/test/java/org/elasticsearch/cluster/metadata/IndexMetadataVerifierTests.java

@@ -302,6 +302,16 @@ public IndexMetadata verifyIndexMetadata(
         ) {
             return upgrade ? IndexMetadata.builder(indexMetadata).build() : indexMetadata;
         }
+
+        @Override
+        public IndexMetadata verifyIndexMetadata(
+            IndexMetadata indexMetadata,
+            IndexVersion minimumIndexCompatibilityVersion,
+            IndexVersion minimumReadOnlyIndexCompatibilityVersion,
+            boolean archiveOrDeleteBrokenIndexSettings
+        ) {
+            return upgrade ? IndexMetadata.builder(indexMetadata).build() : indexMetadata;
+        }
     }
 
     private static class CustomMetadata1 extends TestClusterCustomMetadata {