From 6da22828da01b5dfdc07beb9e912cf5db927018e Mon Sep 17 00:00:00 2001 From: Marco Miller Date: Fri, 29 Jan 2016 12:31:38 -0500 Subject: [PATCH] Remove index defaultMaxClauseCount config setting while reusing maxTerms When browsing Related Changes that are ready to be submitted, the Submit buttons used to disappear from the UI. That was caused by TooManyClauses exceptions being thrown while trying to query the secondary index. And, that exception was caused by either an index.defaultMaxClauseCount value that was too low or just a naturally deep history of related changes. The likelihood of this case increased when index.maxTerms was either not set (thus no limit) or set to a value higher than defaultMaxClauseCount. User experience through the UI should not have suffered from such missing -or challenging- index config tuning. Fix this issue by using index.maxTerms at all levels, leading to proper fallback on the database if that query size limit is reached. Also, change maxTerms default to 1024 (from no-limit), to match that late defaultMaxClauseCount default of 1024. This is so that the original default at index level (former defaultMaxClauseCount) is preserved, to keep default index querying efficient. Update the related Documentation accordingly. Test: - push at least a few Related Changes - vote on them so they become Ready to Submit - before this fix: set index.defaultMaxClauseCount to e.g. 3, to provoke - index.maxTerms needs to be unset => default: no limit before, 1024 now - browse those Related Changes (click on each) - after this fix: Submit buttons should all still show - after this fix: Change tab headers should all look fine. Bug: Issue 3771 Change-Id: If2222d14d7b0345327c9bf9f28a7c1038d9b5a8b --- Documentation/config-gerrit.txt | 19 +++++++------------ .../gerrit/lucene/LuceneChangeIndex.java | 2 +- .../gerrit/server/index/IndexConfig.java | 4 ++-- 3 files changed, 10 insertions(+), 15 deletions(-) diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt index 61270364a7..ec2945efe6 100644 --- a/Documentation/config-gerrit.txt +++ b/Documentation/config-gerrit.txt @@ -2382,10 +2382,14 @@ Defaults to no limit. + Maximum number of leaf terms to allow in a query. Too-large queries may perform poorly, so setting this option causes query parsing to fail fast -before attempting to send them to the secondary index. Set to 0 for no -limit. +before attempting to send them to the secondary index. Should this limit +be reached, database is used instead of index as applicable. + -Defaults to no limit. +When the index type is `LUCENE`, also sets the maximum number of clauses +permitted per BooleanQuery. This is so that all enforced query limits +are the same. ++ +Defaults to 1024. ==== Lucene configuration @@ -2394,14 +2398,6 @@ Open and closed changes are indexed in separate indexes named The following settings are only used when the index type is `LUCENE`. -[[index.defaultMaxClauseCount]]index.defaultMaxClauseCount:: -+ -Only used when the type is `LUCENE`. -+ -Sets the maximum number of clauses permitted per BooleanQuery. -+ -Defaults to 1024. - [[index.name.ramBufferSize]]index.name.ramBufferSize:: + Determines the amount of RAM that may be used for buffering added documents @@ -2445,7 +2441,6 @@ Sample Lucene index configuration: ---- [index] type = LUCENE - defaultMaxClauseCount = 2048 [index "changes_open"] ramBufferSize = 60 m diff --git a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java index 5c3b6294f9..d703bed0cd 100644 --- a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java +++ b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java @@ -248,7 +248,7 @@ public class LuceneChangeIndex implements ChangeIndex { CUSTOM_CHAR_MAPPING); queryBuilder = new QueryBuilder(analyzer); - BooleanQuery.setMaxClauseCount(cfg.getInt("index", "defaultMaxClauseCount", + BooleanQuery.setMaxClauseCount(cfg.getInt("index", "maxTerms", BooleanQuery.getMaxClauseCount())); GerritIndexWriterConfig openConfig = diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/IndexConfig.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/IndexConfig.java index 00aa081180..125d32d9c5 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/index/IndexConfig.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/IndexConfig.java @@ -29,7 +29,7 @@ import org.eclipse.jgit.lib.Config; */ @AutoValue public abstract class IndexConfig { - private static final int DEFAULT_MAX_TERMS = 500; + private static final int DEFAULT_MAX_TERMS = 1024; private static final int DEFAULT_MAX_PREFIX_TERMS = 100; public static IndexConfig createDefault() { @@ -49,7 +49,7 @@ public abstract class IndexConfig { return new AutoValue_IndexConfig( checkLimit(maxLimit, "maxLimit", Integer.MAX_VALUE), checkLimit(maxPages, "maxPages", Integer.MAX_VALUE), - checkLimit(maxTerms, "maxTerms", Integer.MAX_VALUE), + checkLimit(maxTerms, "maxTerms", DEFAULT_MAX_TERMS), checkLimit(maxPrefixTerms, "maxPrefixTerms", DEFAULT_MAX_PREFIX_TERMS)); }