InternalChangeQuery: Don't batch exact commit queries

The batching functionality is to work around performance problems in
secondary index backends when handling lots of prefix queries. If we
have the exact commit field, then this query has no prefix queries at
all, so no need to do this workaround and split up into multiple
queries on the backend.

Change-Id: Ia150eec1c36c403f9e6d16a85b9d857da840f957
This commit is contained in:
Dave Borowitz
2015-08-11 16:58:37 -04:00
parent d8b52583a5
commit 6e66b089f9
2 changed files with 35 additions and 16 deletions

View File

@@ -27,6 +27,7 @@ import com.google.gerrit.common.Nullable;
import com.google.gerrit.reviewdb.client.Branch; import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.index.ChangeField;
import com.google.gerrit.server.index.ChangeIndex; import com.google.gerrit.server.index.ChangeIndex;
import com.google.gerrit.server.index.IndexCollection; import com.google.gerrit.server.index.IndexCollection;
import com.google.gerrit.server.index.IndexConfig; import com.google.gerrit.server.index.IndexConfig;
@@ -126,27 +127,25 @@ public class InternalChangeQuery {
public Iterable<ChangeData> byCommitsOnBranchNotMerged(Branch.NameKey branch, public Iterable<ChangeData> byCommitsOnBranchNotMerged(Branch.NameKey branch,
List<String> hashes) throws OrmException { List<String> hashes) throws OrmException {
Schema<ChangeData> schema = schema(indexes);
if (schema.hasField(ChangeField.EXACT_COMMIT)) {
return query(commitsOnBranchNotMerged(branch, commits(schema, hashes)));
} else {
return byCommitsOnBranchNotMerged( return byCommitsOnBranchNotMerged(
branch, hashes, indexConfig.maxPrefixTerms()); schema, branch, hashes, indexConfig.maxPrefixTerms());
}
} }
@VisibleForTesting @VisibleForTesting
Iterable<ChangeData> byCommitsOnBranchNotMerged(Branch.NameKey branch, Iterable<ChangeData> byCommitsOnBranchNotMerged(Schema<ChangeData> schema,
List<String> hashes, int batchSize) throws OrmException { Branch.NameKey branch, List<String> hashes, int batchSize)
Schema<ChangeData> schema = schema(indexes); throws OrmException {
List<Predicate<ChangeData>> commits = new ArrayList<>(hashes.size()); List<Predicate<ChangeData>> commits = commits(schema, hashes);
for (String s : hashes) {
commits.add(commit(schema, s));
}
int numBatches = (hashes.size() / batchSize) + 1; int numBatches = (hashes.size() / batchSize) + 1;
List<Predicate<ChangeData>> queries = new ArrayList<>(numBatches); List<Predicate<ChangeData>> queries = new ArrayList<>(numBatches);
for (List<Predicate<ChangeData>> batch for (List<Predicate<ChangeData>> batch
: Iterables.partition(commits, batchSize)) { : Iterables.partition(commits, batchSize)) {
queries.add(and( queries.add(commitsOnBranchNotMerged(branch, batch));
ref(branch),
project(branch.getParentKey()),
not(status(Change.Status.MERGED)),
or(batch)));
} }
try { try {
return FluentIterable.from(qp.queryChanges(queries)) return FluentIterable.from(qp.queryChanges(queries))
@@ -161,6 +160,24 @@ public class InternalChangeQuery {
} }
} }
private static List<Predicate<ChangeData>> commits(Schema<ChangeData> schema,
List<String> hashes) {
List<Predicate<ChangeData>> commits = new ArrayList<>(hashes.size());
for (String s : hashes) {
commits.add(commit(schema, s));
}
return commits;
}
private static Predicate<ChangeData> commitsOnBranchNotMerged(
Branch.NameKey branch, List<Predicate<ChangeData>> commits) {
return and(
ref(branch),
project(branch.getParentKey()),
not(status(Change.Status.MERGED)),
or(commits));
}
public List<ChangeData> byProjectOpen(Project.NameKey project) public List<ChangeData> byProjectOpen(Project.NameKey project)
throws OrmException { throws OrmException {
return query(and(project(project), open())); return query(and(project(project), open()));

View File

@@ -54,6 +54,7 @@ import com.google.gerrit.server.change.ChangeInserter;
import com.google.gerrit.server.change.ChangeTriplet; import com.google.gerrit.server.change.ChangeTriplet;
import com.google.gerrit.server.change.PatchSetInserter; import com.google.gerrit.server.change.PatchSetInserter;
import com.google.gerrit.server.change.PatchSetInserter.ValidatePolicy; import com.google.gerrit.server.change.PatchSetInserter.ValidatePolicy;
import com.google.gerrit.server.index.IndexCollection;
import com.google.gerrit.server.notedb.NotesMigration; import com.google.gerrit.server.notedb.NotesMigration;
import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.ProjectControl; import com.google.gerrit.server.project.ProjectControl;
@@ -116,6 +117,7 @@ public abstract class AbstractQueryChangesTest {
@Inject protected ChangeControl.GenericFactory changeControlFactory; @Inject protected ChangeControl.GenericFactory changeControlFactory;
@Inject protected GerritApi gApi; @Inject protected GerritApi gApi;
@Inject protected IdentifiedUser.GenericFactory userFactory; @Inject protected IdentifiedUser.GenericFactory userFactory;
@Inject protected IndexCollection indexes;
@Inject protected InMemoryDatabase schemaFactory; @Inject protected InMemoryDatabase schemaFactory;
@Inject protected InMemoryRepositoryManager repoManager; @Inject protected InMemoryRepositoryManager repoManager;
@Inject protected InternalChangeQuery internalChangeQuery; @Inject protected InternalChangeQuery internalChangeQuery;
@@ -1156,8 +1158,8 @@ public abstract class AbstractQueryChangesTest {
} }
for (int i = 1; i <= 11; i++) { for (int i = 1; i <= 11; i++) {
Iterable<ChangeData> cds = Iterable<ChangeData> cds = internalChangeQuery.byCommitsOnBranchNotMerged(
internalChangeQuery.byCommitsOnBranchNotMerged(dest, shas, i); indexes.getSearchIndex().getSchema(), dest, shas, i);
Iterable<Integer> ids = FluentIterable.from(cds).transform( Iterable<Integer> ids = FluentIterable.from(cds).transform(
new Function<ChangeData, Integer>() { new Function<ChangeData, Integer>() {
@Override @Override