change the setting to be int

Signed-off-by: bowenlan-amzn <[email protected]>
opensearch-project · Apr 15, 2024 · 22af708 · 22af708
1 parent b807d18
commit 22af708
Show file tree

Hide file tree

Showing 6 changed files with 24 additions and 21 deletions.
diff --git a/...c/internalClusterTest/java/org/opensearch/search/aggregations/bucket/FilterRewriteIT.java b/...c/internalClusterTest/java/org/opensearch/search/aggregations/bucket/FilterRewriteIT.java
@@ -35,6 +35,7 @@
 
 import static org.opensearch.common.xcontent.XContentFactory.jsonBuilder;
 import static org.opensearch.search.SearchService.CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING;
+import static org.opensearch.search.SearchService.MAX_AGGREGATION_REWRITE_FILTERS;
 import static org.opensearch.search.aggregations.AggregationBuilders.dateHistogram;
 import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked;
 
@@ -117,10 +118,10 @@ public void testDisableOptimizationGivesSameResults() throws Exception {
         final ClusterUpdateSettingsResponse updateSettingResponse = client().admin()
             .cluster()
             .prepareUpdateSettings()
-            .setTransientSettings(Settings.builder().put("search.filter_rewrite.enabled", false))
+            .setTransientSettings(Settings.builder().put(MAX_AGGREGATION_REWRITE_FILTERS.getKey(), 0))
             .get();
 
-        assertEquals(updateSettingResponse.getTransientSettings().get("search.filter_rewrite.enabled"), "false");
+        assertEquals(updateSettingResponse.getTransientSettings().get(MAX_AGGREGATION_REWRITE_FILTERS.getKey()), "0");
 
         response = client().prepareSearch("idx")
             .setSize(0)
@@ -134,7 +135,7 @@ public void testDisableOptimizationGivesSameResults() throws Exception {
         client().admin()
             .cluster()
             .prepareUpdateSettings()
-            .setTransientSettings(Settings.builder().putNull("search.filter_rewrite.enabled"))
+            .setTransientSettings(Settings.builder().putNull(MAX_AGGREGATION_REWRITE_FILTERS.getKey()))
             .get();
     }
 }
diff --git a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java
@@ -732,7 +732,7 @@ public void apply(Settings value, Settings current, Settings previous) {
                 RemoteStoreSettings.CLUSTER_REMOTE_TRANSLOG_BUFFER_INTERVAL_SETTING,
                 RemoteStoreSettings.CLUSTER_REMOTE_TRANSLOG_TRANSFER_TIMEOUT_SETTING,
 
-                SearchService.FILTER_REWRITE_SETTING
+                SearchService.MAX_AGGREGATION_REWRITE_FILTERS
             )
         )
     );

diff --git a/server/src/main/java/org/opensearch/search/DefaultSearchContext.java b/server/src/main/java/org/opensearch/search/DefaultSearchContext.java
@@ -107,7 +107,7 @@
 import java.util.function.LongSupplier;
 
 import static org.opensearch.search.SearchService.CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING;
