Write change mergeability info to a persistent cache
The mergeability bit and the last SHA-1 it was checked against are not
really properties of the change, but rather properties of the state of
Gerrit's mergeability checker. Moreover, mergeability checking happens
sufficiently often (at least on every branch update) that it would
clutter information in the notedb.
As a first step towards eliminating these fields from Change, start
writing them to a persistent loading cache when the value is
recomputed. Cache only the most recent value for each change.
The cache just records the commit, the branch tip it was tested
against, the submit type, and merge strategy; this result is fixed for
all time (modulo implementation changes) regardless of change IDs.
Change-Id: I0c1c4b73725a6de3fc90b21f3e65714ad2208a6e
diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Reindex.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Reindex.java
index bcd34bc..9848351 100644
--- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Reindex.java
+++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Reindex.java
@@ -28,6 +28,8 @@
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.cache.h2.DefaultCacheFactory;
+import com.google.gerrit.server.change.MergeabilityCache;
import com.google.gerrit.server.change.MergeabilityChecker;
import com.google.gerrit.server.change.MergeabilityChecksExecutor;
import com.google.gerrit.server.change.MergeabilityChecksExecutor.Priority;
@@ -175,6 +177,8 @@
factory(MergeUtil.Factory.class);
install(new NoteDbModule());
install(new BatchGitModule());
+ install(new DefaultCacheFactory.Module());
+ install(MergeabilityCache.module());
}
@Provides
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/MergeabilityCache.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/MergeabilityCache.java
new file mode 100644
index 0000000..b468f35
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/MergeabilityCache.java
@@ -0,0 +1,301 @@
+// Copyright (C) 2014 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.change;
+
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.base.Preconditions.checkState;
+import static com.google.gerrit.server.ioutil.BasicSerialization.readString;
+import static com.google.gerrit.server.ioutil.BasicSerialization.writeString;
+import static org.eclipse.jgit.lib.ObjectIdSerialization.readNotNull;
+import static org.eclipse.jgit.lib.ObjectIdSerialization.writeNotNull;
+
+import com.google.common.base.MoreObjects;
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.LoadingCache;
+import com.google.common.cache.Weigher;
+import com.google.common.collect.BiMap;
+import com.google.common.collect.ImmutableBiMap;
+import com.google.common.collect.Sets;
+import com.google.gerrit.extensions.common.SubmitType;
+import com.google.gerrit.reviewdb.client.Branch;
+import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.cache.CacheModule;
+import com.google.gerrit.server.git.CodeReviewCommit;
+import com.google.gerrit.server.git.MergeException;
+import com.google.gerrit.server.git.strategy.SubmitStrategyFactory;
+import com.google.gerrit.server.project.NoSuchProjectException;
+import com.google.inject.Inject;
+import com.google.inject.Module;
+import com.google.inject.Singleton;
+import com.google.inject.name.Named;
+
+import org.eclipse.jgit.errors.IncorrectObjectTypeException;
+import org.eclipse.jgit.errors.MissingObjectException;
+import org.eclipse.jgit.lib.Constants;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.revwalk.RevFlag;
+import org.eclipse.jgit.revwalk.RevWalk;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.io.ObjectInputStream;
+import java.io.ObjectOutputStream;
+import java.io.Serializable;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import java.util.concurrent.ExecutionException;
+
+@Singleton
+public class MergeabilityCache {
+ private static final Logger log =
+ LoggerFactory.getLogger(MergeabilityCache.class);
+
+ private static final String CACHE_NAME = "mergeability";
+
+ public static final BiMap<SubmitType, Character> SUBMIT_TYPES = ImmutableBiMap.of(
+ SubmitType.FAST_FORWARD_ONLY, 'F',
+ SubmitType.MERGE_IF_NECESSARY, 'M',
+ SubmitType.REBASE_IF_NECESSARY, 'R',
+ SubmitType.MERGE_ALWAYS, 'A',
+ SubmitType.CHERRY_PICK, 'C');
+
+ static {
+ checkState(SUBMIT_TYPES.size() == SubmitType.values().length,
+ "SubmitType <-> char BiMap needs updating");
+ }
+
+ public static Module module() {
+ return new CacheModule() {
+ @Override
+ protected void configure() {
+ persist(CACHE_NAME, EntryKey.class, Boolean.class)
+ .maximumWeight(1 << 20)
+ .weigher(MergeabilityWeigher.class)
+ .loader(Loader.class);
+ bind(MergeabilityCache.class);
+ }
+ };
+ }
+
+ public static class EntryKey implements Serializable {
+ private static final long serialVersionUID = 1L;
+
+ private ObjectId commit;
+ private ObjectId into;
+ private SubmitType submitType;
+ private String mergeStrategy;
+
+ // Only used for loading, not stored.
+ private transient LoadHelper load;
+
+ public EntryKey(ObjectId commit, ObjectId into, SubmitType submitType,
+ String mergeStrategy) {
+ this.commit = checkNotNull(commit, "commit");
+ this.into = checkNotNull(into, "into");
+ this.submitType = checkNotNull(submitType, "submitType");
+ this.mergeStrategy = checkNotNull(mergeStrategy, "mergeStrategy");
+ }
+
+ private EntryKey(ObjectId commit, ObjectId into, SubmitType submitType,
+ String mergeStrategy, Branch.NameKey dest, Repository repo,
+ ReviewDb db) {
+ this(commit, into, submitType, mergeStrategy);
+ load = new LoadHelper(dest, repo, db);
+ }
+
+ public ObjectId getCommit() {
+ return commit;
+ }
+
+ public ObjectId getInto() {
+ return into;
+ }
+
+ public SubmitType getSubmitType() {
+ return submitType;
+ }
+
+ public String getMergeStrategy() {
+ return mergeStrategy;
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ if (o instanceof EntryKey) {
+ EntryKey k = (EntryKey) o;
+ return commit.equals(k.commit)
+ && into.equals(k.into)
+ && submitType == k.submitType
+ && mergeStrategy.equals(k.mergeStrategy);
+ }
+ return false;
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(commit, into, submitType, mergeStrategy);
+ }
+
+ @Override
+ public String toString() {
+ return MoreObjects.toStringHelper(this)
+ .add("commit", commit.name())
+ .add("into", into.name())
+ .addValue(submitType)
+ .addValue(mergeStrategy)
+ .toString();
+ }
+
+ private void writeObject(ObjectOutputStream out) throws IOException {
+ writeNotNull(out, commit);
+ writeNotNull(out, into);
+ Character c = SUBMIT_TYPES.get(submitType);
+ if (c == null) {
+ throw new IOException("Invalid submit type: " + submitType);
+ }
+ out.writeChar(c);
+ writeString(out, mergeStrategy);
+ }
+
+ private void readObject(ObjectInputStream in) throws IOException {
+ commit = readNotNull(in);
+ into = readNotNull(in);
+ char t = in.readChar();
+ submitType = SUBMIT_TYPES.inverse().get(t);
+ if (submitType == null) {
+ throw new IOException("Invalid submit type code: " + t);
+ }
+ mergeStrategy = readString(in);
+ }
+ }
+
+ private static class LoadHelper {
+ private final Branch.NameKey dest;
+ private final Repository repo;
+ private final ReviewDb db;
+
+ private LoadHelper(Branch.NameKey dest, Repository repo, ReviewDb db) {
+ this.dest = checkNotNull(dest, "dest");
+ this.repo = checkNotNull(repo, "repo");
+ this.db = checkNotNull(db, "db");
+ }
+ }
+
+ @Singleton
+ public static class Loader extends CacheLoader<EntryKey, Boolean> {
+ private final SubmitStrategyFactory submitStrategyFactory;
+
+ @Inject
+ Loader(SubmitStrategyFactory submitStrategyFactory) {
+ this.submitStrategyFactory = submitStrategyFactory;
+ }
+
+ @Override
+ public Boolean load(EntryKey key)
+ throws NoSuchProjectException, MergeException, IOException {
+ checkArgument(key.load != null, "Key cannot be loaded: %s", key);
+ if (key.into.equals(ObjectId.zeroId())) {
+ return true; // Assume yes on new branch.
+ }
+ try {
+ Map<String, Ref> refs = key.load.repo.getAllRefs();
+ RevWalk rw = CodeReviewCommit.newRevWalk(key.load.repo);
+ try {
+ RevFlag canMerge = rw.newFlag("CAN_MERGE");
+ CodeReviewCommit rev = parse(rw, key.commit);
+ rev.add(canMerge);
+ CodeReviewCommit tip = parse(rw, key.into);
+ Set<RevCommit> accepted = alreadyAccepted(rw, refs.values());
+ accepted.add(tip);
+ accepted.addAll(Arrays.asList(rev.getParents()));
+ return submitStrategyFactory.create(
+ key.submitType,
+ key.load.db,
+ key.load.repo,
+ rw,
+ null /*inserter*/,
+ canMerge,
+ accepted,
+ key.load.dest).dryRun(tip, rev);
+ } finally {
+ rw.release();
+ }
+ } finally {
+ key.load = null;
+ }
+ }
+
+ private static Set<RevCommit> alreadyAccepted(RevWalk rw,
+ Collection<Ref> refs) throws MissingObjectException, IOException {
+ Set<RevCommit> accepted = Sets.newHashSet();
+ for (Ref r : refs) {
+ if (r.getName().startsWith(Constants.R_HEADS)
+ || r.getName().startsWith(Constants.R_TAGS)) {
+ try {
+ accepted.add(rw.parseCommit(r.getObjectId()));
+ } catch (IncorrectObjectTypeException nonCommit) {
+ // Not a commit? Skip over it.
+ }
+ }
+ }
+ return accepted;
+ }
+
+ private static CodeReviewCommit parse(RevWalk rw, ObjectId id)
+ throws MissingObjectException, IncorrectObjectTypeException,
+ IOException {
+ return (CodeReviewCommit) rw.parseCommit(id);
+ }
+ }
+
+ public static class MergeabilityWeigher
+ implements Weigher<EntryKey, Boolean> {
+ @Override
+ public int weigh(EntryKey k, Boolean v) {
+ return 16 + 2 * (16 + 20) + 3 * 8 // Size of EntryKey, 64-bit JVM.
+ + 8; // Size of Boolean.
+ }
+ }
+
+ private final LoadingCache<EntryKey, Boolean> cache;
+
+ @Inject
+ MergeabilityCache(@Named(CACHE_NAME) LoadingCache<EntryKey, Boolean> cache) {
+ this.cache = cache;
+ }
+
+ public boolean get(ObjectId commit, Ref intoRef, SubmitType submitType,
+ String mergeStrategy, Branch.NameKey dest, Repository repo, ReviewDb db) {
+ ObjectId into = intoRef != null ? intoRef.getObjectId() : ObjectId.zeroId();
+ EntryKey key =
+ new EntryKey(commit, into, submitType, mergeStrategy, dest, repo, db);
+ try {
+ return cache.get(key);
+ } catch (ExecutionException e) {
+ log.error(String.format("Error checking mergeability of %s into %s (%s)",
+ key.commit.name(), key.into.name(), key.submitType.name()),
+ e.getCause());
+ return false;
+ }
+ }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Mergeable.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Mergeable.java
index e5800e3..fee87ef 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Mergeable.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Mergeable.java
@@ -14,7 +14,8 @@
package com.google.gerrit.server.change;
-import com.google.common.collect.Sets;
+import static com.google.gerrit.extensions.common.SubmitType.CHERRY_PICK;
+
import com.google.gerrit.common.data.SubmitTypeRecord;
import com.google.gerrit.extensions.common.SubmitType;
import com.google.gerrit.extensions.restapi.AuthException;
@@ -26,13 +27,11 @@
import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.git.BranchOrderSection;
-import com.google.gerrit.server.git.CodeReviewCommit;
import com.google.gerrit.server.git.GitRepositoryManager;
-import com.google.gerrit.server.git.MergeException;
-import com.google.gerrit.server.git.strategy.SubmitStrategyFactory;
+import com.google.gerrit.server.git.MergeUtil;
import com.google.gerrit.server.index.ChangeIndexer;
-import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.ProjectCache;
+import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.project.SubmitRuleEvaluator;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gwtorm.server.AtomicUpdate;
@@ -40,28 +39,17 @@
import com.google.inject.Inject;
import com.google.inject.Provider;
-import org.eclipse.jgit.errors.IncorrectObjectTypeException;
-import org.eclipse.jgit.errors.MissingObjectException;
-import org.eclipse.jgit.lib.AnyObjectId;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Ref;
-import org.eclipse.jgit.lib.RefDatabase;
import org.eclipse.jgit.lib.Repository;
-import org.eclipse.jgit.revwalk.RevCommit;
-import org.eclipse.jgit.revwalk.RevFlag;
-import org.eclipse.jgit.revwalk.RevWalk;
import org.kohsuke.args4j.Option;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.io.IOException;
import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collection;
import java.util.List;
-import java.util.Map;
-import java.util.Set;
public class Mergeable implements RestReadView<RevisionResource> {
private static final Logger log = LoggerFactory.getLogger(Mergeable.class);
@@ -88,10 +76,11 @@
private final GitRepositoryManager gitManager;
private final ProjectCache projectCache;
+ private final MergeUtil.Factory mergeUtilFactory;
private final ChangeData.Factory changeDataFactory;
- private final SubmitStrategyFactory submitStrategyFactory;
private final Provider<ReviewDb> db;
private final ChangeIndexer indexer;
+ private final MergeabilityCache cache;
private boolean force;
private boolean reindex;
@@ -99,16 +88,18 @@
@Inject
Mergeable(GitRepositoryManager gitManager,
ProjectCache projectCache,
+ MergeUtil.Factory mergeUtilFactory,
ChangeData.Factory changeDataFactory,
- SubmitStrategyFactory submitStrategyFactory,
Provider<ReviewDb> db,
- ChangeIndexer indexer) {
+ ChangeIndexer indexer,
+ MergeabilityCache cache) {
this.gitManager = gitManager;
this.projectCache = projectCache;
+ this.mergeUtilFactory = mergeUtilFactory;
this.changeDataFactory = changeDataFactory;
- this.submitStrategyFactory = submitStrategyFactory;
this.db = db;
this.indexer = indexer;
+ this.cache = cache;
reindex = true;
}
@@ -138,25 +129,31 @@
Repository git = gitManager.openRepository(change.getProject());
try {
- Map<String, Ref> refs = git.getRefDatabase().getRefs(RefDatabase.ALL);
- Ref ref = refs.get(change.getDest().get());
- if (force || isStale(change, ref)) {
+ Ref ref = git.getRef(change.getDest().get());
+ boolean refresh = force || isStale(change, ref);
+ if (!refresh && !otherBranches) {
+ return result;
+ }
+ ProjectState projectState = projectCache.get(change.getProject());
+ String strategy = mergeUtilFactory.create(projectState)
+ .mergeStrategyName();
+
+ if (refresh) {
result.mergeable =
- refresh(change, ps, result.submitType, git, refs, ref);
+ refresh(change, ps, ref, result.submitType, strategy, git);
}
if (otherBranches) {
result.mergeableInto = new ArrayList<>();
- BranchOrderSection branchOrder =
- projectCache.get(change.getProject()).getBranchOrderSection();
+ BranchOrderSection branchOrder = projectState.getBranchOrderSection();
if (branchOrder != null) {
int prefixLen = Constants.R_HEADS.length();
for (String n : branchOrder.getMoreStable(ref.getName())) {
- Ref other = refs.get(n);
+ Ref other = git.getRef(n);
if (other == null) {
continue;
}
- if (isMergeable(change, ps, SubmitType.CHERRY_PICK, git, refs, other)) {
+ if (isMergeable(change, ps, other, CHERRY_PICK, strategy, git)) {
result.mergeableInto.add(other.getName().substring(prefixLen));
}
}
@@ -179,14 +176,10 @@
: "");
}
- private boolean refresh(Change change,
- final PatchSet ps,
- SubmitType type,
- Repository git,
- Map<String, Ref> refs,
- final Ref ref) throws IOException, OrmException {
-
- final boolean mergeable = isMergeable(change, ps, type, git, refs, ref);
+ private boolean refresh(final Change change, final PatchSet ps,
+ final Ref ref, SubmitType type, String strategy, Repository git)
+ throws OrmException, IOException {
+ final boolean mergeable = isMergeable(change, ps, ref, type, strategy, git);
Change c = db.get().changes().atomicUpdate(
change.getId(),
@@ -209,80 +202,16 @@
return mergeable;
}
- private boolean isMergeable(Change change,
- final PatchSet ps,
- SubmitType type,
- Repository git,
- Map<String, Ref> refs,
- final Ref ref) {
- RevWalk rw = new RevWalk(git) {
- @Override
- protected CodeReviewCommit createCommit(AnyObjectId id) {
- return new CodeReviewCommit(id);
- }
- };
+ private boolean isMergeable(Change change, PatchSet ps, Ref ref,
+ SubmitType type, String strategy, Repository git) {
+ ObjectId commit;
try {
- ObjectId id;
- try {
- id = ObjectId.fromString(ps.getRevision().get());
- } catch (IllegalArgumentException e) {
- log.error(String.format(
- "Invalid revision on patch set %d of %d",
- ps.getId().get(),
- change.getId().get()));
- return false;
- }
-
- RevFlag canMerge = rw.newFlag("CAN_MERGE");
- CodeReviewCommit rev = parse(rw, id);
- rev.add(canMerge);
-
- final boolean mergeable;
- if (ref == null || ref.getObjectId() == null) {
- mergeable = true; // Assume yes on new branch.
- } else {
- CodeReviewCommit tip = parse(rw, ref.getObjectId());
- Set<RevCommit> accepted = alreadyAccepted(rw, refs.values());
- accepted.add(tip);
- accepted.addAll(Arrays.asList(rev.getParents()));
- mergeable = submitStrategyFactory.create(
- type,
- db.get(),
- git,
- rw,
- null /*inserter*/,
- canMerge,
- accepted,
- change.getDest()).dryRun(tip, rev);
- }
- return mergeable;
- } catch (MergeException | IOException | NoSuchProjectException e) {
- log.error(String.format(
- "Cannot merge test change %d", change.getId().get()), e);
+ commit = ObjectId.fromString(ps.getRevision().get());
+ } catch (IllegalArgumentException e) {
+ log.error("Invalid revision on patch set " + ps);
return false;
- } finally {
- rw.release();
}
- }
-
- private static Set<RevCommit> alreadyAccepted(RevWalk rw, Collection<Ref> refs)
- throws MissingObjectException, IOException {
- Set<RevCommit> accepted = Sets.newHashSet();
- for (Ref r : refs) {
- if (r.getName().startsWith(Constants.R_HEADS)
- || r.getName().startsWith(Constants.R_TAGS)) {
- try {
- accepted.add(rw.parseCommit(r.getObjectId()));
- } catch (IncorrectObjectTypeException nonCommit) {
- // Not a commit? Skip over it.
- }
- }
- }
- return accepted;
- }
-
- private static CodeReviewCommit parse(RevWalk rw, ObjectId id)
- throws MissingObjectException, IncorrectObjectTypeException, IOException {
- return (CodeReviewCommit) rw.parseCommit(id);
+ return cache.get(
+ commit, ref, type, strategy, change.getDest(), git, db.get());
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java
index 8e06229..e22c45e 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -71,6 +71,7 @@
import com.google.gerrit.server.avatar.AvatarProvider;
import com.google.gerrit.server.cache.CacheRemovalListener;
import com.google.gerrit.server.change.ChangeKindCacheImpl;
+import com.google.gerrit.server.change.MergeabilityCache;
import com.google.gerrit.server.change.MergeabilityChecker;
import com.google.gerrit.server.events.EventFactory;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
@@ -166,6 +167,7 @@
install(ConflictsCacheImpl.module());
install(GroupCacheImpl.module());
install(GroupIncludeCacheImpl.module());
+ install(MergeabilityCache.module());
install(PatchListCacheImpl.module());
install(ProjectCacheImpl.module());
install(SectionSortCache.module());
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java
index 5824c6f..9eb9945 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java
@@ -634,8 +634,11 @@
public ThreeWayMerger newThreeWayMerger(final Repository repo,
final ObjectInserter inserter) {
- return newThreeWayMerger(repo, inserter,
- mergeStrategyName(useContentMerge, useRecursiveMerge));
+ return newThreeWayMerger(repo, inserter, mergeStrategyName());
+ }
+
+ public String mergeStrategyName() {
+ return mergeStrategyName(useContentMerge, useRecursiveMerge);
}
public static String mergeStrategyName(boolean useContentMerge,
