Fix duplicate registration of FieldDataCache dynamic setting (#20140)

Signed-off-by: Sagar Upadhyaya <[email protected]>
Signed-off-by: Sagar <[email protected]>

Author: sgup432

CHANGELOG.md

@@ -98,6 +98,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Fix toBuilder method in EngineConfig to include mergedSegmentTransferTracker([#20105](https://github.com/opensearch-project/OpenSearch/pull/20105))
 - Fixed handling of property index in BulkRequest during deserialization ([#20132](https://github.com/opensearch-project/OpenSearch/pull/20132))
 - Fix negative CPU usage values in node stats ([#19120](https://github.com/opensearch-project/OpenSearch/issues/19120))
+- Fix duplicate registration of FieldDataCache dynamic setting ([20140](https://github.com/opensearch-project/OpenSearch/pull/20140))
 
 ### Dependencies
 - Bump Apache Lucene from 10.3.1 to 10.3.2 ([#20026](https://github.com/opensearch-project/OpenSearch/pull/20026))

server/src/internalClusterTest/java/org/opensearch/cluster/settings/ClusterSettingsIT.java

@@ -37,18 +37,27 @@
 import org.opensearch.action.admin.cluster.settings.ClusterUpdateSettingsRequestBuilder;
 import org.opensearch.action.admin.cluster.settings.ClusterUpdateSettingsResponse;
 import org.opensearch.action.admin.cluster.state.ClusterStateResponse;
+import org.opensearch.cluster.metadata.IndexMetadata;
 import org.opensearch.cluster.metadata.Metadata;
 import org.opensearch.cluster.routing.allocation.decider.EnableAllocationDecider;
+import org.opensearch.common.settings.AbstractScopedSettings;
+import org.opensearch.common.settings.ClusterSettings;
 import org.opensearch.common.settings.Setting;
 import org.opensearch.common.settings.Settings;
 import org.opensearch.common.settings.SettingsException;
 import org.opensearch.common.unit.TimeValue;
 import org.opensearch.core.common.unit.ByteSizeUnit;
+import org.opensearch.indices.IndicesQueryCache;
 import org.opensearch.indices.recovery.RecoverySettings;
 import org.opensearch.test.OpenSearchIntegTestCase;
 import org.junit.After;
 
 import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 import static org.opensearch.cluster.routing.allocation.DiskThresholdSettings.CLUSTER_ROUTING_ALLOCATION_REROUTE_INTERVAL_SETTING;
 import static org.opensearch.cluster.routing.allocation.decider.ThrottlingAllocationDecider.CLUSTER_ROUTING_ALLOCATION_NODE_INITIAL_PRIMARIES_RECOVERIES_SETTING;
@@ -72,6 +81,14 @@ public void cleanup() throws Exception {
         );
     }
 
+    @Override
+    protected Settings nodeSettings(int nodeOrdinal) {
+        return Settings.builder()
+            .put(super.nodeSettings(nodeOrdinal))
+            .put(IndicesQueryCache.INDICES_QUERIES_CACHE_ALL_SEGMENTS_SETTING.getKey(), false)
+            .build();
+    }
+
     public void testClusterNonExistingSettingsUpdate() {
         String key1 = "no_idea_what_you_are_talking_about";
         int value1 = 10;
@@ -552,4 +569,39 @@ public void testUserMetadata() {
         }
     }
 
+    public void testWithMultipleIndexCreationAndVerifySettingRegisteredOnce() {
+        int randomInt = randomIntBetween(10, 50);
+        for (int i = 0; i < randomInt; i++) {
+            String indexName = "test" + i;
+            assertAcked(prepareCreate(indexName).setSettings(Settings.builder().put(IndexMetadata.SETTING_BLOCKS_METADATA, true)));
+            assertBlocked(client().admin().indices().prepareGetSettings(indexName), IndexMetadata.INDEX_METADATA_BLOCK);
+            disableIndexBlock(indexName, IndexMetadata.SETTING_BLOCKS_METADATA);
+        }
+        Map<String, Boolean> settingToVerify = new HashMap<>();
+        settingToVerify.put("indices.fielddata.cache.size", false);
+        settingToVerify.put("indices.queries.cache.skip_cache_factor", false);
+        ClusterSettings clusterSettings = clusterService().getClusterSettings();
+        List<AbstractScopedSettings.SettingUpdater<?>> settingUpgraders = clusterSettings.getSettingUpdaters();
+        for (AbstractScopedSettings.SettingUpdater<?> settingUpgrader : settingUpgraders) {
+            String input = settingUpgrader.toString();
+            // Trying to fetch key value as there is no other way
+            Pattern p = Pattern.compile("\"key\"\\s*:\\s*\"([^\"]+)\"");
+            Matcher m = p.matcher(input);
+            String key = "";
+            if (m.find()) {
+                key = m.group(1);
+            }
+            if (settingToVerify.containsKey(key) && !settingToVerify.get(key)) {
+                settingToVerify.put(key, true);
+            } else if (settingToVerify.containsKey(key) && settingToVerify.get(key)) {
+                fail("Setting registered multiple times");
+            }
+        }
+        for (Map.Entry<String, Boolean> entry : settingToVerify.entrySet()) {
+            if (!entry.getValue()) {
+                fail("Was not able to read this setting: " + entry.getKey());
+            }
+        }
+    }
+
 }

server/src/main/java/org/opensearch/common/settings/AbstractScopedSettings.java

@@ -123,6 +123,14 @@ protected AbstractScopedSettings(
         this.keySettings = keySettings;
     }
 
+    /**
+     *  Returns the list of setting updaters registered in this scope.
+     * @return an unmodifiable view of the setting updaters
+     */
+    public List<SettingUpdater<?>> getSettingUpdaters() {
+        return Collections.unmodifiableList(settingUpdaters);
+    }
+
     protected void validateSettingKey(Setting<?> setting) {
         if (isValidKey(setting.getKey()) == false
             && (setting.isGroupSetting() && isValidGroupKey(setting.getKey()) || isValidAffixKey(setting.getKey())) == false

server/src/main/java/org/opensearch/indices/IndicesService.java

@@ -1232,11 +1232,6 @@ public synchronized MapperService createIndexMapperService(IndexMetadata indexMe
     public synchronized void verifyIndexMetadata(IndexMetadata metadata, IndexMetadata metadataUpdate) throws IOException {
         final List<Closeable> closeables = new ArrayList<>();
         try {
-            IndicesFieldDataCache indicesFieldDataCache = new IndicesFieldDataCache(settings, new IndexFieldDataCache.Listener() {
-            }, clusterService, threadPool);
-            closeables.add(indicesFieldDataCache);
-            IndicesQueryCache indicesQueryCache = new IndicesQueryCache(settings, clusterService.getClusterSettings());
-            closeables.add(indicesQueryCache);
             // this will also fail if some plugin fails etc. which is nice since we can verify that early
             final IndexService service = createIndexService(
                 METADATA_VERIFICATION,