From d03143571d2ef104a0c306e0e1c1831e7adb7958 Mon Sep 17 00:00:00 2001 From: Patrick Hiesel Date: Wed, 13 Nov 2019 08:08:19 -0800 Subject: [PATCH] Replace reindexAfterRefUpdate setting with indexMergeable Gerrit can index a boolean field called 'mergeable' that determines if a change can be merged into the target ref. This bit can change whenever the target ref advances. Gerrit therefore has logic to reindex all open changes when the target ref advances. Depending on the number of open changes, the frequency of updates of the target ref and the size of the repo, this can be a very expensive operation. For large installations, 'reindexAfterRefUpdate' was added as a setting back in 2015 (I88ae7f4ad) to turn off automatic reindexing. This setting however, leads to inconsistent behavior: Gerrit stops updating documents when the target ref advances, so the 'mergeable' bit in the indexed document can be stale. For large repos, it is most likely stale. Users can still query for 'is:mergeable' though and Gerrit happily serves that stale bit in any query response. It is worth noting, that all of this does not affect the UI as that sends a separate, asynchronous request to compute mergeablitly when needed and does not rely on the index. This commit cleans this behavior up by replacing reindexAfterRefUpdate with indexMergeable. After this commit, there are two modes of operation: 1) Gerrit indexes 'mergable' and keeps it up to date when the target ref advances. Gerrit allows queries for 'is:mergeable'. 2) Gerrit does not index 'mergeable' at all. Gerrit does not allow queries for 'is:mergeable'. This way, users always get a consistent and correct result. Change-Id: I053af1b99616920db7f0dda8f8ec770e8683df5c --- Documentation/config-gerrit.txt | 29 +++++++------- Documentation/rest-api-changes.txt | 5 +++ .../gerrit/acceptance/GerritServer.java | 4 +- .../elasticsearch/ElasticAccountIndex.java | 3 +- .../elasticsearch/ElasticChangeIndex.java | 13 ++++++- .../elasticsearch/ElasticGroupIndex.java | 3 +- .../elasticsearch/ElasticProjectIndex.java | 3 +- .../elasticsearch/bulk/UpdateRequest.java | 7 +++- java/com/google/gerrit/index/Schema.java | 17 ++++++--- .../gerrit/lucene/AbstractLuceneIndex.java | 6 ++- .../google/gerrit/lucene/ChangeSubIndex.java | 6 ++- .../gerrit/lucene/LuceneAccountIndex.java | 2 + .../gerrit/lucene/LuceneChangeIndex.java | 38 ++++++++++++++++--- .../gerrit/lucene/LuceneGroupIndex.java | 2 + .../gerrit/lucene/LuceneProjectIndex.java | 2 + .../index/change/ReindexAfterRefUpdate.java | 2 +- .../query/change/ChangeQueryBuilder.java | 19 ++++++++-- .../google/gerrit/testing/IndexConfig.java | 4 +- .../acceptance/api/change/ChangeIT.java | 14 +++++++ .../acceptance/api/revision/RevisionIT.java | 2 + .../rest/change/AbstractSubmit.java | 2 +- .../acceptance/rest/change/IndexChangeIT.java | 2 + .../server/index/change/FakeQueryBuilder.java | 31 ++++++++++++++- .../change/AbstractQueryChangesTest.java | 20 +++++++++- .../google/gerrit/server/query/change/BUILD | 2 + 25 files changed, 194 insertions(+), 44 deletions(-) diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt index 452ef23210..a9d922d6e4 100644 --- a/Documentation/config-gerrit.txt +++ b/Documentation/config-gerrit.txt @@ -2766,6 +2766,22 @@ online schema upgrades. If not set or set to a zero, defaults to the number of logical CPUs as returned by the JVM. If set to a negative value, defaults to a direct executor. +[[index.change.indexMergeable]]index.change.indexMergeable:: ++ +Specifies if `mergeable` should be index or not. Indexing this field enables +queries that contain the mergeability operator (`is:mergeable`). If enabled, +Gerrit will check if the change is mergeable into the target branch when +reindexing a change. This is an expensive operation. ++ +If true, Gerrit will reindex all open changes when the target ref advances. +Depending on the frequency of updates to the ref and the number of open changes, +this can be very expensive. ++ +When this setting is changed from `false` to `true`, all changes need to be +reindexed. ++ +Defaults to true. + [[index.onlineUpgrade]]index.onlineUpgrade:: + Whether to upgrade to new index schema versions while the server is @@ -2813,19 +2829,6 @@ are the same. + Defaults to 1024. -[[index.reindexAfterRefUpdate]]index.reindexAfterRefUpdate:: -+ -Whether to reindex all affected open changes after a ref is updated. This -includes reindexing all open changes to recompute the "mergeable" bit every time -the destination branch moves, as well as reindexing changes to take into account -new project configuration (e.g. label definitions). -+ -Leaving this enabled may result in fresher results, but may cause performance -problems if there are lots of open changes on a project whose branches advance -frequently. -+ -Defaults to true. - [[index.autoReindexIfStale]]index.autoReindexIfStale:: + Whether to automatically check if a document became stale in the index diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt index 69a7641ace..0fc733abfa 100644 --- a/Documentation/rest-api-changes.txt +++ b/Documentation/rest-api-changes.txt @@ -315,6 +315,11 @@ When link:config-gerrit.html#change.api.excludeMergeableInChangeInfo[ the `mergeable` field will always be omitted and `SKIP_MERGEABLE` has no effect. + +When link:config-gerrit.html#index.change.indexMergeable[ +`index.change.indexMergeable`] is set to `false` in the `gerrit.config`, +the `mergeable` field will always be omitted when querying changes and +`SKIP_MERGEABLE` has no effect. ++ A change's mergeability can be requested separately by calling the link:#get-mergeable[get-mergeable] endpoint. -- diff --git a/java/com/google/gerrit/acceptance/GerritServer.java b/java/com/google/gerrit/acceptance/GerritServer.java index 73d37be3a4..e03e08497f 100644 --- a/java/com/google/gerrit/acceptance/GerritServer.java +++ b/java/com/google/gerrit/acceptance/GerritServer.java @@ -563,8 +563,8 @@ public class GerritServer implements AutoCloseable { cfg.setInt("sshd", null, "commandStartThreads", 1); cfg.setInt("receive", null, "threadPoolSize", 1); cfg.setInt("index", null, "threads", 1); - if (cfg.getString("index", null, "reindexAfterRefUpdate") == null) { - cfg.setBoolean("index", null, "reindexAfterRefUpdate", false); + if (cfg.getString("index", "change", "indexMergeable") == null) { + cfg.setBoolean("index", "change", "indexMergeable", false); } } diff --git a/java/com/google/gerrit/elasticsearch/ElasticAccountIndex.java b/java/com/google/gerrit/elasticsearch/ElasticAccountIndex.java index a06f90f70b..c3e3264e4c 100644 --- a/java/com/google/gerrit/elasticsearch/ElasticAccountIndex.java +++ b/java/com/google/gerrit/elasticsearch/ElasticAccountIndex.java @@ -14,6 +14,7 @@ package com.google.gerrit.elasticsearch; +import com.google.common.collect.ImmutableSet; import com.google.gerrit.elasticsearch.ElasticMapping.MappingProperties; import com.google.gerrit.elasticsearch.bulk.BulkRequest; import com.google.gerrit.elasticsearch.bulk.IndexRequest; @@ -74,7 +75,7 @@ public class ElasticAccountIndex extends AbstractElasticIndex(schema, as)); + .add(new UpdateRequest<>(schema, as, ImmutableSet.of())); String uri = getURI(type, BULK); Response response = postRequest(uri, bulk, getRefreshParam()); diff --git a/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java b/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java index 37184cc2e7..084c2ecb95 100644 --- a/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java +++ b/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java @@ -22,6 +22,7 @@ import static java.util.Objects.requireNonNull; import com.google.common.collect.FluentIterable; import com.google.common.collect.ImmutableListMultimap; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.collect.ListMultimap; import com.google.common.collect.Lists; @@ -48,6 +49,7 @@ import com.google.gerrit.index.query.QueryParseException; import com.google.gerrit.server.ReviewerByEmailSet; import com.google.gerrit.server.ReviewerSet; import com.google.gerrit.server.StarredChangesUtil; +import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.config.SitePaths; import com.google.gerrit.server.index.IndexUtils; import com.google.gerrit.server.index.change.ChangeField; @@ -65,6 +67,7 @@ import java.util.List; import java.util.Optional; import java.util.Set; import org.apache.http.HttpStatus; +import org.eclipse.jgit.lib.Config; import org.elasticsearch.client.Response; /** Secondary index implementation using Elasticsearch. */ @@ -91,6 +94,7 @@ class ElasticChangeIndex extends AbstractElasticIndex private final ChangeData.Factory changeDataFactory; private final Schema schema; private final FieldDef idField; + private final ImmutableSet skipFields; @Inject ElasticChangeIndex( @@ -98,6 +102,7 @@ class ElasticChangeIndex extends AbstractElasticIndex ChangeData.Factory changeDataFactory, SitePaths sitePaths, ElasticRestClientProvider clientBuilder, + @GerritServerConfig Config gerritConfig, @Assisted Schema schema) { super(cfg, sitePaths, schema, clientBuilder, CHANGES); this.changeDataFactory = changeDataFactory; @@ -105,6 +110,10 @@ class ElasticChangeIndex extends AbstractElasticIndex this.mapping = new ChangeMapping(schema, client.adapter()); this.idField = this.schema.useLegacyNumericFields() ? ChangeField.LEGACY_ID : ChangeField.LEGACY_ID_STR; + this.skipFields = + gerritConfig.getBoolean("index", "change", "indexMergeable", true) + ? ImmutableSet.of() + : ImmutableSet.of(ChangeField.MERGEABLE.getName()); } @Override @@ -123,7 +132,7 @@ class ElasticChangeIndex extends AbstractElasticIndex ElasticQueryAdapter adapter = client.adapter(); BulkRequest bulk = new IndexRequest(getId(cd), indexName, adapter.getType(insertIndex), adapter) - .add(new UpdateRequest<>(schema, cd)); + .add(new UpdateRequest<>(schema, cd, skipFields)); if (adapter.deleteToReplace()) { bulk.add(new DeleteRequest(cd.getId().toString(), indexName, deleteIndex, adapter)); } @@ -263,7 +272,7 @@ class ElasticChangeIndex extends AbstractElasticIndex // Mergeable. JsonElement mergeableElement = source.get(ChangeField.MERGEABLE.getName()); - if (mergeableElement != null) { + if (mergeableElement != null && !skipFields.contains(ChangeField.MERGEABLE.getName())) { String mergeable = mergeableElement.getAsString(); if ("1".equals(mergeable)) { cd.setMergeable(true); diff --git a/java/com/google/gerrit/elasticsearch/ElasticGroupIndex.java b/java/com/google/gerrit/elasticsearch/ElasticGroupIndex.java index c215132feb..ce2025f3db 100644 --- a/java/com/google/gerrit/elasticsearch/ElasticGroupIndex.java +++ b/java/com/google/gerrit/elasticsearch/ElasticGroupIndex.java @@ -14,6 +14,7 @@ package com.google.gerrit.elasticsearch; +import com.google.common.collect.ImmutableSet; import com.google.gerrit.elasticsearch.ElasticMapping.MappingProperties; import com.google.gerrit.elasticsearch.bulk.BulkRequest; import com.google.gerrit.elasticsearch.bulk.IndexRequest; @@ -74,7 +75,7 @@ public class ElasticGroupIndex extends AbstractElasticIndex(schema, group)); + .add(new UpdateRequest<>(schema, group, ImmutableSet.of())); String uri = getURI(type, BULK); Response response = postRequest(uri, bulk, getRefreshParam()); diff --git a/java/com/google/gerrit/elasticsearch/ElasticProjectIndex.java b/java/com/google/gerrit/elasticsearch/ElasticProjectIndex.java index 29f85079e5..b636706d49 100644 --- a/java/com/google/gerrit/elasticsearch/ElasticProjectIndex.java +++ b/java/com/google/gerrit/elasticsearch/ElasticProjectIndex.java @@ -14,6 +14,7 @@ package com.google.gerrit.elasticsearch; +import com.google.common.collect.ImmutableSet; import com.google.gerrit.elasticsearch.ElasticMapping.MappingProperties; import com.google.gerrit.elasticsearch.bulk.BulkRequest; import com.google.gerrit.elasticsearch.bulk.IndexRequest; @@ -74,7 +75,7 @@ public class ElasticProjectIndex extends AbstractElasticIndex(schema, projectState)); + .add(new UpdateRequest<>(schema, projectState, ImmutableSet.of())); String uri = getURI(type, BULK); Response response = postRequest(uri, bulk, getRefreshParam()); diff --git a/java/com/google/gerrit/elasticsearch/bulk/UpdateRequest.java b/java/com/google/gerrit/elasticsearch/bulk/UpdateRequest.java index 2f0bd0118e..196b8d6ad3 100644 --- a/java/com/google/gerrit/elasticsearch/bulk/UpdateRequest.java +++ b/java/com/google/gerrit/elasticsearch/bulk/UpdateRequest.java @@ -16,6 +16,7 @@ package com.google.gerrit.elasticsearch.bulk; import static java.util.stream.Collectors.toList; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.collect.Streams; import com.google.gerrit.elasticsearch.builders.XContentBuilder; @@ -27,17 +28,19 @@ public class UpdateRequest extends BulkRequest { private final Schema schema; private final V v; + private final ImmutableSet skipFields; - public UpdateRequest(Schema schema, V v) { + public UpdateRequest(Schema schema, V v, ImmutableSet skipFields) { this.schema = schema; this.v = v; + this.skipFields = skipFields; } @Override protected String getRequest() { try (XContentBuilder closeable = new XContentBuilder()) { XContentBuilder builder = closeable.startObject(); - for (Values values : schema.buildFields(v)) { + for (Values values : schema.buildFields(v, skipFields)) { String name = values.getField().getName(); if (values.getField().isRepeatable()) { builder.field(name, Streams.stream(values.getValues()).collect(toList())); diff --git a/java/com/google/gerrit/index/Schema.java b/java/com/google/gerrit/index/Schema.java index f9f8c487d2..0aa374b0e0 100644 --- a/java/com/google/gerrit/index/Schema.java +++ b/java/com/google/gerrit/index/Schema.java @@ -15,11 +15,12 @@ package com.google.gerrit.index; import static com.google.common.base.Preconditions.checkState; +import static com.google.common.collect.ImmutableList.toImmutableList; import com.google.common.base.MoreObjects; -import com.google.common.collect.FluentIterable; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.google.common.flogger.FluentLogger; import java.util.ArrayList; import java.util.Arrays; @@ -181,12 +182,17 @@ public class Schema { *

