Skip to content

Commit

Permalink
Remove Adjacency_matrix setting in favour of Lucene Boolean query cla…
Browse files Browse the repository at this point in the history
…use setting (elastic#46327)

Closes elastic#46324
  • Loading branch information
markharwood committed Sep 19, 2019
1 parent 08e3ceb commit dc0abec
Show file tree
Hide file tree
Showing 8 changed files with 23 additions and 66 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -108,5 +108,7 @@ additional levels of data required to perform https://en.wikipedia.org/wiki/Dyna
where examining interactions _over time_ becomes important.

==== Limitations
For N filters the matrix of buckets produced can be N²/2 and so there is a default maximum
For N filters the matrix of buckets produced can be N²/2 which can be costly.
The circuit breaker settings prevent results producing too many buckets and to avoid excessive disk seeks
the `indices.query.bool.max_clause_count` setting is used to limit the number of filters.
imposed of 100 filters . This setting can be changed using the `index.max_adjacency_matrix_filters` index-level setting.
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,6 @@ public final class IndexScopedSettings extends AbstractScopedSettings {
IndexSettings.MAX_NGRAM_DIFF_SETTING,
IndexSettings.MAX_SHINGLE_DIFF_SETTING,
IndexSettings.MAX_RESCORE_WINDOW_SETTING,
IndexSettings.MAX_ADJACENCY_MATRIX_FILTERS_SETTING,
IndexSettings.MAX_ANALYZED_OFFSET_SETTING,
IndexSettings.MAX_TERMS_COUNT_SETTING,
IndexSettings.INDEX_TRANSLOG_SYNC_INTERVAL_SETTING,
Expand Down
21 changes: 0 additions & 21 deletions server/src/main/java/org/elasticsearch/index/IndexSettings.java
Original file line number Diff line number Diff line change
Expand Up @@ -173,13 +173,6 @@ public final class IndexSettings {
public static final Setting<Integer> MAX_RESCORE_WINDOW_SETTING =
Setting.intSetting("index.max_rescore_window", MAX_RESULT_WINDOW_SETTING, 1,
Property.Dynamic, Property.IndexScope);
/**
* Index setting describing the maximum number of filters clauses that can be used
* in an adjacency_matrix aggregation. The max number of buckets produced by
* N filters is (N*N)/2 so a limit of 100 filters is imposed by default.
*/
public static final Setting<Integer> MAX_ADJACENCY_MATRIX_FILTERS_SETTING =
Setting.intSetting("index.max_adjacency_matrix_filters", 100, 2, Property.Dynamic, Property.IndexScope);
public static final TimeValue DEFAULT_REFRESH_INTERVAL = new TimeValue(1, TimeUnit.SECONDS);
public static final Setting<TimeValue> INDEX_REFRESH_INTERVAL_SETTING =
Setting.timeSetting("index.refresh_interval", DEFAULT_REFRESH_INTERVAL, new TimeValue(-1, TimeUnit.MILLISECONDS),
Expand Down Expand Up @@ -373,7 +366,6 @@ private void setRetentionLeaseMillis(final TimeValue retentionLease) {
private volatile boolean warmerEnabled;
private volatile int maxResultWindow;
private volatile int maxInnerResultWindow;
private volatile int maxAdjacencyMatrixFilters;
private volatile int maxRescoreWindow;
private volatile int maxDocvalueFields;
private volatile int maxScriptFields;
Expand Down Expand Up @@ -488,7 +480,6 @@ public IndexSettings(final IndexMetaData indexMetaData, final Settings nodeSetti
warmerEnabled = scopedSettings.get(INDEX_WARMER_ENABLED_SETTING);
maxResultWindow = scopedSettings.get(MAX_RESULT_WINDOW_SETTING);
maxInnerResultWindow = scopedSettings.get(MAX_INNER_RESULT_WINDOW_SETTING);
maxAdjacencyMatrixFilters = scopedSettings.get(MAX_ADJACENCY_MATRIX_FILTERS_SETTING);
maxRescoreWindow = scopedSettings.get(MAX_RESCORE_WINDOW_SETTING);
maxDocvalueFields = scopedSettings.get(MAX_DOCVALUE_FIELDS_SEARCH_SETTING);
maxScriptFields = scopedSettings.get(MAX_SCRIPT_FIELDS_SETTING);
Expand Down Expand Up @@ -530,7 +521,6 @@ public IndexSettings(final IndexMetaData indexMetaData, final Settings nodeSetti
scopedSettings.addSettingsUpdateConsumer(INDEX_TRANSLOG_SYNC_INTERVAL_SETTING, this::setTranslogSyncInterval);
scopedSettings.addSettingsUpdateConsumer(MAX_RESULT_WINDOW_SETTING, this::setMaxResultWindow);
scopedSettings.addSettingsUpdateConsumer(MAX_INNER_RESULT_WINDOW_SETTING, this::setMaxInnerResultWindow);
scopedSettings.addSettingsUpdateConsumer(MAX_ADJACENCY_MATRIX_FILTERS_SETTING, this::setMaxAdjacencyMatrixFilters);
scopedSettings.addSettingsUpdateConsumer(MAX_RESCORE_WINDOW_SETTING, this::setMaxRescoreWindow);
scopedSettings.addSettingsUpdateConsumer(MAX_DOCVALUE_FIELDS_SEARCH_SETTING, this::setMaxDocvalueFields);
scopedSettings.addSettingsUpdateConsumer(MAX_SCRIPT_FIELDS_SETTING, this::setMaxScriptFields);
Expand Down Expand Up @@ -821,17 +811,6 @@ private void setMaxInnerResultWindow(int maxInnerResultWindow) {
this.maxInnerResultWindow = maxInnerResultWindow;
}

/**
* Returns the max number of filters in adjacency_matrix aggregation search requests
*/
public int getMaxAdjacencyMatrixFilters() {
return this.maxAdjacencyMatrixFilters;
}

private void setMaxAdjacencyMatrixFilters(int maxAdjacencyFilters) {
this.maxAdjacencyMatrixFilters = maxAdjacencyFilters;
}

/**
* Returns the maximum rescore window for search requests.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,17 @@

package org.elasticsearch.search.aggregations.bucket.adjacency;

import org.apache.lucene.search.BooleanQuery;
import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.xcontent.ObjectParser;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.index.IndexSettings;
import org.elasticsearch.index.query.QueryBuilder;
import org.elasticsearch.index.query.QueryShardContext;
import org.elasticsearch.index.query.Rewriteable;
import org.elasticsearch.search.SearchModule;
import org.elasticsearch.search.aggregations.AbstractAggregationBuilder;
import org.elasticsearch.search.aggregations.AggregationBuilder;
import org.elasticsearch.search.aggregations.AggregatorFactories.Builder;
Expand Down Expand Up @@ -198,13 +199,13 @@ public Map<String, QueryBuilder> filters() {
@Override
protected AggregatorFactory doBuild(QueryShardContext queryShardContext, AggregatorFactory parent, Builder subFactoriesBuilder)
throws IOException {
int maxFilters = queryShardContext.getIndexSettings().getMaxAdjacencyMatrixFilters();
int maxFilters = BooleanQuery.getMaxClauseCount();
if (filters.size() > maxFilters){
throw new IllegalArgumentException(
"Number of filters is too large, must be less than or equal to: [" + maxFilters + "] but was ["
+ filters.size() + "]."
+ "This limit can be set by changing the [" + IndexSettings.MAX_ADJACENCY_MATRIX_FILTERS_SETTING.getKey()
+ "] index level setting.");
+ "This limit can be set by changing the [" + SearchModule.INDICES_MAX_CLAUSE_COUNT_SETTING.getKey()
+ "] setting.");
}

List<KeyedFilter> rewrittenFilters = new ArrayList<>(filters.size());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -344,29 +344,6 @@ public void testMaxScriptFields() {
assertEquals(IndexSettings.MAX_SCRIPT_FIELDS_SETTING.get(Settings.EMPTY).intValue(), settings.getMaxScriptFields());
}

public void testMaxAdjacencyMatrixFiltersSetting() {
IndexMetaData metaData = newIndexMeta("index", Settings.builder()
.put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT)
.put(IndexSettings.MAX_ADJACENCY_MATRIX_FILTERS_SETTING.getKey(), 15)
.build());
IndexSettings settings = new IndexSettings(metaData, Settings.EMPTY);
assertEquals(15, settings.getMaxAdjacencyMatrixFilters());
settings.updateIndexMetaData(newIndexMeta("index",
Settings.builder().put(IndexSettings.MAX_ADJACENCY_MATRIX_FILTERS_SETTING.getKey(),
42).build()));
assertEquals(42, settings.getMaxAdjacencyMatrixFilters());
settings.updateIndexMetaData(newIndexMeta("index", Settings.EMPTY));
assertEquals(IndexSettings.MAX_ADJACENCY_MATRIX_FILTERS_SETTING.get(Settings.EMPTY).intValue(),
settings.getMaxAdjacencyMatrixFilters());

metaData = newIndexMeta("index", Settings.builder()
.put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT)
.build());
settings = new IndexSettings(metaData, Settings.EMPTY);
assertEquals(IndexSettings.MAX_ADJACENCY_MATRIX_FILTERS_SETTING.get(Settings.EMPTY).intValue(),
settings.getMaxAdjacencyMatrixFilters());
}

public void testMaxRegexLengthSetting() {
IndexMetaData metaData = newIndexMeta("index", Settings.builder()
.put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.index.IndexSettings;
import org.elasticsearch.index.query.BoolQueryBuilder;
import org.elasticsearch.index.query.QueryBuilder;
import org.elasticsearch.search.SearchModule;
import org.elasticsearch.search.aggregations.InternalAggregation;
import org.elasticsearch.search.aggregations.bucket.adjacency.AdjacencyMatrix;
import org.elasticsearch.search.aggregations.bucket.adjacency.AdjacencyMatrix.Bucket;
Expand All @@ -46,7 +46,6 @@
import static org.elasticsearch.search.aggregations.AggregationBuilders.adjacencyMatrix;
import static org.elasticsearch.search.aggregations.AggregationBuilders.avg;
import static org.elasticsearch.search.aggregations.AggregationBuilders.histogram;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
Expand All @@ -58,16 +57,11 @@
public class AdjacencyMatrixIT extends ESIntegTestCase {

static int numDocs, numSingleTag1Docs, numSingleTag2Docs, numTag1Docs, numTag2Docs, numMultiTagDocs;
static final int MAX_NUM_FILTERS = 3;

@Override
public void setupSuiteScopeCluster() throws Exception {
createIndex("idx");
createIndex("idx2");
assertAcked(client().admin().indices().prepareUpdateSettings("idx")
.setSettings(
Settings.builder().put(IndexSettings.MAX_ADJACENCY_MATRIX_FILTERS_SETTING.getKey(), MAX_NUM_FILTERS))
.get());

numDocs = randomIntBetween(5, 20);
numTag1Docs = randomIntBetween(1, numDocs - 1);
Expand Down Expand Up @@ -300,9 +294,10 @@ public void testWithSubAggregation() throws Exception {

public void testTooLargeMatrix() throws Exception{

// Create more filters than is permitted by index settings.
// Create more filters than is permitted by Lucene Bool clause settings.
MapBuilder filtersMap = new MapBuilder();
for (int i = 0; i <= MAX_NUM_FILTERS; i++) {
int maxFilters = SearchModule.INDICES_MAX_CLAUSE_COUNT_SETTING.get(Settings.EMPTY);
for (int i = 0; i <= maxFilters; i++) {
filtersMap.add("tag" + i, termQuery("tag", "tag" + i));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.elasticsearch.index.query.QueryBuilder;
import org.elasticsearch.index.query.QueryShardContext;
import org.elasticsearch.index.shard.IndexShard;
import org.elasticsearch.search.SearchModule;
import org.elasticsearch.search.aggregations.AggregatorFactories;
import org.elasticsearch.search.aggregations.AggregatorFactory;
import org.elasticsearch.search.internal.SearchContext;
Expand All @@ -48,7 +49,6 @@ public void testFilterSizeLimitation() throws Exception {
QueryShardContext queryShardContext = mock(QueryShardContext.class);
IndexShard indexShard = mock(IndexShard.class);
Settings settings = Settings.builder()
.put("index.max_adjacency_matrix_filters", 2)
.put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT)
.put(IndexMetaData.SETTING_NUMBER_OF_REPLICAS, 1)
.put(IndexMetaData.SETTING_NUMBER_OF_SHARDS, 2)
Expand All @@ -58,9 +58,13 @@ public void testFilterSizeLimitation() throws Exception {
when(indexShard.indexSettings()).thenReturn(indexSettings);
when(queryShardContext.getIndexSettings()).thenReturn(indexSettings);
SearchContext context = new TestSearchContext(queryShardContext, indexShard);

int maxFilters = SearchModule.INDICES_MAX_CLAUSE_COUNT_SETTING.get(context.indexShard().indexSettings().getSettings());
int maxFiltersPlusOne = maxFilters + 1;


Map<String, QueryBuilder> filters = new HashMap<>(3);
for (int i = 0; i < 3; i++) {
Map<String, QueryBuilder> filters = new HashMap<>(maxFilters);
for (int i = 0; i < maxFiltersPlusOne; i++) {
QueryBuilder queryBuilder = mock(QueryBuilder.class);
// return builder itself to skip rewrite
when(queryBuilder.rewrite(queryShardContext)).thenReturn(queryBuilder);
Expand All @@ -69,9 +73,10 @@ public void testFilterSizeLimitation() throws Exception {
AdjacencyMatrixAggregationBuilder builder = new AdjacencyMatrixAggregationBuilder("dummy", filters);
IllegalArgumentException ex = expectThrows(IllegalArgumentException.class,
() -> builder.doBuild(context.getQueryShardContext(), null, new AggregatorFactories.Builder()));
assertThat(ex.getMessage(), equalTo("Number of filters is too large, must be less than or equal to: [2] but was [3]."
+ "This limit can be set by changing the [" + IndexSettings.MAX_ADJACENCY_MATRIX_FILTERS_SETTING.getKey()
+ "] index level setting."));
assertThat(ex.getMessage(), equalTo("Number of filters is too large, must be less than or equal to: ["+ maxFilters
+"] but was ["+ maxFiltersPlusOne +"]."
+ "This limit can be set by changing the [" + SearchModule.INDICES_MAX_CLAUSE_COUNT_SETTING.getKey()
+ "] setting."));

// filter size not grater than max size should return an instance of AdjacencyMatrixAggregatorFactory
Map<String, QueryBuilder> emptyFilters = Collections.emptyMap();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,6 @@ static String[] extractLeaderShardHistoryUUIDs(Map<String, String> ccrIndexMetaD
IndexSettings.MAX_DOCVALUE_FIELDS_SEARCH_SETTING,
IndexSettings.MAX_TOKEN_COUNT_SETTING,
IndexSettings.MAX_SLICES_PER_SCROLL,
IndexSettings.MAX_ADJACENCY_MATRIX_FILTERS_SETTING,
IndexSettings.DEFAULT_PIPELINE,
IndexSettings.INDEX_SEARCH_THROTTLED,
IndexSettings.INDEX_TRANSLOG_RETENTION_AGE_SETTING,
Expand Down

0 comments on commit dc0abec

Please sign in to comment.