opendev/gerrit
Code Issues Proposed changes

Merge "Replace reindexAfterRefUpdate setting with indexMergeable"

Browse Source
This commit is contained in:
Patrick Hiesel
2019-11-20 22:15:18 +00:00
committed by Gerrit Code Review
parent 106387eb6a d03143571d
commit cd30ef3c0d
25 changed files with 194 additions and 44 deletions
Download Patch File Download Diff File Expand all files Collapse all files

29
Documentation/config-gerrit.txt
View File

@@ -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

5
Documentation/rest-api-changes.txt
View File

@@ -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.
--

4
java/com/google/gerrit/acceptance/GerritServer.java
View File

@@ -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);
}
}

3
java/com/google/gerrit/elasticsearch/ElasticAccountIndex.java
View File

@@ -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<Account.Id, Accoun
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());

13
java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java
View File

@@ -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<Change.Id, ChangeData>
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 @@ class ElasticChangeIndex extends AbstractElasticIndex<Change.Id, ChangeData>
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 @@ class ElasticChangeIndex extends AbstractElasticIndex<Change.Id, ChangeData>
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<Change.Id, ChangeData>
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<Change.Id, ChangeData>
// 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);

3
java/com/google/gerrit/elasticsearch/ElasticGroupIndex.java
View File

@@ -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<AccountGroup.UUID, I
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());

3
java/com/google/gerrit/elasticsearch/ElasticProjectIndex.java
View File

@@ -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<Project.NameKey, P
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());

7
java/com/google/gerrit/elasticsearch/bulk/UpdateRequest.java
View File

@@ -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<V> extends BulkRequest {
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()));

17
java/com/google/gerrit/index/Schema.java
View File

@@ -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<T> {
* <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 @@ public class Schema<T> {
return new Values<>(f, Collections.singleton(v));
}
})
.filter(Objects::nonNull);
.filter(Objects::nonNull)
.collect(toImmutableList());
}
@Override

6
java/com/google/gerrit/lucene/AbstractLuceneIndex.java
View File

@@ -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<K, V> implements Index<K, V> {
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 @@ public abstract class AbstractLuceneIndex<K, V> implements Index<K, V> {
SitePaths sitePaths,
Directory dir,
String name,
ImmutableSet<String> skipFields,
String subIndex,
GerritIndexWriterConfig writerConfig,
SearcherFactory searcherFactory)
@@ -118,6 +121,7 @@ public abstract class AbstractLuceneIndex<K, V> implements Index<K, V> {
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<K, V> implements Index<K, V> {
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);
}

6
java/com/google/gerrit/lucene/ChangeSubIndex.java
View File

@@ -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<Change.Id, ChangeData>
Schema<ChangeData> schema,
SitePaths sitePaths,
Path path,
ImmutableSet<String> skipFields,
GerritIndexWriterConfig writerConfig,
SearcherFactory searcherFactory)
throws IOException {
@@ -56,6 +58,7 @@ public class ChangeSubIndex extends AbstractLuceneIndex<Change.Id, ChangeData>
sitePaths,
FSDirectory.open(path),
path.getFileName().toString(),
skipFields,
writerConfig,
searcherFactory);
}
@@ -65,10 +68,11 @@ public class ChangeSubIndex extends AbstractLuceneIndex<Change.Id, ChangeData>
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

2
java/com/google/gerrit/lucene/LuceneAccountIndex.java
View File

@@ -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<Account.Id, AccountS
sitePaths,
dir(schema, cfg, sitePaths),
ACCOUNTS,
ImmutableSet.of(),
null,
new GerritIndexWriterConfig(cfg, ACCOUNTS),
new SearcherFactory());

38
java/com/google/gerrit/lucene/LuceneChangeIndex.java
View File

@@ -29,6 +29,7 @@ import com.google.common.base.Throwables;
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 @@ public class LuceneChangeIndex implements ChangeIndex {
private final String idSortFieldName;
private final IdTerm idTerm;
private final ChangeIdExtractor extractor;
private final ImmutableSet<String> 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<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);

2
java/com/google/gerrit/lucene/LuceneGroupIndex.java
View File

@@ -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<AccountGroup.UUID, Int
sitePaths,
dir(schema, cfg, sitePaths),
GROUPS,
ImmutableSet.of(),
null,
new GerritIndexWriterConfig(cfg, GROUPS),
new SearcherFactory());

