From 22af708f72f60f39516860ac9a74b695a321e8f6 Mon Sep 17 00:00:00 2001 From: bowenlan-amzn Date: Mon, 15 Apr 2024 11:39:56 -0700 Subject: [PATCH] change the setting to be int Signed-off-by: bowenlan-amzn --- .../aggregations/bucket/FilterRewriteIT.java | 7 ++++--- .../common/settings/ClusterSettings.java | 2 +- .../opensearch/search/DefaultSearchContext.java | 16 ++++++++-------- .../org/opensearch/search/SearchService.java | 8 +++++--- .../bucket/FastFilterRewriteHelper.java | 8 ++++---- .../search/internal/SearchContext.java | 4 ++-- 6 files changed, 24 insertions(+), 21 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/search/aggregations/bucket/FilterRewriteIT.java b/server/src/internalClusterTest/java/org/opensearch/search/aggregations/bucket/FilterRewriteIT.java index 207804c19b6e3..3f32cf3bfbbe7 100644 --- a/server/src/internalClusterTest/java/org/opensearch/search/aggregations/bucket/FilterRewriteIT.java +++ b/server/src/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 index 65415e325569c..9934da9fcef4e 100644 --- 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 index be20b4dd4f20a..cd8714f6b556a 100644 --- 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 requestToAggReduceContextBuilder; private final boolean concurrentSearchSettingsEnabled; private final SetOnce 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 index 8387cc53c43ac..78b91bc2f3645 100644 --- 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 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 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 index d2c4812f5db4d..dde748bf0dc07 100644 --- 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, Function> 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 index 20320e40507bc..07f3616bbc138 100644 --- 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; } }