-import static org.opensearch.search.SearchService.FILTER_REWRITE_SETTING;
+import static org.opensearch.search.SearchService.MAX_AGGREGATION_REWRITE_FILTERS;
 
 /**
  * The main search context used during search phase
@@ -188,7 +188,7 @@ final class DefaultSearchContext extends SearchContext {
     private final Function<SearchSourceBuilder, InternalAggregation.ReduceContextBuilder> requestToAggReduceContextBuilder;
     private final boolean concurrentSearchSettingsEnabled;
     private final SetOnce<Boolean> requestShouldUseConcurrentSearch = new SetOnce<>();
-    private boolean filterRewriteEnabled;
+    private final int maxAggRewriteFilters;
 
     DefaultSearchContext(
         ReaderContext readerContext,
@@ -243,7 +243,7 @@ final class DefaultSearchContext extends SearchContext {
         this.lowLevelCancellation = lowLevelCancellation;
         this.requestToAggReduceContextBuilder = requestToAggReduceContextBuilder;
 
-        this.filterRewriteEnabled = evaluateFilterRewriteSetting();
+        this.maxAggRewriteFilters = evaluateFilterRewriteSetting();
     }
 
     @Override
@@ -1000,14 +1000,14 @@ public boolean shouldUseTimeSeriesDescSortOptimization() {
     }
 
     @Override
-    public boolean isFilterRewriteEnabled() {
-        return filterRewriteEnabled;
+    public int maxAggRewriteFilters() {
+        return maxAggRewriteFilters;
     }
 
-    private boolean evaluateFilterRewriteSetting() {
+    private int evaluateFilterRewriteSetting() {
         if (clusterService != null) {
-            return clusterService.getClusterSettings().get(FILTER_REWRITE_SETTING);
+            return clusterService.getClusterSettings().get(MAX_AGGREGATION_REWRITE_FILTERS);
         }
-        return false;
+        return 0;
     }
 }
diff --git a/server/src/main/java/org/opensearch/search/SearchService.java b/server/src/main/java/org/opensearch/search/SearchService.java
@@ -272,9 +272,11 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv
         Property.NodeScope
     );
 
-    public static final Setting<Boolean> FILTER_REWRITE_SETTING = Setting.boolSetting(
-        "search.filter_rewrite.enabled",
-        true,
+    // value 0 means rewrite filters optimization in aggregations will be disabled
+    public static final Setting<Integer> MAX_AGGREGATION_REWRITE_FILTERS = Setting.intSetting(
+        "search.max_aggregation_rewrite_filters",
+        24,
+        0,
         Property.Dynamic,
         Property.NodeScope
     );

diff --git a/server/src/main/java/org/opensearch/search/aggregations/bucket/FastFilterRewriteHelper.java b/server/src/main/java/org/opensearch/search/aggregations/bucket/FastFilterRewriteHelper.java
@@ -63,7 +63,6 @@ private FastFilterRewriteHelper() {}
 
     private static final Logger logger = LogManager.getLogger(FastFilterRewriteHelper.class);
 
-    private static final int MAX_NUM_FILTER_BUCKETS = 24;
     private static final Map<Class<?>, Function<Query, Query>> queryWrappers;
 
     // Initialize the wrapper map for unwrapping the query
@@ -186,8 +185,9 @@ private static Weight[] createFilterForAggregations(
         int bucketCount = 0;
         while (roundedLow <= fieldType.convertNanosToMillis(high)) {
             bucketCount++;
-            if (bucketCount > MAX_NUM_FILTER_BUCKETS) {
-                logger.debug("Max number of filters reached [{}], skip the fast filter optimization", MAX_NUM_FILTER_BUCKETS);
+            int maxNumFilterBuckets = context.maxAggRewriteFilters();
+            if (bucketCount > maxNumFilterBuckets) {
+                logger.debug("Max number of filters reached [{}], skip the fast filter optimization", maxNumFilterBuckets);
                 return null;
             }
             // Below rounding is needed as the interval could return in
@@ -254,7 +254,7 @@ public void setAggregationType(AggregationType aggregationType) {
         }
 
         public boolean isRewriteable(final Object parent, final int subAggLength) {
-            if (!context.isFilterRewriteEnabled()) return false;
+            if (context.maxAggRewriteFilters() == 0) return false;
 
             boolean rewriteable = aggregationType.isRewriteable(parent, subAggLength);
             logger.debug("Fast filter rewriteable: {} for shard {}", rewriteable, context.indexShard().shardId());

diff --git a/server/src/main/java/org/opensearch/search/internal/SearchContext.java b/server/src/main/java/org/opensearch/search/internal/SearchContext.java
@@ -523,7 +523,7 @@ public String toString() {
 
     public abstract boolean shouldUseTimeSeriesDescSortOptimization();
 
-    public boolean isFilterRewriteEnabled() {
-        return false;
+    public int maxAggRewriteFilters() {
+        return 0;
     }
 }