2
java/com/google/gerrit/lucene/LuceneProjectIndex.java
View File

@@ -17,6 +17,7 @@ package com.google.gerrit.lucene;
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 @@ public class LuceneProjectIndex extends AbstractLuceneIndex<Project.NameKey, Pro
sitePaths,
dir(schema, cfg, sitePaths),
PROJECTS,
ImmutableSet.of(),
null,
new GerritIndexWriterConfig(cfg, PROJECTS),
new SearcherFactory());

2
java/com/google/gerrit/server/index/change/ReindexAfterRefUpdate.java
View File

@@ -76,7 +76,7 @@ public class ReindexAfterRefUpdate implements GitReferenceUpdatedListener {
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

19
java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
View File

@@ -63,6 +63,7 @@ import com.google.gerrit.server.account.VersionedAccountQueries;
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.function.Function;
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 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData, ChangeQueryBuil
final GroupMembers groupMembers;
final Provider<AnonymousUser> anonymousUserProvider;
final OperatorAliasConfig operatorAliasConfig;
final boolean indexMergeable;
private final Provider<CurrentUser> self;
@@ -253,7 +256,8 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData, ChangeQueryBuil
AccountCache accountCache,
GroupMembers groupMembers,
Provider<AnonymousUser> anonymousUserProvider,
OperatorAliasConfig operatorAliasConfig) {
OperatorAliasConfig operatorAliasConfig,
@GerritServerConfig Config gerritConfig) {
this(
queryProvider,
rewriter,
@@ -281,7 +285,8 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData, ChangeQueryBuil
accountCache,
groupMembers,
anonymousUserProvider,
operatorAliasConfig);
operatorAliasConfig,
gerritConfig.getBoolean("index", "change", "indexMergeable", true));
}
private Arguments(
@@ -311,7 +316,8 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData, ChangeQueryBuil
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 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData, ChangeQueryBuil
this.groupMembers = groupMembers;
this.anonymousUserProvider = anonymousUserProvider;
this.operatorAliasConfig = operatorAliasConfig;
this.indexMergeable = indexMergeable;
}
Arguments asUser(CurrentUser otherUser) {
@@ -369,7 +376,8 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData, ChangeQueryBuil
accountCache,
groupMembers,
anonymousUserProvider,
operatorAliasConfig);
operatorAliasConfig,
indexMergeable);
}
Arguments asUser(Account.Id otherId) {
@@ -570,6 +578,9 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData, ChangeQueryBuil
}
if ("mergeable".equalsIgnoreCase(value)) {
if (!args.indexMergeable) {
throw new QueryParseException("server does not support 'mergeable'. check configs");
}
return new BooleanPredicate(ChangeField.MERGEABLE);
}

4
java/com/google/gerrit/testing/IndexConfig.java
View File

@@ -23,7 +23,9 @@ public class IndexConfig {
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");

14
javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
View File

@@ -4396,6 +4396,20 @@ public class ChangeIT extends AbstractDaemonTest {
}
}
@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");
}

2
javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
View File

@@ -46,6 +46,7 @@ import com.google.gerrit.acceptance.PushOneCommit;
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 @@ public class RevisionIT extends AbstractDaemonTest {
}
@Test
@GerritConfig(name = "index.change.indexMergeable", value = "true")
public void mergeable() throws Exception {
ObjectId initial = repo().exactRef(HEAD).getLeaf().getObjectId();

2
javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
View File

@@ -1205,7 +1205,7 @@ public abstract class AbstractSubmit extends AbstractDaemonTest {
}
@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 =

2
javatests/com/google/gerrit/acceptance/rest/change/IndexChangeIT.java
View File

@@ -22,6 +22,7 @@ import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS
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 @@ public class IndexChangeIT extends AbstractDaemonTest {
}
@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();

31
javatests/com/google/gerrit/server/index/change/FakeQueryBuilder.java
View File

@@ -18,6 +18,7 @@ import com.google.gerrit.index.query.OperatorPredicate;
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 @@ public class FakeQueryBuilder extends ChangeQueryBuilder {
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

20
javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
View File

@@ -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> 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> 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);
}

2
javatests/com/google/gerrit/server/query/change/BUILD
View File

@@ -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",