From b50fba72b27accaeb256f95bcfdae03dfa6befd5 Mon Sep 17 00:00:00 2001
From: Dave Borowitz <dborowitz@google.com>
Date: Thu, 10 Sep 2015 12:10:36 -0400
Subject: [PATCH] CherryPick: Use a single BatchUpdate

Add all ops to the same update, so database updates and ref updates
each happen in a single batch. In theory this means we could update
multiple new patch set refs in the same BatchRefUpdate, although in
practice we don't, since we only ever call this strategy with a single
change, and the actual ref update is done elsewhere in MergeOp.
However, hoisting the BatchUpdate outside the loop will make it easier
to eventually pass in a shared BatchUpdate from outside.

Change-Id: Ifaedc86c7c47957ea314a186d07a2b2b3a9d87bb
---
 .../server/git/strategy/CherryPick.java       | 20 +++++++++----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/CherryPick.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/CherryPick.java
index 2a3a6c2e38..fec70f08b1 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/CherryPick.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/CherryPick.java
@@ -45,7 +45,6 @@ import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.transport.ReceiveCommand;
 
 import java.io.IOException;
-import java.sql.Timestamp;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -69,13 +68,11 @@ public class CherryPick extends SubmitStrategy {
       Collection<CodeReviewCommit> toMerge) throws MergeException {
     MergeTip mergeTip = new MergeTip(branchTip, toMerge);
     List<CodeReviewCommit> sorted = CodeReviewCommit.ORDER.sortedCopy(toMerge);
-    while (!sorted.isEmpty()) {
-      CodeReviewCommit n = sorted.remove(0);
-      Timestamp now = TimeUtil.nowTs();
-      try (BatchUpdate u = args.newBatchUpdate(now)) {
-        // TODO(dborowitz): This won't work when mergeTip is updated only at the
-        // end of the batch.
-        if (mergeTip.getCurrentTip() == null) {
+    boolean first = true;
+    try (BatchUpdate u = args.newBatchUpdate(TimeUtil.nowTs())) {
+      while (!sorted.isEmpty()) {
+        CodeReviewCommit n = sorted.remove(0);
+        if (first && branchTip == null) {
           u.addOp(n.getControl(), new CherryPickUnbornRootOp(mergeTip, n));
         } else if (n.getParentCount() == 0) {
           u.addOp(n.getControl(), new CherryPickRootOp(n));
@@ -84,10 +81,11 @@ public class CherryPick extends SubmitStrategy {
         } else {
           u.addOp(n.getControl(), new CherryPickMultipleParentsOp(mergeTip, n));
         }
-        u.execute();
-      } catch (UpdateException | RestApiException e) {
-        throw new MergeException("Cannot merge " + n.name(), e);
+        first = false;
       }
+      u.execute();
+    } catch (UpdateException | RestApiException e) {
+      throw new MergeException("Cannot cherry-pick onto " + args.destBranch);
     }
     // TODO(dborowitz): When BatchUpdate is hoisted out of CherryPick,
     // SubmitStrategy should probably no longer return MergeTip, instead just