diff --git a/CHANGELOG.md b/CHANGELOG.md index a1ca2ed152555..cea9ca1fb1eb2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -96,6 +96,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)) diff --git a/server/src/internalClusterTest/java/org/opensearch/cluster/settings/ClusterSettingsIT.java b/server/src/internalClusterTest/java/org/opensearch/cluster/settings/ClusterSettingsIT.java index 59542f886dcac..6a17118d21d6b 100644 --- a/server/src/internalClusterTest/java/org/opensearch/cluster/settings/ClusterSettingsIT.java +++ b/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 settingToVerify = new HashMap<>(); + settingToVerify.put("indices.fielddata.cache.size", false); + settingToVerify.put("indices.queries.cache.skip_cache_factor", false); + ClusterSettings clusterSettings = clusterService().getClusterSettings(); + List> 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 entry : settingToVerify.entrySet()) { + if (!entry.getValue()) { + fail("Was not able to read this setting: " + entry.getKey()); + } + } + } + } diff --git a/server/src/main/java/org/opensearch/common/settings/AbstractScopedSettings.java b/server/src/main/java/org/opensearch/common/settings/AbstractScopedSettings.java index 8c10623e48fe4..d3ed1d839093f 100644 --- a/server/src/main/java/org/opensearch/common/settings/AbstractScopedSettings.java +++ b/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> getSettingUpdaters() { + return Collections.unmodifiableList(settingUpdaters); + } + protected void validateSettingKey(Setting setting) { if (isValidKey(setting.getKey()) == false && (setting.isGroupSetting() && isValidGroupKey(setting.getKey()) || isValidAffixKey(setting.getKey())) == false diff --git a/server/src/main/java/org/opensearch/indices/IndicesService.java b/server/src/main/java/org/opensearch/indices/IndicesService.java index 6e922c18c79ff..6d4c3610ded70 100644 --- a/server/src/main/java/org/opensearch/indices/IndicesService.java +++ b/server/src/main/java/org/opensearch/indices/IndicesService.java @@ -1228,11 +1228,6 @@ public synchronized MapperService createIndexMapperService(IndexMetadata indexMe public synchronized void verifyIndexMetadata(IndexMetadata metadata, IndexMetadata metadataUpdate) throws IOException { final List 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,