Null values are omitted, as are fields which cause errors, which are logged. * * @param obj input object. + * @param skipFields set of field names to skip when indexing the document * @return all non-null field values from the object. */ - public final Iterable> buildFields(T obj) { - return FluentIterable.from(fields.values()) - .transform( + public final Iterable> buildFields(T obj, ImmutableSet skipFields) { + return fields.values().stream() + .map( f -> { + if (skipFields.contains(f.getName())) { + return null; + } + Object v; try { v = f.get(obj); @@ -203,7 +209,8 @@ public class Schema { return new Values<>(f, Collections.singleton(v)); } }) - .filter(Objects::nonNull); + .filter(Objects::nonNull) + .collect(toImmutableList()); } @Override diff --git a/java/com/google/gerrit/lucene/AbstractLuceneIndex.java b/java/com/google/gerrit/lucene/AbstractLuceneIndex.java index deb3203038..5392ab464a 100644 --- a/java/com/google/gerrit/lucene/AbstractLuceneIndex.java +++ b/java/com/google/gerrit/lucene/AbstractLuceneIndex.java @@ -21,6 +21,7 @@ import static java.util.concurrent.TimeUnit.MILLISECONDS; import com.google.common.base.Joiner; import com.google.common.collect.ArrayListMultimap; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.ListMultimap; import com.google.common.collect.Sets; import com.google.common.flogger.FluentLogger; @@ -98,6 +99,7 @@ public abstract class AbstractLuceneIndex implements Index { private final SitePaths sitePaths; private final Directory dir; private final String name; + private final ImmutableSet skipFields; private final ListeningExecutorService writerThread; private final IndexWriter writer; private final ReferenceManager searcherManager; @@ -110,6 +112,7 @@ public abstract class AbstractLuceneIndex implements Index { SitePaths sitePaths, Directory dir, String name, + ImmutableSet skipFields, String subIndex, GerritIndexWriterConfig writerConfig, SearcherFactory searcherFactory) @@ -118,6 +121,7 @@ public abstract class AbstractLuceneIndex implements Index { this.sitePaths = sitePaths; this.dir = dir; this.name = name; + this.skipFields = skipFields; String index = Joiner.on('_').skipNulls().join(name, subIndex); long commitPeriod = writerConfig.getCommitWithinMs(); @@ -311,7 +315,7 @@ public abstract class AbstractLuceneIndex implements Index { Document toDocument(V obj) { Document result = new Document(); - for (Values vs : schema.buildFields(obj)) { + for (Values vs : schema.buildFields(obj, skipFields)) { if (vs.getValues() != null) { add(result, vs); } diff --git a/java/com/google/gerrit/lucene/ChangeSubIndex.java b/java/com/google/gerrit/lucene/ChangeSubIndex.java index fd439f199f..e51a91a7e6 100644 --- a/java/com/google/gerrit/lucene/ChangeSubIndex.java +++ b/java/com/google/gerrit/lucene/ChangeSubIndex.java @@ -20,6 +20,7 @@ import static com.google.gerrit.lucene.LuceneChangeIndex.ID_SORT_FIELD; import static com.google.gerrit.lucene.LuceneChangeIndex.UPDATED_SORT_FIELD; import static com.google.gerrit.server.index.change.ChangeSchemaDefinitions.NAME; +import com.google.common.collect.ImmutableSet; import com.google.gerrit.entities.Change; import com.google.gerrit.index.FieldDef; import com.google.gerrit.index.QueryOptions; @@ -48,6 +49,7 @@ public class ChangeSubIndex extends AbstractLuceneIndex Schema schema, SitePaths sitePaths, Path path, + ImmutableSet skipFields, GerritIndexWriterConfig writerConfig, SearcherFactory searcherFactory) throws IOException { @@ -56,6 +58,7 @@ public class ChangeSubIndex extends AbstractLuceneIndex sitePaths, FSDirectory.open(path), path.getFileName().toString(), + skipFields, writerConfig, searcherFactory); } @@ -65,10 +68,11 @@ public class ChangeSubIndex extends AbstractLuceneIndex SitePaths sitePaths, Directory dir, String subIndex, + ImmutableSet skipFields, GerritIndexWriterConfig writerConfig, SearcherFactory searcherFactory) throws IOException { - super(schema, sitePaths, dir, NAME, subIndex, writerConfig, searcherFactory); + super(schema, sitePaths, dir, NAME, skipFields, subIndex, writerConfig, searcherFactory); } @Override diff --git a/java/com/google/gerrit/lucene/LuceneAccountIndex.java b/java/com/google/gerrit/lucene/LuceneAccountIndex.java index efd7ea363d..242cffdbb0 100644 --- a/java/com/google/gerrit/lucene/LuceneAccountIndex.java +++ b/java/com/google/gerrit/lucene/LuceneAccountIndex.java @@ -20,6 +20,7 @@ import static com.google.gerrit.server.index.account.AccountField.ID; import static com.google.gerrit.server.index.account.AccountField.ID_STR; import static com.google.gerrit.server.index.account.AccountField.PREFERRED_EMAIL_EXACT; +import com.google.common.collect.ImmutableSet; import com.google.gerrit.entities.Account; import com.google.gerrit.exceptions.StorageException; import com.google.gerrit.index.FieldDef; @@ -100,6 +101,7 @@ public class LuceneAccountIndex extends AbstractLuceneIndex skipFields; @Inject LuceneChangeIndex( @@ -179,6 +181,10 @@ public class LuceneChangeIndex implements ChangeIndex { this.executor = executor; this.changeDataFactory = changeDataFactory; this.schema = schema; + this.skipFields = + cfg.getBoolean("index", "change", "indexMergeable", true) + ? ImmutableSet.of() + : ImmutableSet.of(ChangeField.MERGEABLE.getName()); GerritIndexWriterConfig openConfig = new GerritIndexWriterConfig(cfg, "changes_open"); GerritIndexWriterConfig closedConfig = new GerritIndexWriterConfig(cfg, "changes_closed"); @@ -189,18 +195,40 @@ public class LuceneChangeIndex implements ChangeIndex { if (LuceneIndexModule.isInMemoryTest(cfg)) { openIndex = new ChangeSubIndex( - schema, sitePaths, new RAMDirectory(), "ramOpen", openConfig, searcherFactory); + schema, + sitePaths, + new RAMDirectory(), + "ramOpen", + skipFields, + openConfig, + searcherFactory); closedIndex = new ChangeSubIndex( - schema, sitePaths, new RAMDirectory(), "ramClosed", closedConfig, searcherFactory); + schema, + sitePaths, + new RAMDirectory(), + "ramClosed", + skipFields, + closedConfig, + searcherFactory); } else { Path dir = LuceneVersionManager.getDir(sitePaths, CHANGES, schema); openIndex = new ChangeSubIndex( - schema, sitePaths, dir.resolve(CHANGES_OPEN), openConfig, searcherFactory); + schema, + sitePaths, + dir.resolve(CHANGES_OPEN), + skipFields, + openConfig, + searcherFactory); closedIndex = new ChangeSubIndex( - schema, sitePaths, dir.resolve(CHANGES_CLOSED), closedConfig, searcherFactory); + schema, + sitePaths, + dir.resolve(CHANGES_CLOSED), + skipFields, + closedConfig, + searcherFactory); } idField = this.schema.useLegacyNumericFields() ? LEGACY_ID : LEGACY_ID_STR; @@ -565,7 +593,7 @@ public class LuceneChangeIndex implements ChangeIndex { private void decodeMergeable(ListMultimap doc, ChangeData cd) { IndexableField f = Iterables.getFirst(doc.get(MERGEABLE_FIELD), null); - if (f != null) { + if (f != null && !skipFields.contains(MERGEABLE_FIELD)) { String mergeable = f.stringValue(); if ("1".equals(mergeable)) { cd.setMergeable(true); diff --git a/java/com/google/gerrit/lucene/LuceneGroupIndex.java b/java/com/google/gerrit/lucene/LuceneGroupIndex.java index 99cd40dae4..3d1d471768 100644 --- a/java/com/google/gerrit/lucene/LuceneGroupIndex.java +++ b/java/com/google/gerrit/lucene/LuceneGroupIndex.java @@ -17,6 +17,7 @@ package com.google.gerrit.lucene; import static com.google.common.collect.Iterables.getOnlyElement; import static com.google.gerrit.server.index.group.GroupField.UUID; +import com.google.common.collect.ImmutableSet; import com.google.gerrit.entities.AccountGroup; import com.google.gerrit.exceptions.StorageException; import com.google.gerrit.index.FieldDef; @@ -90,6 +91,7 @@ public class LuceneGroupIndex extends AbstractLuceneIndex anonymousUserProvider; final OperatorAliasConfig operatorAliasConfig; + final boolean indexMergeable; private final Provider self; @@ -253,7 +256,8 @@ public class ChangeQueryBuilder extends QueryBuilder anonymousUserProvider, - OperatorAliasConfig operatorAliasConfig) { + OperatorAliasConfig operatorAliasConfig, + @GerritServerConfig Config gerritConfig) { this( queryProvider, rewriter, @@ -281,7 +285,8 @@ public class ChangeQueryBuilder extends QueryBuilder anonymousUserProvider, - OperatorAliasConfig operatorAliasConfig) { + OperatorAliasConfig operatorAliasConfig, + boolean indexMergeable) { this.queryProvider = queryProvider; this.rewriter = rewriter; this.opFactories = opFactories; @@ -339,6 +345,7 @@ public class ChangeQueryBuilder extends QueryBuilder(FakeQueryBuilder.class), new ChangeQueryBuilder.Arguments( - null, null, null, null, null, null, null, null, null, null, null, null, null, null, - null, null, null, null, indexes, null, null, null, null, null, null, null, null)); + null, + null, + null, + null, + null, + null, + null, + null, + null, + null, + null, + null, + null, + null, + null, + null, + null, + null, + indexes, + null, + null, + null, + null, + null, + null, + null, + null, + new Config())); } @Operator diff --git a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java index 231340d65d..56b3aea7ab 100644 --- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java +++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java @@ -30,6 +30,7 @@ import static java.util.concurrent.TimeUnit.MILLISECONDS; import static java.util.concurrent.TimeUnit.MINUTES; import static java.util.concurrent.TimeUnit.SECONDS; import static java.util.stream.Collectors.toList; +import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.fail; import com.google.common.base.MoreObjects; @@ -41,6 +42,7 @@ import com.google.common.collect.Iterables; import com.google.common.collect.Lists; import com.google.common.collect.Streams; import com.google.common.truth.ThrowableSubject; +import com.google.gerrit.acceptance.config.GerritConfig; import com.google.gerrit.acceptance.testsuite.project.ProjectOperations; import com.google.gerrit.common.Nullable; import com.google.gerrit.common.data.LabelType; @@ -79,6 +81,7 @@ import com.google.gerrit.index.FieldDef; import com.google.gerrit.index.IndexConfig; import com.google.gerrit.index.Schema; import com.google.gerrit.index.query.Predicate; +import com.google.gerrit.index.query.QueryParseException; import com.google.gerrit.lifecycle.LifecycleManager; import com.google.gerrit.server.AnonymousUser; import com.google.gerrit.server.CurrentUser; @@ -2023,6 +2026,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { } @Test + @GerritConfig(name = "index.change.indexMergeable", value = "true") public void mergeable() throws Exception { TestRepository repo = createProject("repo"); RevCommit commit1 = repo.parseBody(repo.commit().add("file1", "contents1").create()); @@ -2040,7 +2044,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { // If a change gets submitted, the remaining open changes get reindexed asynchronously to update // their mergeability information. If the further assertions in this test are done before the // asynchronous reindex completed they fail because the mergeability information in the index - // was not updated yet. To avoid this flakiness reindexAfterRefUpdate is switched off for the + // was not updated yet. To avoid this flakiness indexMergeable is switched off for the // tests and we index change2 synchronously here. gApi.changes().id(change2.getChangeId()).index(); @@ -3079,6 +3083,20 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { } } + @Test + @GerritConfig(name = "index.change.indexMergeable", value = "false") + public void mergeableFailsWhenNotIndexed() throws Exception { + TestRepository repo = createProject("repo"); + RevCommit commit1 = repo.parseBody(repo.commit().add("file1", "contents1").create()); + insert(repo, newChangeForCommit(repo, commit1)); + + Throwable thrown = assertThrows(Throwable.class, () -> assertQuery("status:open is:mergeable")); + assertThat(thrown.getCause()).isInstanceOf(QueryParseException.class); + assertThat(thrown) + .hasMessageThat() + .contains("server does not support 'mergeable'. check configs"); + } + protected ChangeInserter newChange(TestRepository repo) throws Exception { return newChange(repo, null, null, null, null, false); } diff --git a/javatests/com/google/gerrit/server/query/change/BUILD b/javatests/com/google/gerrit/server/query/change/BUILD index d0162d3810..e5b51e7bce 100644 --- a/javatests/com/google/gerrit/server/query/change/BUILD +++ b/javatests/com/google/gerrit/server/query/change/BUILD @@ -16,12 +16,14 @@ java_library( "//prolog:gerrit-prolog-common", ], deps = [ + "//java/com/google/gerrit/acceptance/config", "//java/com/google/gerrit/acceptance/testsuite/project", "//java/com/google/gerrit/common:annotations", "//java/com/google/gerrit/common:server", "//java/com/google/gerrit/entities", "//java/com/google/gerrit/extensions:api", "//java/com/google/gerrit/index", + "//java/com/google/gerrit/index:query_exception", "//java/com/google/gerrit/lifecycle", "//java/com/google/gerrit/server", "//java/com/google/gerrit/server/project/testing:project-test-util",