Addressing review comments

Signed-off-by: Ankit Jain <[email protected]>
opensearch-project · Oct 10, 2024 · d55a3d2 · d55a3d2
1 parent da2e3dd
commit d55a3d2
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 11 deletions.
diff --git a/src/main/java/org/opensearch/plugin/insights/QueryInsightsPlugin.java b/src/main/java/org/opensearch/plugin/insights/QueryInsightsPlugin.java
@@ -31,7 +31,6 @@
 import org.opensearch.plugin.insights.core.listener.QueryInsightsListener;
 import org.opensearch.plugin.insights.core.metrics.OperationalMetricsCounter;
 import org.opensearch.plugin.insights.core.service.QueryInsightsService;
-import org.opensearch.plugin.insights.core.service.categorizer.IndicesFieldTypeCache;
 import org.opensearch.plugin.insights.rules.action.health_stats.HealthStatsAction;
 import org.opensearch.plugin.insights.rules.action.top_queries.TopQueriesAction;
 import org.opensearch.plugin.insights.rules.resthandler.health_stats.RestHealthStatsAction;
@@ -146,7 +145,7 @@ public List<Setting<?>> getSettings() {
             QueryInsightsSettings.TOP_N_QUERIES_GROUPING_FIELD_NAME,
             QueryInsightsSettings.TOP_N_QUERIES_GROUPING_FIELD_TYPE,
             QueryCategorizationSettings.SEARCH_QUERY_METRICS_ENABLED_SETTING,
-            IndicesFieldTypeCache.INDICES_FIELD_TYPE_CACHE_SIZE_KEY
+            QueryCategorizationSettings.SEARCH_QUERY_FIELD_TYPE_CACHE_SIZE_KEY
         );
     }
 }
diff --git a/...n/java/org/opensearch/plugin/insights/core/service/categorizer/IndicesFieldTypeCache.java b/...n/java/org/opensearch/plugin/insights/core/service/categorizer/IndicesFieldTypeCache.java
@@ -1,3 +1,11 @@
+/*
+ * 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.plugin.insights.core.service.categorizer;
 
 import java.util.concurrent.ConcurrentHashMap;
@@ -8,23 +16,21 @@
 import org.opensearch.common.cache.Cache;
 import org.opensearch.common.cache.CacheBuilder;
 import org.opensearch.common.metrics.CounterMetric;
-import org.opensearch.common.settings.Setting;
 import org.opensearch.common.settings.Settings;
-import org.opensearch.core.common.unit.ByteSizeValue;
 import org.opensearch.core.index.Index;
+import org.opensearch.plugin.insights.settings.QueryCategorizationSettings;
 
+/**
+ * Cache implementation specifically for maintaining the field name type mappings
+ * for indices that are part of successful search requests
+ */
 public class IndicesFieldTypeCache {
 
     private static final Logger logger = LogManager.getLogger(IndicesFieldTypeCache.class);
-    public static final Setting<ByteSizeValue> INDICES_FIELD_TYPE_CACHE_SIZE_KEY = Setting.memorySizeSetting(
-        "search.insights.indices.fieldtype.cache.size",
-        new ByteSizeValue(-1),
-        Setting.Property.NodeScope
-    );
     private final Cache<Index, IndexFieldMap> cache;
 
     public IndicesFieldTypeCache(Settings settings) {
-        final long sizeInBytes = -1; // TODO: INDICES_FIELD_TYPE_CACHE_SIZE_KEY.get(settings).getBytes();
+        final long sizeInBytes = QueryCategorizationSettings.SEARCH_QUERY_FIELD_TYPE_CACHE_SIZE_KEY.get(settings).getBytes();
         CacheBuilder<Index, IndexFieldMap> cacheBuilder = CacheBuilder.<Index, IndexFieldMap>builder();
         if (sizeInBytes > 0) {
             cacheBuilder.setMaximumWeight(sizeInBytes).weigher((k, v) -> RamUsageEstimator.sizeOfObject(k) + v.weight());
@@ -39,6 +45,9 @@ public IndexFieldMap getOrInitialize(Index index) {
             logger.error("Unexpected execution exception while initializing for index " + index);
         }
 
+        // Should never return null as the ExecutionException is only thrown
+        // if loader throws an exception or returns a null value, which cannot
+        // be the case in this scenario
         return null;
     }
 
@@ -64,7 +73,7 @@ public String get(String fieldName) {
         }
 
         public void putIfAbsent(String key, String value) {
-            // Increment the weight only if the key value got added to the Map
+            // Increment the weight only if the key value pair added to the Map
             if (fieldTypeMap.putIfAbsent(key, value) == null) {
                 weight.inc(RamUsageEstimator.sizeOf(key) + RamUsageEstimator.sizeOf(value));
             }

diff --git a/src/main/java/org/opensearch/plugin/insights/settings/QueryCategorizationSettings.java b/src/main/java/org/opensearch/plugin/insights/settings/QueryCategorizationSettings.java
@@ -9,6 +9,7 @@
 package org.opensearch.plugin.insights.settings;
 
 import org.opensearch.common.settings.Setting;
+import org.opensearch.core.common.unit.ByteSizeValue;
 
 /**
  * Settings for Query Categorization
@@ -24,6 +25,12 @@ public class QueryCategorizationSettings {
         Setting.Property.Dynamic
     );
 
+    public static final Setting<ByteSizeValue> SEARCH_QUERY_FIELD_TYPE_CACHE_SIZE_KEY = Setting.memorySizeSetting(
+        "search.query.fieldtype.cache.size",
+        "0.1%",
+        Setting.Property.NodeScope
+    );
+
     /**
      * Default constructor
      */