Skip to content

Commit

Permalink
change the setting to be int
Browse files Browse the repository at this point in the history
Signed-off-by: bowenlan-amzn <[email protected]>
  • Loading branch information
bowenlan-amzn committed Apr 15, 2024
1 parent b807d18 commit 22af708
Show file tree
Hide file tree
Showing 6 changed files with 24 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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)
Expand All @@ -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();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
)
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -243,7 +243,7 @@ final class DefaultSearchContext extends SearchContext {
this.lowLevelCancellation = lowLevelCancellation;
this.requestToAggReduceContextBuilder = requestToAggReduceContextBuilder;

this.filterRewriteEnabled = evaluateFilterRewriteSetting();
this.maxAggRewriteFilters = evaluateFilterRewriteSetting();
}

@Override
Expand Down Expand Up @@ -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;
}
}
8 changes: 5 additions & 3 deletions server/src/main/java/org/opensearch/search/SearchService.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -523,7 +523,7 @@ public String toString() {

public abstract boolean shouldUseTimeSeriesDescSortOptimization();

public boolean isFilterRewriteEnabled() {
return false;
public int maxAggRewriteFilters() {
return 0;
}
}

0 comments on commit 22af708

Please sign in to comment.