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
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 452ef23..a9d922d 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -2766,6 +2766,22 @@
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 @@
+
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 69a7641..0fc733a 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -315,6 +315,11 @@
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 73d37be..e03e084 100644
--- a/java/com/google/gerrit/acceptance/GerritServer.java
+++ b/java/com/google/gerrit/acceptance/GerritServer.java
@@ -563,8 +563,8 @@
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 a06f90f..c3e3264 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 void replace(AccountState as) {
BulkRequest bulk =
new IndexRequest(getId(as), indexName, type, client.adapter())
- .add(new UpdateRequest<>(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 37184cc..084c2ec 100644
--- a/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java
+++ b/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java
@@ -22,6 +22,7 @@
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.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.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 @@
private final ChangeData.Factory changeDataFactory;
private final Schema<ChangeData> schema;
private final FieldDef<ChangeData, ?> idField;
+ private final ImmutableSet<String> skipFields;
@Inject
ElasticChangeIndex(
@@ -98,6 +102,7 @@
ChangeData.Factory changeDataFactory,
SitePaths sitePaths,
ElasticRestClientProvider clientBuilder,
+ @GerritServerConfig Config gerritConfig,
@Assisted Schema<ChangeData> schema) {
super(cfg, sitePaths, schema, clientBuilder, CHANGES);
this.changeDataFactory = changeDataFactory;
@@ -105,6 +110,10 @@
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 @@
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 @@
// 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 c215132..ce2025f 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 void replace(InternalGroup group) {
BulkRequest bulk =
new IndexRequest(getId(group), indexName, type, client.adapter())
- .add(new UpdateRequest<>(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 29f8507..b636706 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 void replace(ProjectData projectState) {
BulkRequest bulk =
new IndexRequest(projectState.getProject().getName(), indexName, type, client.adapter())
- .add(new UpdateRequest<>(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 2f0bd01..196b8d6 100644
--- a/java/com/google/gerrit/elasticsearch/bulk/UpdateRequest.java
+++ b/java/com/google/gerrit/elasticsearch/bulk/UpdateRequest.java
@@ -16,6 +16,7 @@
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 @@
private final Schema<V> schema;
private final V v;
+ private final ImmutableSet<String> skipFields;
- public UpdateRequest(Schema<V> schema, V v) {
+ public UpdateRequest(Schema<V> schema, V v, ImmutableSet<String> 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<V> values : schema.buildFields(v)) {
+ for (Values<V> 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 f9f8c48..0aa374b 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 @@
* <p>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<Values<T>> buildFields(T obj) {
- return FluentIterable.from(fields.values())
- .transform(
+ public final Iterable<Values<T>> buildFields(T obj, ImmutableSet<String> 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 @@
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 deb3203..5392ab4 100644
--- a/java/com/google/gerrit/lucene/AbstractLuceneIndex.java
+++ b/java/com/google/gerrit/lucene/AbstractLuceneIndex.java
@@ -21,6 +21,7 @@
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 @@
private final SitePaths sitePaths;
private final Directory dir;
private final String name;
+ private final ImmutableSet<String> skipFields;
private final ListeningExecutorService writerThread;
private final IndexWriter writer;
private final ReferenceManager<IndexSearcher> searcherManager;
@@ -110,6 +112,7 @@
SitePaths sitePaths,
Directory dir,
String name,
+ ImmutableSet<String> skipFields,
String subIndex,
GerritIndexWriterConfig writerConfig,
SearcherFactory searcherFactory)
@@ -118,6 +121,7 @@
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 @@
Document toDocument(V obj) {
Document result = new Document();
- for (Values<V> vs : schema.buildFields(obj)) {
+ for (Values<V> 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 fd439f1..e51a91a7 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.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 @@
Schema<ChangeData> schema,
SitePaths sitePaths,
Path path,
+ ImmutableSet<String> skipFields,
GerritIndexWriterConfig writerConfig,
SearcherFactory searcherFactory)
throws IOException {
@@ -56,6 +58,7 @@
sitePaths,
FSDirectory.open(path),
path.getFileName().toString(),
+ skipFields,
writerConfig,
searcherFactory);
}
@@ -65,10 +68,11 @@
SitePaths sitePaths,
Directory dir,
String subIndex,
+ ImmutableSet<String> 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 efd7ea3..242cffd 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_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 @@
sitePaths,
dir(schema, cfg, sitePaths),
ACCOUNTS,
+ ImmutableSet.of(),
null,
new GerritIndexWriterConfig(cfg, ACCOUNTS),
new SearcherFactory());
diff --git a/java/com/google/gerrit/lucene/LuceneChangeIndex.java b/java/com/google/gerrit/lucene/LuceneChangeIndex.java
index 16d66b6..3b277dd 100644
--- a/java/com/google/gerrit/lucene/LuceneChangeIndex.java
+++ b/java/com/google/gerrit/lucene/LuceneChangeIndex.java
@@ -29,6 +29,7 @@
import com.google.common.collect.Collections2;
import com.google.common.collect.FluentIterable;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.ListMultimap;
import com.google.common.collect.MultimapBuilder;
@@ -167,6 +168,7 @@
private final String idSortFieldName;
private final IdTerm idTerm;
private final ChangeIdExtractor extractor;
+ private final ImmutableSet<String> skipFields;
@Inject
LuceneChangeIndex(
@@ -179,6 +181,10 @@
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 @@
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 @@
private void decodeMergeable(ListMultimap<String, IndexableField> 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 99cd40d..3d1d471 100644
--- a/java/com/google/gerrit/lucene/LuceneGroupIndex.java
+++ b/java/com/google/gerrit/lucene/LuceneGroupIndex.java
@@ -17,6 +17,7 @@
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 @@
sitePaths,
dir(schema, cfg, sitePaths),
GROUPS,
+ ImmutableSet.of(),
null,
new GerritIndexWriterConfig(cfg, GROUPS),
new SearcherFactory());
diff --git a/java/com/google/gerrit/lucene/LuceneProjectIndex.java b/java/com/google/gerrit/lucene/LuceneProjectIndex.java
index 97454c7..a3a0d9c 100644
--- a/java/com/google/gerrit/lucene/LuceneProjectIndex.java
+++ b/java/com/google/gerrit/lucene/LuceneProjectIndex.java
@@ -17,6 +17,7 @@
import static com.google.common.collect.Iterables.getOnlyElement;
import static com.google.gerrit.index.project.ProjectField.NAME;
+import com.google.common.collect.ImmutableSet;
import com.google.gerrit.entities.Project;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.index.FieldDef;
@@ -90,6 +91,7 @@
sitePaths,
dir(schema, cfg, sitePaths),
PROJECTS,
+ ImmutableSet.of(),
null,
new GerritIndexWriterConfig(cfg, PROJECTS),
new SearcherFactory());
diff --git a/java/com/google/gerrit/server/index/change/ReindexAfterRefUpdate.java b/java/com/google/gerrit/server/index/change/ReindexAfterRefUpdate.java
index f6d3b6f..5efa065 100644
--- a/java/com/google/gerrit/server/index/change/ReindexAfterRefUpdate.java
+++ b/java/com/google/gerrit/server/index/change/ReindexAfterRefUpdate.java
@@ -76,7 +76,7 @@
this.accountCache = accountCache;
this.indexer = indexer;
this.executor = executor;
- this.enabled = cfg.getBoolean("index", null, "reindexAfterRefUpdate", true);
+ this.enabled = cfg.getBoolean("index", "change", "indexMergeable", true);
}
@Override
diff --git a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
index c86bd94..61b90f1 100644
--- a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
+++ b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
@@ -63,6 +63,7 @@
import com.google.gerrit.server.change.ChangeTriplet;
import com.google.gerrit.server.config.AllProjectsName;
import com.google.gerrit.server.config.AllUsersName;
+import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.OperatorAliasConfig;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.index.change.ChangeField;
@@ -94,6 +95,7 @@
import java.util.regex.Pattern;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
+import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.Repository;
/** Parses a query string meant to be applied to change objects. */
@@ -221,6 +223,7 @@
final GroupMembers groupMembers;
final Provider<AnonymousUser> anonymousUserProvider;
final OperatorAliasConfig operatorAliasConfig;
+ final boolean indexMergeable;
private final Provider<CurrentUser> self;
@@ -253,7 +256,8 @@
AccountCache accountCache,
GroupMembers groupMembers,
Provider<AnonymousUser> anonymousUserProvider,
- OperatorAliasConfig operatorAliasConfig) {
+ OperatorAliasConfig operatorAliasConfig,
+ @GerritServerConfig Config gerritConfig) {
this(
queryProvider,
rewriter,
@@ -281,7 +285,8 @@
accountCache,
groupMembers,
anonymousUserProvider,
- operatorAliasConfig);
+ operatorAliasConfig,
+ gerritConfig.getBoolean("index", "change", "indexMergeable", true));
}
private Arguments(
@@ -311,7 +316,8 @@
AccountCache accountCache,
GroupMembers groupMembers,
Provider<AnonymousUser> anonymousUserProvider,
- OperatorAliasConfig operatorAliasConfig) {
+ OperatorAliasConfig operatorAliasConfig,
+ boolean indexMergeable) {
this.queryProvider = queryProvider;
this.rewriter = rewriter;
this.opFactories = opFactories;
@@ -339,6 +345,7 @@
this.groupMembers = groupMembers;
this.anonymousUserProvider = anonymousUserProvider;
this.operatorAliasConfig = operatorAliasConfig;
+ this.indexMergeable = indexMergeable;
}
Arguments asUser(CurrentUser otherUser) {
@@ -369,7 +376,8 @@
accountCache,
groupMembers,
anonymousUserProvider,
- operatorAliasConfig);
+ operatorAliasConfig,
+ indexMergeable);
}
Arguments asUser(Account.Id otherId) {
@@ -570,6 +578,9 @@
}
if ("mergeable".equalsIgnoreCase(value)) {
+ if (!args.indexMergeable) {
+ throw new QueryParseException("server does not support 'mergeable'. check configs");
+ }
return new BooleanPredicate(ChangeField.MERGEABLE);
}
diff --git a/java/com/google/gerrit/testing/IndexConfig.java b/java/com/google/gerrit/testing/IndexConfig.java
index 21c49dd..fb6c926 100644
--- a/java/com/google/gerrit/testing/IndexConfig.java
+++ b/java/com/google/gerrit/testing/IndexConfig.java
@@ -23,7 +23,9 @@
public static Config createFromExistingConfig(Config cfg) {
cfg.setInt("index", null, "maxPages", 10);
- cfg.setBoolean("index", null, "reindexAfterRefUpdate", false);
+ // To avoid this flakiness indexMergeable is switched off for the tests as it incurs background
+ // reindex calls.
+ cfg.setBoolean("index", "change", "indexMergeable", false);
cfg.setString("trackingid", "query-bug", "footer", "Bug:");
cfg.setString("trackingid", "query-bug", "match", "QUERY\\d{2,8}");
cfg.setString("trackingid", "query-bug", "system", "querytests");
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 1df5ca3..1a4faa2 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -4393,6 +4393,20 @@
}
}
+ @Test
+ @GerritConfig(name = "index.change.indexMergeable", value = "true")
+ public void changeQueryReturnsMergeableWhenGerritIndexMergeable() throws Exception {
+ String changeId = createChange().getChangeId();
+ assertThat(gApi.changes().query(changeId).get().get(0).mergeable).isTrue();
+ }
+
+ @Test
+ @GerritConfig(name = "index.change.indexMergeable", value = "false")
+ public void changeQueryDoesNotReturnMergeableWhenGerritDoesNotIndexMergeable() throws Exception {
+ String changeId = createChange().getChangeId();
+ assertThat(gApi.changes().query(changeId).get().get(0).mergeable).isNull();
+ }
+
private PushOneCommit.Result createWorkInProgressChange() throws Exception {
return pushTo("refs/for/master%wip");
}
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
index ad73e0f..61d0fd5 100644
--- a/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
@@ -46,6 +46,7 @@
import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.acceptance.TestAccount;
import com.google.gerrit.acceptance.TestProjectInput;
+import com.google.gerrit.acceptance.config.GerritConfig;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
import com.google.gerrit.common.data.Permission;
@@ -1062,6 +1063,7 @@
}
@Test
+ @GerritConfig(name = "index.change.indexMergeable", value = "true")
public void mergeable() throws Exception {
ObjectId initial = repo().exactRef(HEAD).getLeaf().getObjectId();
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java b/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
index aaf52a1..eff98b3 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
@@ -1205,7 +1205,7 @@
}
@Test
- @GerritConfig(name = "index.reindexAfterRefUpdate", value = "true")
+ @GerritConfig(name = "index.change.indexMergeable", value = "true")
public void submitSchedulesOpenChangesOfSameBranchForReindexing() throws Throwable {
// Create a merged change.
PushOneCommit push =
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/IndexChangeIT.java b/javatests/com/google/gerrit/acceptance/rest/change/IndexChangeIT.java
index 3030b02..ea3a6a0 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/IndexChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/IndexChangeIT.java
@@ -22,6 +22,7 @@
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.TestAccount;
+import com.google.gerrit.acceptance.config.GerritConfig;
import com.google.gerrit.acceptance.testsuite.group.GroupOperations;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
@@ -58,6 +59,7 @@
}
@Test
+ @GerritConfig(name = "index.change.indexMergeable", value = "true")
public void indexChangeAfterOwnerLosesVisibility() throws Exception {
// Create a test group with 2 users as members
TestAccount user2 = accountCreator.user2();
diff --git a/javatests/com/google/gerrit/server/index/change/FakeQueryBuilder.java b/javatests/com/google/gerrit/server/index/change/FakeQueryBuilder.java
index 9a48a68..3dfbefe 100644
--- a/javatests/com/google/gerrit/server/index/change/FakeQueryBuilder.java
+++ b/javatests/com/google/gerrit/server/index/change/FakeQueryBuilder.java
@@ -18,6 +18,7 @@
import com.google.gerrit.index.query.Predicate;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.ChangeQueryBuilder;
+import org.eclipse.jgit.lib.Config;
import org.junit.Ignore;
@Ignore
@@ -26,8 +27,34 @@
super(
new ChangeQueryBuilder.Definition<>(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 231340d..56b3aea 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.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.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.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 @@
}
@Test
+ @GerritConfig(name = "index.change.indexMergeable", value = "true")
public void mergeable() throws Exception {
TestRepository<Repo> repo = createProject("repo");
RevCommit commit1 = repo.parseBody(repo.commit().add("file1", "contents1").create());
@@ -2040,7 +2044,7 @@
// 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 @@
}
}
+ @Test
+ @GerritConfig(name = "index.change.indexMergeable", value = "false")
+ public void mergeableFailsWhenNotIndexed() throws Exception {
+ TestRepository<Repo> 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> 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 d0162d3..e5b51e7 100644
--- a/javatests/com/google/gerrit/server/query/change/BUILD
+++ b/javatests/com/google/gerrit/server/query/change/BUILD
@@ -16,12 +16,14 @@
"//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",
