Make "conflicts" cache reentrant

Move conflicts cache population logic to cache loader to make sure that
only one thread at a time computes the cache entry value. This prevents
the issue with high resources utilisation when multiple threads compute
the same cache entries for a change with large number of potential
conflicts.

Bug: Issue 14726
Change-Id: Ide3f3256ca627274fa89aaa4a9cda35bdb3b81a3

3 files changed, 70 insertions, 42 deletions

diff --git a/java/com/google/gerrit/server/query/change/ConflictsCache.java b/java/com/google/gerrit/server/query/change/ConflictsCache.java
index c7ee79b9db..1e7ba9397a 100644
--- a/java/com/google/gerrit/server/query/change/ConflictsCache.java
+++ b/java/com/google/gerrit/server/query/change/ConflictsCache.java
@@ -14,12 +14,12 @@
 
 package com.google.gerrit.server.query.change;
 
-import com.google.gerrit.common.Nullable;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutionException;
 
 public interface ConflictsCache {
 
   void put(ConflictKey key, boolean value);
 
-  @Nullable
-  Boolean getIfPresent(ConflictKey key);
+  Boolean get(ConflictKey key, Callable<? extends Boolean> loader) throws ExecutionException;
 }
diff --git a/java/com/google/gerrit/server/query/change/ConflictsCacheImpl.java b/java/com/google/gerrit/server/query/change/ConflictsCacheImpl.java
index 426c5d6b89..4926314e1f 100644
--- a/java/com/google/gerrit/server/query/change/ConflictsCacheImpl.java
+++ b/java/com/google/gerrit/server/query/change/ConflictsCacheImpl.java
@@ -21,6 +21,8 @@ import com.google.inject.Inject;
 import com.google.inject.Module;
 import com.google.inject.Singleton;
 import com.google.inject.name.Named;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutionException;
 
 @Singleton
 public class ConflictsCacheImpl implements ConflictsCache {
@@ -53,7 +55,8 @@ public class ConflictsCacheImpl implements ConflictsCache {
   }
 
   @Override
-  public Boolean getIfPresent(ConflictKey key) {
-    return conflictsCache.getIfPresent(key);
+  public Boolean get(ConflictKey key, Callable<? extends Boolean> loader)
+      throws ExecutionException {
+    return conflictsCache.get(key, loader);
   }
 }
diff --git a/java/com/google/gerrit/server/query/change/ConflictsPredicate.java b/java/com/google/gerrit/server/query/change/ConflictsPredicate.java
index 16f85b191d..f4af4ca02f 100644
--- a/java/com/google/gerrit/server/query/change/ConflictsPredicate.java
+++ b/java/com/google/gerrit/server/query/change/ConflictsPredicate.java
@@ -20,6 +20,7 @@ import static com.google.gerrit.server.project.ProjectCache.noSuchProject;
 import static java.util.concurrent.TimeUnit.MINUTES;
 
 import com.google.common.flogger.FluentLogger;
+import com.google.common.util.concurrent.UncheckedExecutionException;
 import com.google.gerrit.entities.BooleanProjectConfig;
 import com.google.gerrit.entities.BranchNameKey;
 import com.google.gerrit.entities.Change;
@@ -30,7 +31,6 @@ import com.google.gerrit.index.query.PostFilterPredicate;
 import com.google.gerrit.index.query.Predicate;
 import com.google.gerrit.index.query.QueryParseException;
 import com.google.gerrit.server.git.CodeReviewCommit;
-import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk;
 import com.google.gerrit.server.project.NoSuchProjectException;
 import com.google.gerrit.server.project.ProjectCache;
 import com.google.gerrit.server.project.ProjectState;
@@ -41,6 +41,8 @@ import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutionException;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.revwalk.RevCommit;
@@ -142,27 +144,8 @@ public class ConflictsPredicate {
                 other,
                 str.type,
                 projectState.is(BooleanProjectConfig.USE_CONTENT_MERGE));
-        Boolean maybeConflicts = args.conflictsCache.getIfPresent(conflictsKey);
-        if (maybeConflicts != null) {
-          return maybeConflicts;
-        }
-
-        try (Repository repo = args.repoManager.openRepository(otherChange.getProject());
-            CodeReviewRevWalk rw = CodeReviewCommit.newRevWalk(repo)) {
-          boolean conflicts =
-              !args.submitDryRun.run(
-                  null,
-                  str.type,
-                  repo,
-                  rw,
-                  otherChange.getDest(),
-                  changeDataCache.getTestAgainst(),
-                  other,
-                  getAlreadyAccepted(repo, rw));
-          args.conflictsCache.put(conflictsKey, conflicts);
-          return conflicts;
-        }
-      } catch (NoSuchProjectException | StorageException | IOException e) {
+        return args.conflictsCache.get(conflictsKey, new Loader(object, changeDataCache, args));
+      } catch (StorageException | ExecutionException | UncheckedExecutionException e) {
         ObjectId finalOther = other;
         warnWithOccasionalStackTrace(
             e,
@@ -179,23 +162,9 @@ public class ConflictsPredicate {
     public int getCost() {
       return 5;
     }
-
-    private Set<RevCommit> getAlreadyAccepted(Repository repo, RevWalk rw) {
-      try {
-        Set<RevCommit> accepted = new HashSet<>();
-        SubmitDryRun.addCommits(changeDataCache.getAlreadyAccepted(repo), rw, accepted);
-        ObjectId tip = changeDataCache.getTestAgainst();
-        if (tip != null) {
-          accepted.add(rw.parseCommit(tip));
-        }
-        return accepted;
-      } catch (StorageException | IOException e) {
-        throw new StorageException("Failed to determine already accepted commits.", e);
-      }
-    }
   }
 
-  private static class ChangeDataCache {
+  static class ChangeDataCache {
     private final ChangeData cd;
     private final ProjectCache projectCache;
 
@@ -238,4 +207,60 @@ public class ConflictsPredicate {
         .atMostEvery(1, MINUTES)
         .logVarargs("(Re-logging with stack trace) " + format, args);
   }
+
+  private static class Loader implements Callable<Boolean> {
+    private final ChangeData changeData;
+    private final ConflictsPredicate.ChangeDataCache changeDataCache;
+    private final ChangeQueryBuilder.Arguments args;
+
+    private Loader(
+        ChangeData changeData,
+        ConflictsPredicate.ChangeDataCache changeDataCache,
+        ChangeQueryBuilder.Arguments args) {
+      this.changeData = changeData;
+      this.changeDataCache = changeDataCache;
+      this.args = args;
+    }
+
+    @Override
+    public Boolean call() throws Exception {
+      Change otherChange = changeData.change();
+      ObjectId other = changeData.currentPatchSet().commitId();
+      try (Repository repo = args.repoManager.openRepository(otherChange.getProject());
+          CodeReviewCommit.CodeReviewRevWalk rw = CodeReviewCommit.newRevWalk(repo)) {
+        return !args.submitDryRun.run(
+            null,
+            changeData.submitTypeRecord().type,
+            repo,
+            rw,
+            otherChange.getDest(),
+            changeDataCache.getTestAgainst(),
+            other,
+            getAlreadyAccepted(repo, rw));
+      } catch (NoSuchProjectException | IOException e) {
+        warnWithOccasionalStackTrace(
+            e,
+            "Failure when loading conflicts of change %s in %s (%s): %s",
+            lazy(changeData::getId),
+            lazy(() -> firstNonNull(otherChange.getProject(), "unknown project")),
+            lazy(() -> other != null ? other.name() : "unknown commit"),
+            e.getMessage());
+        return false;
+      }
+    }
+
+    private Set<RevCommit> getAlreadyAccepted(Repository repo, RevWalk rw) {
+      try {
+        Set<RevCommit> accepted = new HashSet<>();
+        SubmitDryRun.addCommits(changeDataCache.getAlreadyAccepted(repo), rw, accepted);
+        ObjectId tip = changeDataCache.getTestAgainst();
+        if (tip != null) {
+          accepted.add(rw.parseCommit(tip));
+        }
+        return accepted;
+      } catch (StorageException | IOException e) {
+        throw new StorageException("Failed to determine already accepted commits.", e);
+      }
+    }
+  }
 }