Remove published draft comments asynchronously

NoteDb operations on the draft comments storage are slower than
operations on change refs. When adding, modifying or deleting single
draft comments, the UI papers over this by performing those actions in
the background. It presents a notification once done.

When posting a review, we do two operations that are inheritanty
non-atomic because they touch different repositories: Update the change
metadata to post new comments and delete draft comments from All-Users.

Since these are non-atomic, existing Gerrit logic already dedupes
zombie draft comments. The fixup logic for a change can remove them.

This commit adapts NoteDbUpdateManager to perform draft comment
deletions asynchronously when we are publishing comments. If the update
is anything else, we perform the update synchronously.

Change-Id: I85f4e9580460a0313374045ec7b64950da72d5f7

java/com/google/gerrit/server/PatchSetUtil.java

@@ -108,7 +108,7 @@ public class PatchSetUtil {
   }
 
   private static void ensurePatchSetMatches(PatchSet.Id psId, ChangeUpdate update) {
-    Change.Id changeId = update.getChange().getId();
+    Change.Id changeId = update.getId();
     checkArgument(
         psId.changeId().equals(changeId),
         "cannot modify patch set %s on update for change %s",

java/com/google/gerrit/server/notedb/AllUsersAsyncUpdate.java

@@ -0,0 +1,116 @@
+// Copyright (C) 2019 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.notedb;
+
+import static com.google.common.base.MoreObjects.firstNonNull;
+import static com.google.common.base.Preconditions.checkState;
+
+import com.google.common.collect.Iterables;
+import com.google.common.collect.ListMultimap;
+import com.google.common.collect.MultimapBuilder;
+import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.git.RefUpdateUtil;
+import com.google.gerrit.server.FanOutExecutor;
+import com.google.gerrit.server.config.AllUsersName;
+import com.google.gerrit.server.git.GitRepositoryManager;
+import java.io.IOException;
+import java.util.Map;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Future;
+import javax.inject.Inject;
+import org.eclipse.jgit.lib.BatchRefUpdate;
+import org.eclipse.jgit.lib.PersonIdent;
+import org.eclipse.jgit.transport.PushCertificate;
+
+/**
+ * Performs an update on {@code All-Users} asynchronously if required. No-op in case no updates were
+ * scheduled for asynchronous execution.
+ */
+public class AllUsersAsyncUpdate {
+  private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
+  private final ExecutorService executor;
+  private final AllUsersName allUsersName;
+  private final GitRepositoryManager repoManager;
+  private final ListMultimap<String, ChangeDraftUpdate> draftUpdates;
+
+  private PersonIdent serverIdent;
+
+  @Inject
+  AllUsersAsyncUpdate(
+      @FanOutExecutor ExecutorService executor,
+      AllUsersName allUsersName,
+      GitRepositoryManager repoManager) {
+    this.executor = executor;
+    this.allUsersName = allUsersName;
+    this.repoManager = repoManager;
+    this.draftUpdates = MultimapBuilder.hashKeys().arrayListValues().build();
+  }
+
+  void setDraftUpdates(ListMultimap<String, ChangeDraftUpdate> draftUpdates) {
+    checkState(isEmpty(), "attempted to set draft comment updates for async execution twice");
+    boolean allPublishOnly =
+        draftUpdates.values().stream().allMatch(ChangeDraftUpdate::canRunAsync);
+    checkState(allPublishOnly, "not all updates can be run asynchronously");
+    // Add deep copies to avoid any threading issues.
+    for (Map.Entry<String, ChangeDraftUpdate> entry : draftUpdates.entries()) {
+      this.draftUpdates.put(entry.getKey(), entry.getValue().copy());
+    }
+    if (draftUpdates.size() > 0) {
+      // Save the PersonIdent for later so that we get consistent time stamps in the commit and ref
+      // log.
+      serverIdent = Iterables.get(draftUpdates.entries(), 0).getValue().serverIdent;
+    }
+  }
+
+  /** Returns true if no operations should be performed on the repo. */
+  boolean isEmpty() {
+    return draftUpdates.isEmpty();
+  }
+
+  /** Executes repository update asynchronously. No-op in case no updates were scheduled. */
+  void execute(PersonIdent refLogIdent, String refLogMessage, PushCertificate pushCert) {
+    if (isEmpty()) {
+      return;
+    }
+
+    @SuppressWarnings("unused")
+    Future<?> possiblyIgnoredError =
+        executor.submit(
+            () -> {
+              try (OpenRepo allUsersRepo = OpenRepo.open(repoManager, allUsersName)) {
+                allUsersRepo.addUpdates(draftUpdates);
+                allUsersRepo.flush();
+                BatchRefUpdate bru = allUsersRepo.repo.getRefDatabase().newBatchUpdate();
+                bru.setPushCertificate(pushCert);
+                if (refLogMessage != null) {
+                  bru.setRefLogMessage(refLogMessage, false);
+                } else {
+                  bru.setRefLogMessage(
+                      firstNonNull(NoteDbUtil.guessRestApiHandler(), "Update NoteDb refs async"),
+                      false);
+                }
+                bru.setRefLogIdent(refLogIdent != null ? refLogIdent : serverIdent);
+                bru.setAtomic(true);
+                allUsersRepo.cmds.addTo(bru);
+                bru.setAllowNonFastForwards(true);
+                RefUpdateUtil.executeChecked(bru, allUsersRepo.rw);
+              } catch (IOException e) {
+                logger.atSevere().withCause(e).log(
+                    "Failed to delete draft comments asynchronously after publishing them");
+              }
+            });
+  }
+}

java/com/google/gerrit/server/notedb/ChangeDraftUpdate.java

@@ -15,6 +15,7 @@
 package com.google.gerrit.server.notedb;
 
 import static com.google.common.base.MoreObjects.firstNonNull;
+import static com.google.common.base.Preconditions.checkState;
 import static org.eclipse.jgit.lib.Constants.OBJ_BLOB;
 
 import com.google.auto.value.AutoValue;
@@ -34,7 +35,7 @@ import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Date;
-import java.util.HashSet;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -78,6 +79,12 @@ public class ChangeDraftUpdate extends AbstractChangeUpdate {
     abstract Comment.Key key();
   }
 
+  enum DeleteReason {
+    DELETED,
+    PUBLISHED,
+    FIXED
+  }
+
   private static Key key(Comment c) {
     return new AutoValue_ChangeDraftUpdate_Key(c.getCommitId(), c.key);
   }
@@ -85,7 +92,7 @@ public class ChangeDraftUpdate extends AbstractChangeUpdate {
   private final AllUsersName draftsProject;
 
   private List<Comment> put = new ArrayList<>();
-  private Set<Key> delete = new HashSet<>();
+  private Map<Key, DeleteReason> delete = new HashMap<>();
 
   @AssistedInject
   private ChangeDraftUpdate(
@@ -116,17 +123,69 @@ public class ChangeDraftUpdate extends AbstractChangeUpdate {
   }
 
   public void putComment(Comment c) {
+    checkState(!put.contains(c), "comment already added");
     verifyComment(c);
     put.add(c);
   }
 
-  public void deleteComment(Comment c) {
+  /**
+   * Marks a comment for deletion. Called when the comment is deleted because the user published it.
+   */
+  public void markCommentPublished(Comment c) {
+    checkState(!delete.containsKey(key(c)), "comment already marked for deletion");
     verifyComment(c);
-    delete.add(key(c));
+    delete.put(key(c), DeleteReason.PUBLISHED);
   }
 
+  /**
+   * Marks a comment for deletion. Called when the comment is deleted because the user removed it.
+   */
+  public void deleteComment(Comment c) {
+    checkState(!delete.containsKey(key(c)), "comment already marked for deletion");
+    verifyComment(c);
+    delete.put(key(c), DeleteReason.DELETED);
+  }
+
+  /**
+   * Marks a comment for deletion. Called when the comment should have been deleted previously, but
+   * wasn't, so we're fixing it up.
+   */
   public void deleteComment(ObjectId commitId, Comment.Key key) {
-    delete.add(new AutoValue_ChangeDraftUpdate_Key(commitId, key));
+    Key commentKey = new AutoValue_ChangeDraftUpdate_Key(commitId, key);
+    checkState(!delete.containsKey(commentKey), "comment already marked for deletion");
+    delete.put(commentKey, DeleteReason.FIXED);
+  }
+
+  /**
+   * Returns true if all we do in this operations is deletes caused by publishing or fixing up
+   * comments.
+   */
+  public boolean canRunAsync() {
+    return put.isEmpty()
+        && delete.values().stream()
+            .allMatch(r -> r == DeleteReason.PUBLISHED || r == DeleteReason.FIXED);
+  }
+
+  /**
+   * Returns a copy of the current {@link ChangeDraftUpdate} that contains references to all
+   * deletions. Copying of {@link ChangeDraftUpdate} is only allowed if it contains no new comments.
+   */
+  ChangeDraftUpdate copy() {
+    checkState(
+        put.isEmpty(),
+        "copying ChangeDraftUpdate is allowed only if it doesn't contain new comments");
+    ChangeDraftUpdate clonedUpdate =
+        new ChangeDraftUpdate(
+            authorIdent,
+            draftsProject,
+            noteUtil,
+            new Change(getChange()),
+            accountId,
+            realAccountId,
+            authorIdent,
+            when);
+    clonedUpdate.delete.putAll(delete);
+    return clonedUpdate;
   }
 
   private CommitBuilder storeCommentsInNotes(
@@ -137,11 +196,11 @@ public class ChangeDraftUpdate extends AbstractChangeUpdate {
     RevisionNoteBuilder.Cache cache = new RevisionNoteBuilder.Cache(rnm);
 
     for (Comment c : put) {
-      if (!delete.contains(key(c))) {
+      if (!delete.keySet().contains(key(c))) {
         cache.get(c.getCommitId()).putComment(c);
       }
     }
-    for (Key k : delete) {
+    for (Key k : delete.keySet()) {
       cache.get(k.commitId()).deleteComment(k.key());
     }
 

java/com/google/gerrit/server/notedb/ChangeUpdate.java

@@ -280,11 +280,7 @@ public class ChangeUpdate extends AbstractChangeUpdate {
       draftUpdate.putComment(c);
     } else {
       comments.add(c);
-      // Always delete the corresponding comment from drafts. Published comments
+      draftUpdate.markCommentPublished(c);
-      // are immutable, meaning in normal operation we only hit this path when
-      // publishing a comment. It's exactly in that case that we have to delete
-      // the draft.
-      draftUpdate.deleteComment(c);
     }
   }
 
@@ -414,7 +410,7 @@ public class ChangeUpdate extends AbstractChangeUpdate {
   }
 
   public void setRevertOf(int revertOf) {
-    int ownId = getChange().getId().get();
+    int ownId = getId().get();
     checkArgument(ownId != revertOf, "A change cannot revert itself");
     this.revertOf = revertOf;
     rootOnly = true;

java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java

@@ -18,16 +18,12 @@ import static com.google.common.base.MoreObjects.firstNonNull;
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkState;
 import static com.google.gerrit.server.notedb.NoteDbTable.CHANGES;
-import static java.util.Objects.requireNonNull;
 
-import com.google.common.annotations.VisibleForTesting;
-import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.ListMultimap;
 import com.google.common.collect.MultimapBuilder;
+import com.google.common.flogger.FluentLogger;
 import com.google.gerrit.common.Nullable;
 import com.google.gerrit.exceptions.StorageException;
-import com.google.gerrit.extensions.restapi.RestModifyView;
 import com.google.gerrit.git.RefUpdateUtil;
 import com.google.gerrit.metrics.Timer1;
 import com.google.gerrit.reviewdb.client.Change;
@@ -37,10 +33,7 @@ import com.google.gerrit.server.GerritPersonIdent;
 import com.google.gerrit.server.config.AllUsersName;
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.git.GitRepositoryManager;
-import com.google.gerrit.server.git.InMemoryInserter;
-import com.google.gerrit.server.git.InsertedObject;
 import com.google.gerrit.server.update.ChainedReceiveCommands;
-import com.google.gerrit.server.update.RetryingRestModifyView;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 import com.google.inject.assistedinject.Assisted;
@@ -48,7 +41,6 @@ import java.io.IOException;
 import java.util.Collection;
 import java.util.HashSet;
 import java.util.Map;
-import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
 import org.eclipse.jgit.errors.ConfigInvalidException;
@@ -56,7 +48,6 @@ import org.eclipse.jgit.lib.BatchRefUpdate;
 import org.eclipse.jgit.lib.Config;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.ObjectInserter;
-import org.eclipse.jgit.lib.ObjectReader;
 import org.eclipse.jgit.lib.PersonIdent;
 import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.lib.Repository;
@@ -74,98 +65,12 @@ import org.eclipse.jgit.transport.ReceiveCommand;
  * {@link #stage()}.
  */
 public class NoteDbUpdateManager implements AutoCloseable {
-  private static final ImmutableList<String> PACKAGE_PREFIXES =
+  private static final FluentLogger logger = FluentLogger.forEnclosingClass();
-      ImmutableList.of("com.google.gerrit.server.", "com.google.gerrit.");
-  private static final ImmutableSet<String> SERVLET_NAMES =
-      ImmutableSet.of(
-          "com.google.gerrit.httpd.restapi.RestApiServlet", RetryingRestModifyView.class.getName());
 
   public interface Factory {
     NoteDbUpdateManager create(Project.NameKey projectName);
   }
 
-  public static class TooManyUpdatesException extends StorageException {
-    @VisibleForTesting
-    public static String message(Change.Id id, int maxUpdates) {
-      return "Change "
-          + id
-          + " may not exceed "
-          + maxUpdates
-          + " updates. It may still be abandoned or submitted. To continue working on this "
-          + "change, recreate it with a new Change-Id, then abandon this one.";
-    }
-
-    private static final long serialVersionUID = 1L;
-
-    private TooManyUpdatesException(Change.Id id, int maxUpdates) {
-      super(message(id, maxUpdates));
-    }
-  }
-
-  public static class OpenRepo implements AutoCloseable {
-    public final Repository repo;
-    public final RevWalk rw;
-    public final ChainedReceiveCommands cmds;
-
-    private final InMemoryInserter inMemIns;
-    private final ObjectInserter tempIns;
-    @Nullable private final ObjectInserter finalIns;
-
-    private final boolean close;
-
-    private OpenRepo(
-        Repository repo,
-        RevWalk rw,
-        @Nullable ObjectInserter ins,
-        ChainedReceiveCommands cmds,
-        boolean close) {
-      ObjectReader reader = rw.getObjectReader();
-      checkArgument(
-          ins == null || reader.getCreatedFromInserter() == ins,
-          "expected reader to be created from %s, but was %s",
-          ins,
-          reader.getCreatedFromInserter());
-      this.repo = requireNonNull(repo);
-
-      this.inMemIns = new InMemoryInserter(rw.getObjectReader());
-      this.tempIns = inMemIns;
-
-      this.rw = new RevWalk(tempIns.newReader());
-      this.finalIns = ins;
-      this.cmds = requireNonNull(cmds);
-      this.close = close;
-    }
-
-    public Optional<ObjectId> getObjectId(String refName) throws IOException {
-      return cmds.get(refName);
-    }
-
-    void flush() throws IOException {
-      flushToFinalInserter();
-      finalIns.flush();
-    }
-
-    void flushToFinalInserter() throws IOException {
-      checkState(finalIns != null);
-      for (InsertedObject obj : inMemIns.getInsertedObjects()) {
-        finalIns.insert(obj.type(), obj.data().toByteArray());
-      }
-      inMemIns.clear();
-    }
-
-    @Override
-    public void close() {
-      rw.getObjectReader().close();
-      rw.close();
-      if (close) {
-        if (finalIns != null) {
-          finalIns.close();
-        }
-        repo.close();
-      }
-    }
-  }
-
   private final Provider<PersonIdent> serverIdent;
   private final GitRepositoryManager repoManager;
   private final AllUsersName allUsersName;
@@ -180,6 +85,7 @@ public class NoteDbUpdateManager implements AutoCloseable {
 
   private OpenRepo changeRepo;
   private OpenRepo allUsersRepo;
+  private AllUsersAsyncUpdate updateAllUsersAsync;
   private boolean executed;
   private String refLogMessage;
   private PersonIdent refLogIdent;
@@ -192,11 +98,13 @@ public class NoteDbUpdateManager implements AutoCloseable {
       GitRepositoryManager repoManager,
       AllUsersName allUsersName,
       NoteDbMetrics metrics,
+      AllUsersAsyncUpdate updateAllUsersAsync,
       @Assisted Project.NameKey projectName) {
     this.serverIdent = serverIdent;
     this.repoManager = repoManager;
     this.allUsersName = allUsersName;
     this.metrics = metrics;
+    this.updateAllUsersAsync = updateAllUsersAsync;
     this.projectName = projectName;
     maxUpdates = cfg.getInt("change", null, "maxUpdates", 1000);
     changeUpdates = MultimapBuilder.hashKeys().arrayListValues().build();
@@ -261,28 +169,13 @@ public class NoteDbUpdateManager implements AutoCloseable {
 
   private void initChangeRepo() throws IOException {
     if (changeRepo == null) {
-      changeRepo = openRepo(projectName);
+      changeRepo = OpenRepo.open(repoManager, projectName);
     }
   }
 
   private void initAllUsersRepo() throws IOException {
     if (allUsersRepo == null) {
-      allUsersRepo = openRepo(allUsersName);
+      allUsersRepo = OpenRepo.open(repoManager, allUsersName);
-    }
-  }
-
-  private OpenRepo openRepo(Project.NameKey p) throws IOException {
-    Repository repo = repoManager.openRepository(p); // Closed by OpenRepo#close.
-    ObjectInserter ins = repo.newObjectInserter(); // Closed by OpenRepo#close.
-    ObjectReader reader = ins.newReader(); // Not closed by OpenRepo#close.
-    try (RevWalk rw = new RevWalk(reader)) { // Doesn't escape OpenRepo constructor.
-      return new OpenRepo(repo, rw, ins, new ChainedReceiveCommands(repo), true) {
-        @Override
-        public void close() {
-          reader.close();
-          super.close();
-        }
-      };
     }
   }
 
@@ -293,7 +186,8 @@ public class NoteDbUpdateManager implements AutoCloseable {
         && rewriters.isEmpty()
         && toDelete.isEmpty()
         && !hasCommands(changeRepo)
-        && !hasCommands(allUsersRepo);
+        && !hasCommands(allUsersRepo)
+        && updateAllUsersAsync.isEmpty();
   }
 
   private static boolean hasCommands(@Nullable OpenRepo or) {
@@ -423,6 +317,13 @@ public class NoteDbUpdateManager implements AutoCloseable {
       // comments can only go from DRAFT to PUBLISHED, not vice versa.
       BatchRefUpdate result = execute(changeRepo, dryrun, pushCert);
       execute(allUsersRepo, dryrun, null);
+      if (!dryrun) {
+        // Only execute the asynchronous operation if we are not in dry-run mode: The dry run would
+        // have to run synchronous to be of any value at all. For the removal of draft comments from
+        // All-Users we don't care much of the operation succeeds, so we are skipping the dry run
+        // altogether.
+        updateAllUsersAsync.execute(refLogIdent, refLogMessage, pushCert);
+      }
       executed = true;
       return result;
     } finally {
@@ -448,7 +349,8 @@ public class NoteDbUpdateManager implements AutoCloseable {
     if (refLogMessage != null) {
       bru.setRefLogMessage(refLogMessage, false);
     } else {
-      bru.setRefLogMessage(firstNonNull(guessRestApiHandler(), "Update NoteDb refs"), false);
+      bru.setRefLogMessage(
+          firstNonNull(NoteDbUtil.guessRestApiHandler(), "Update NoteDb refs"), false);
     }
     bru.setRefLogIdent(refLogIdent != null ? refLogIdent : serverIdent.get());
     bru.setAtomic(true);
@@ -461,59 +363,18 @@ public class NoteDbUpdateManager implements AutoCloseable {
     return bru;
   }
 
-  private static String guessRestApiHandler() {
-    StackTraceElement[] trace = Thread.currentThread().getStackTrace();
-    int i = findRestApiServlet(trace);
-    if (i < 0) {
-      return null;
-    }
-    try {
-      for (i--; i >= 0; i--) {
-        String cn = trace[i].getClassName();
-        Class<?> cls = Class.forName(cn);
-        if (RestModifyView.class.isAssignableFrom(cls) && cls != RetryingRestModifyView.class) {
-          return viewName(cn);
-        }
-      }
-      return null;
-    } catch (ClassNotFoundException e) {
-      return null;
-    }
-  }
-
-  private static String viewName(String cn) {
-    String impl = cn.replace('$', '.');
-    for (String p : PACKAGE_PREFIXES) {
-      if (impl.startsWith(p)) {
-        return impl.substring(p.length());
-      }
-    }
-    return impl;
-  }
-
-  private static int findRestApiServlet(StackTraceElement[] trace) {
-    for (int i = 0; i < trace.length; i++) {
-      if (SERVLET_NAMES.contains(trace[i].getClassName())) {
-        return i;
-      }
-    }
-    return -1;
-  }
-
   private void addCommands() throws IOException {
-    if (isEmpty()) {
+    changeRepo.addUpdates(changeUpdates, Optional.of(maxUpdates));
-      return;
-    }
-    checkState(changeRepo != null, "must set change repo");
     if (!draftUpdates.isEmpty()) {
-      checkState(allUsersRepo != null, "must set all users repo");
+      boolean publishOnly = draftUpdates.values().stream().allMatch(ChangeDraftUpdate::canRunAsync);
-    }
+      if (publishOnly) {
-    addUpdates(changeUpdates, changeRepo, Optional.of(maxUpdates));
+        updateAllUsersAsync.setDraftUpdates(draftUpdates);
-    if (!draftUpdates.isEmpty()) {
+      } else {
-      addUpdates(draftUpdates, allUsersRepo, Optional.empty());
+        allUsersRepo.addUpdates(draftUpdates);
+      }
     }
     if (!robotCommentUpdates.isEmpty()) {
-      addUpdates(robotCommentUpdates, changeRepo, Optional.empty());
+      changeRepo.addUpdates(robotCommentUpdates);
     }
     if (!rewriters.isEmpty()) {
       addRewrites(rewriters, changeRepo);
@@ -545,51 +406,6 @@ public class NoteDbUpdateManager implements AutoCloseable {
     checkState(!executed, "update has already been executed");
   }
 
-  private static <U extends AbstractChangeUpdate> void addUpdates(
-      ListMultimap<String, U> all, OpenRepo or, Optional<Integer> maxUpdates) throws IOException {
-    for (Map.Entry<String, Collection<U>> e : all.asMap().entrySet()) {
-      String refName = e.getKey();
-      Collection<U> updates = e.getValue();
-      ObjectId old = or.cmds.get(refName).orElse(ObjectId.zeroId());
-      // Only actually write to the ref if one of the updates explicitly allows
-      // us to do so, i.e. it is known to represent a new change. This avoids
-      // writing partial change meta if the change hasn't been backfilled yet.
-      if (!allowWrite(updates, old)) {
-        continue;
-      }
-
-      int updateCount;
-      U first = updates.iterator().next();
-      if (maxUpdates.isPresent()) {
-        checkState(first.getNotes() != null, "expected ChangeNotes on %s", first);
-        updateCount = first.getNotes().getUpdateCount();
-      } else {
-        updateCount = 0;
-      }
-
-      ObjectId curr = old;
-      for (U u : updates) {
-        if (u.isRootOnly() && !old.equals(ObjectId.zeroId())) {
-          throw new StorageException("Given ChangeUpdate is only allowed on initial commit");
-        }
-        ObjectId next = u.apply(or.rw, or.tempIns, curr);
-        if (next == null) {
-          continue;
-        }
-        if (maxUpdates.isPresent()
-            && !Objects.equals(next, curr)
-            && ++updateCount > maxUpdates.get()
-            && !u.bypassMaxUpdates()) {
-          throw new TooManyUpdatesException(u.getId(), maxUpdates.get());
-        }
-        curr = next;
-      }
-      if (!old.equals(curr)) {
-        or.cmds.add(new ReceiveCommand(old, curr, refName));
-      }
-    }
-  }
-
   private static void addRewrites(ListMultimap<String, NoteDbRewriter> rewriters, OpenRepo openRepo)
       throws IOException {
     for (Map.Entry<String, Collection<NoteDbRewriter>> entry : rewriters.asMap().entrySet()) {
@@ -618,12 +434,4 @@ public class NoteDbUpdateManager implements AutoCloseable {
       }
     }
   }
-
-  private static <U extends AbstractChangeUpdate> boolean allowWrite(
-      Collection<U> updates, ObjectId old) {
-    if (!old.equals(ObjectId.zeroId())) {
-      return true;
-    }
-    return updates.iterator().next().allowWriteToNewRef();
-  }
 }

java/com/google/gerrit/server/notedb/NoteDbUtil.java

@@ -15,8 +15,12 @@
 package com.google.gerrit.server.notedb;
 
 import com.google.common.base.CharMatcher;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.primitives.Ints;
+import com.google.gerrit.extensions.restapi.RestModifyView;
 import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.server.update.RetryingRestModifyView;
 import java.sql.Timestamp;
 import java.util.Optional;
 import org.eclipse.jgit.lib.PersonIdent;
@@ -25,6 +29,14 @@ import org.eclipse.jgit.util.GitDateFormatter.Format;
 
 public class NoteDbUtil {
 
+  private static final CharMatcher INVALID_FOOTER_CHARS = CharMatcher.anyOf("\r\n\0");
+
+  private static final ImmutableList<String> PACKAGE_PREFIXES =
+      ImmutableList.of("com.google.gerrit.server.", "com.google.gerrit.");
+  private static final ImmutableSet<String> SERVLET_NAMES =
+      ImmutableSet.of(
+          "com.google.gerrit.httpd.restapi.RestApiServlet", RetryingRestModifyView.class.getName());
+
   /**
    * Returns an AccountId for the given email address. Returns empty if the address isn't on this
    * server.
@@ -44,8 +56,6 @@ public class NoteDbUtil {
     return Optional.empty();
   }
 
-  private NoteDbUtil() {}
-
   public static String formatTime(PersonIdent ident, Timestamp t) {
     GitDateFormatter dateFormatter = new GitDateFormatter(Format.DEFAULT);
     // TODO(dborowitz): Use a ThreadLocal or use Joda.
@@ -53,7 +63,29 @@ public class NoteDbUtil {
     return dateFormatter.formatDate(newIdent);
   }
 
-  private static final CharMatcher INVALID_FOOTER_CHARS = CharMatcher.anyOf("\r\n\0");
+  /**
+   * Returns the name of the REST API handler that is in the stack trace of the caller of this
+   * method.
+   */
+  static String guessRestApiHandler() {
+    StackTraceElement[] trace = Thread.currentThread().getStackTrace();
+    int i = findRestApiServlet(trace);
+    if (i < 0) {
+      return null;
+    }
+    try {
+      for (i--; i >= 0; i--) {
+        String cn = trace[i].getClassName();
+        Class<?> cls = Class.forName(cn);
+        if (RestModifyView.class.isAssignableFrom(cls) && cls != RetryingRestModifyView.class) {
+          return viewName(cn);
+        }
+      }
+      return null;
+    } catch (ClassNotFoundException e) {
+      return null;
+    }
+  }
 
   static String sanitizeFooter(String value) {
     // Remove characters that would confuse JGit's footer parser if they were
@@ -65,4 +97,25 @@ public class NoteDbUtil {
     // empty paragraph for the purposes of footer parsing.
     return INVALID_FOOTER_CHARS.trimAndCollapseFrom(value, ' ');
   }
+
+  private static int findRestApiServlet(StackTraceElement[] trace) {
+    for (int i = 0; i < trace.length; i++) {
+      if (SERVLET_NAMES.contains(trace[i].getClassName())) {
+        return i;
+      }
+    }
+    return -1;
+  }
+
+  private static String viewName(String cn) {
+    String impl = cn.replace('$', '.');
+    for (String p : PACKAGE_PREFIXES) {
+      if (impl.startsWith(p)) {
+        return impl.substring(p.length());
+      }
+    }
+    return impl;
+  }
+
+  private NoteDbUtil() {}
 }

java/com/google/gerrit/server/notedb/OpenRepo.java

@@ -0,0 +1,177 @@
+// Copyright (C) 2019 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.notedb;
+
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkState;
+import static java.util.Objects.requireNonNull;
+
+import com.google.common.collect.ListMultimap;
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.exceptions.StorageException;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.git.InMemoryInserter;
+import com.google.gerrit.server.git.InsertedObject;
+import com.google.gerrit.server.update.ChainedReceiveCommands;
+import java.io.IOException;
+import java.util.Collection;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.ObjectInserter;
+import org.eclipse.jgit.lib.ObjectReader;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevWalk;
+import org.eclipse.jgit.transport.ReceiveCommand;
+
+/**
+ * Wrapper around {@link Repository} that keeps track of related {@link ObjectInserter}s and other
+ * objects that are jointly closed when invoking {@link #close}.
+ */
+class OpenRepo implements AutoCloseable {
+  /** Returns a {@link OpenRepo} wrapping around an open {@link Repository}. */
+  @SuppressWarnings("resource")
+  static OpenRepo open(GitRepositoryManager repoManager, Project.NameKey project)
+      throws IOException {
+    Repository repo = repoManager.openRepository(project); // Closed by OpenRepo#close.
+    ObjectInserter ins = repo.newObjectInserter(); // Closed by OpenRepo#close.
+    ObjectReader reader = ins.newReader(); // Not closed by OpenRepo#close.
+    try (RevWalk rw = new RevWalk(reader)) { // Doesn't escape OpenRepo constructor.
+      return new OpenRepo(repo, rw, ins, new ChainedReceiveCommands(repo), true) {
+        @Override
+        public void close() {
+          reader.close();
+          super.close();
+        }
+      };
+    }
+  }
+
+  final Repository repo;
+  final RevWalk rw;
+  final ChainedReceiveCommands cmds;
+  final ObjectInserter tempIns;
+
+  private final InMemoryInserter inMemIns;
+  @Nullable private final ObjectInserter finalIns;
+  private final boolean close;
+
+  OpenRepo(
+      Repository repo,
+      RevWalk rw,
+      @Nullable ObjectInserter ins,
+      ChainedReceiveCommands cmds,
+      boolean close) {
+    ObjectReader reader = rw.getObjectReader();
+    checkArgument(
+        ins == null || reader.getCreatedFromInserter() == ins,
+        "expected reader to be created from %s, but was %s",
+        ins,
+        reader.getCreatedFromInserter());
+    this.repo = requireNonNull(repo);
+
+    this.inMemIns = new InMemoryInserter(rw.getObjectReader());
+    this.tempIns = inMemIns;
+
+    this.rw = new RevWalk(tempIns.newReader());
+    this.finalIns = ins;
+    this.cmds = requireNonNull(cmds);
+    this.close = close;
+  }
+
+  @Override
+  public void close() {
+    rw.getObjectReader().close();
+    rw.close();
+    if (close) {
+      if (finalIns != null) {
+        finalIns.close();
+      }
+      repo.close();
+    }
+  }
+
+  void flush() throws IOException {
+    flushToFinalInserter();
+    finalIns.flush();
+  }
+
+  void flushToFinalInserter() throws IOException {
+    checkState(finalIns != null);
+    for (InsertedObject obj : inMemIns.getInsertedObjects()) {
+      finalIns.insert(obj.type(), obj.data().toByteArray());
+    }
+    inMemIns.clear();
+  }
+
+  private static <U extends AbstractChangeUpdate> boolean allowWrite(
+      Collection<U> updates, ObjectId old) {
+    if (!old.equals(ObjectId.zeroId())) {
+      return true;
+    }
+    return updates.iterator().next().allowWriteToNewRef();
+  }
+
+  <U extends AbstractChangeUpdate> void addUpdates(ListMultimap<String, U> all) throws IOException {
+    addUpdates(all, Optional.empty());
+  }
+
+  <U extends AbstractChangeUpdate> void addUpdates(
+      ListMultimap<String, U> all, Optional<Integer> maxUpdates) throws IOException {
+    for (Map.Entry<String, Collection<U>> e : all.asMap().entrySet()) {
+      String refName = e.getKey();
+      Collection<U> updates = e.getValue();
+      ObjectId old = cmds.get(refName).orElse(ObjectId.zeroId());
+      // Only actually write to the ref if one of the updates explicitly allows
+      // us to do so, i.e. it is known to represent a new change. This avoids
+      // writing partial change meta if the change hasn't been backfilled yet.
+      if (!allowWrite(updates, old)) {
+        continue;
+      }
+
+      int updateCount;
+      U first = updates.iterator().next();
+      if (maxUpdates.isPresent()) {
+        checkState(first.getNotes() != null, "expected ChangeNotes on %s", first);
+        updateCount = first.getNotes().getUpdateCount();
+      } else {
+        updateCount = 0;
+      }
+
+      ObjectId curr = old;
+      for (U u : updates) {
+        if (u.isRootOnly() && !old.equals(ObjectId.zeroId())) {
+          throw new StorageException("Given ChangeUpdate is only allowed on initial commit");
+        }
+        ObjectId next = u.apply(rw, tempIns, curr);
+        if (next == null) {
+          continue;
+        }
+        if (maxUpdates.isPresent()
+            && !Objects.equals(next, curr)
+            && ++updateCount > maxUpdates.get()
+            && !u.bypassMaxUpdates()) {
+          throw new TooManyUpdatesException(u.getId(), maxUpdates.get());
+        }
+        curr = next;
+      }
+      if (!old.equals(curr)) {
+        cmds.add(new ReceiveCommand(old, curr, refName));
+      }
+    }
+  }
+}

java/com/google/gerrit/server/notedb/TooManyUpdatesException.java

@@ -0,0 +1,41 @@
+// Copyright (C) 2019 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.notedb;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.gerrit.exceptions.StorageException;
+import com.google.gerrit.reviewdb.client.Change;
+
+/**
+ * Exception indicating that the change has received too many updates. Further actions apart from
+ * {@code abandon} or {@code submit} are blocked.
+ */
+public class TooManyUpdatesException extends StorageException {
+  @VisibleForTesting
+  public static String message(Change.Id id, int maxUpdates) {
+    return "Change "
+        + id
+        + " may not exceed "
+        + maxUpdates
+        + " updates. It may still be abandoned or submitted. To continue working on this "
+        + "change, recreate it with a new Change-Id, then abandon this one.";
+  }
+
+  private static final long serialVersionUID = 1L;
+
+  TooManyUpdatesException(Change.Id id, int maxUpdates) {
+    super(message(id, maxUpdates));
+  }
+}

java/com/google/gerrit/server/update/BatchUpdate.java

@@ -52,7 +52,7 @@ import com.google.gerrit.server.logging.RequestId;
 import com.google.gerrit.server.notedb.ChangeNotes;
 import com.google.gerrit.server.notedb.ChangeUpdate;
 import com.google.gerrit.server.notedb.NoteDbUpdateManager;
-import com.google.gerrit.server.notedb.NoteDbUpdateManager.TooManyUpdatesException;
+import com.google.gerrit.server.notedb.TooManyUpdatesException;
 import com.google.gerrit.server.project.InvalidChangeOperationException;
 import com.google.gerrit.server.project.NoSuchChangeException;
 import com.google.gerrit.server.project.NoSuchProjectException;

java/com/google/gerrit/testing/AssertableExecutorService.java

@@ -0,0 +1,65 @@
+// Copyright (C) 2019 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.testing;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.common.util.concurrent.ForwardingExecutorService;
+import com.google.common.util.concurrent.MoreExecutors;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Future;
+import java.util.concurrent.atomic.AtomicInteger;
+
+/**
+ * Forwards all calls to a direct executor making it so that the submitted {@link Runnable}s run
+ * synchronously. Holds a count of the number of tasks that were executed.
+ */
+public class AssertableExecutorService extends ForwardingExecutorService {
+
+  private final ExecutorService delegate = MoreExecutors.newDirectExecutorService();
+  private final AtomicInteger numInteractions = new AtomicInteger();
+
+  @Override
+  protected ExecutorService delegate() {
+    return delegate;
+  }
+
+  @Override
+  public <T> Future<T> submit(Callable<T> task) {
+    numInteractions.incrementAndGet();
+    return super.submit(task);
+  }
+
+  @Override
+  public Future<?> submit(Runnable task) {
+    numInteractions.incrementAndGet();
+    return super.submit(task);
+  }
+
+  @Override
+  public <T> Future<T> submit(Runnable task, T result) {
+    numInteractions.incrementAndGet();
+    return super.submit(task, result);
+  }
+
+  /** Asserts and resets the number of executions this executor observed. */
+  public void assertInteractions(int expectedNumInteractions) {
+    assertThat(numInteractions.get())
+        .named("expectedRunnablesSubmittedOnExecutor")
+        .isEqualTo(expectedNumInteractions);
+    numInteractions.set(0);
+  }
+}

java/com/google/gerrit/testing/BUILD

@@ -1,7 +1,10 @@
 java_library(
     name = "gerrit-test-util",
     testonly = True,
-    srcs = glob(["**/*.java"]),
+    srcs = glob(
+        ["**/*.java"],
+        exclude = ["AssertableExecutorService.java"],
+    ),
     visibility = ["//visibility:public"],
     exports = [
         "//lib/easymock",
@@ -47,3 +50,15 @@ java_library(
         "//lib/truth",
     ],
 )
+
+java_library(
+    # This can't be part of gerrit-test-util because of https://github.com/google/guava/issues/2837
+    name = "assertable-executor",
+    testonly = True,
+    srcs = ["AssertableExecutorService.java"],
+    visibility = ["//visibility:public"],
+    deps = [
+        "//lib:guava",
+        "//lib/truth",
+    ],
+)

javatests/com/google/gerrit/server/BUILD

@@ -59,6 +59,7 @@ junit_tests(
         "//java/com/google/gerrit/server/schema",
         "//java/com/google/gerrit/server/schema/testing",
         "//java/com/google/gerrit/server/util/time",
+        "//java/com/google/gerrit/testing:assertable-executor",
         "//java/com/google/gerrit/testing:gerrit-test-util",
         "//java/com/google/gerrit/truth",
         "//java/org/eclipse/jgit:server",

javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java

@@ -29,6 +29,7 @@ import com.google.gerrit.reviewdb.client.CommentRange;
 import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.FanOutExecutor;
 import com.google.gerrit.server.GerritPersonIdent;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.InternalUser;
@@ -51,6 +52,7 @@ import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.gerrit.server.group.SystemGroupBackend;
 import com.google.gerrit.server.project.ProjectCache;
 import com.google.gerrit.server.util.time.TimeUtil;
+import com.google.gerrit.testing.AssertableExecutorService;
 import com.google.gerrit.testing.ConfigSuite;
 import com.google.gerrit.testing.FakeAccountCache;
 import com.google.gerrit.testing.GerritBaseTests;
@@ -63,6 +65,7 @@ import com.google.inject.Injector;
 import com.google.inject.util.Providers;
 import java.sql.Timestamp;
 import java.util.TimeZone;
+import java.util.concurrent.ExecutorService;
 import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
 import org.eclipse.jgit.junit.TestRepository;
 import org.eclipse.jgit.lib.Config;
@@ -92,6 +95,7 @@ public abstract class AbstractChangeNotesTest extends GerritBaseTests {
   protected Project.NameKey project;
   protected RevWalk rw;
   protected TestRepository<InMemoryRepository> tr;
+  protected AssertableExecutorService assertableFanOutExecutor;
 
   @Inject protected IdentifiedUser.GenericFactory userFactory;
 
@@ -125,6 +129,7 @@ public abstract class AbstractChangeNotesTest extends GerritBaseTests {
     ou.setFullName("Other Account");
     ou.setPreferredEmail("other@account.com");
     accountCache.put(ou);
+    assertableFanOutExecutor = new AssertableExecutorService();
 
     injector =
         Guice.createInjector(
@@ -157,6 +162,9 @@ public abstract class AbstractChangeNotesTest extends GerritBaseTests {
                     .toInstance(serverIdent);
                 bind(GitReferenceUpdated.class).toInstance(GitReferenceUpdated.DISABLED);
                 bind(MetricMaker.class).to(DisabledMetricMaker.class);
+                bind(ExecutorService.class)
+                    .annotatedWith(FanOutExecutor.class)
+                    .toInstance(assertableFanOutExecutor);
               }
             });
 

javatests/com/google/gerrit/server/notedb/DraftCommentNotesTest.java

@@ -0,0 +1,99 @@
+// Copyright (C) 2019 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.notedb;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.Comment;
+import com.google.gerrit.reviewdb.client.PatchLineComment.Status;
+import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.server.util.time.TimeUtil;
+import org.eclipse.jgit.lib.ObjectId;
+import org.junit.Test;
+
+public class DraftCommentNotesTest extends AbstractChangeNotesTest {
+
+  @Test
+  public void createAndPublishCommentInOneAction_runsDraftOperationAsynchronously()
+      throws Exception {
+    Change c = newChange();
+    ChangeUpdate update = newUpdate(c, otherUser);
+    update.setPatchSetId(c.currentPatchSetId());
+    update.putComment(Status.PUBLISHED, comment(c.currentPatchSetId()));
+    update.commit();
+
+    assertThat(newNotes(c).getDraftComments(otherUserId)).isEmpty();
+    assertableFanOutExecutor.assertInteractions(1);
+  }
+
+  @Test
+  public void createAndPublishComment_runsPublishDraftOperationAsynchronously() throws Exception {
+    Change c = newChange();
+    ChangeUpdate update = newUpdate(c, otherUser);
+
+    update.setPatchSetId(c.currentPatchSetId());
+    update.putComment(Status.DRAFT, comment(c.currentPatchSetId()));
+    update.commit();
+    assertThat(newNotes(c).getDraftComments(otherUserId)).hasSize(1);
+    assertableFanOutExecutor.assertInteractions(0);
+
+    update = newUpdate(c, otherUser);
+    update.putComment(Status.PUBLISHED, comment(c.currentPatchSetId()));
+    update.commit();
+
+    assertThat(newNotes(c).getDraftComments(otherUserId)).isEmpty();
+    assertableFanOutExecutor.assertInteractions(1);
+  }
+
+  @Test
+  public void createAndDeleteDraftComment_runsDraftOperationSynchronously() throws Exception {
+    Change c = newChange();
+
+    ChangeUpdate update = newUpdate(c, otherUser);
+    update.setPatchSetId(c.currentPatchSetId());
+    update.putComment(Status.DRAFT, comment(c.currentPatchSetId()));
+    update.commit();
+
+    ChangeNotes notes = newNotes(c);
+    assertThat(notes.getDraftComments(otherUserId)).hasSize(1);
+    assertableFanOutExecutor.assertInteractions(0);
+
+    update = newUpdate(c, otherUser);
+    update.setPatchSetId(c.currentPatchSetId());
+    update.deleteComment(comment(c.currentPatchSetId()));
+    update.commit();
+
+    notes = newNotes(c);
+    assertThat(notes.getDraftComments(otherUserId)).isEmpty();
+    assertableFanOutExecutor.assertInteractions(0);
+  }
+
+  private Comment comment(PatchSet.Id psId) {
+    return newComment(
+        psId,
+        "filename",
+        "uuid",
+        null,
+        0,
+        otherUser,
+        null,
+        TimeUtil.nowTs(),
+        "comment",
+        (short) 0,
+        ObjectId.fromString("abcd1234abcd1234abcd1234abcd1234abcd1234"),
+        false);
+  }
+}

javatests/com/google/gerrit/server/update/BatchUpdateTest.java

@@ -34,8 +34,8 @@ import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.gerrit.server.logging.RequestId;
 import com.google.gerrit.server.notedb.ChangeNotes;
 import com.google.gerrit.server.notedb.ChangeUpdate;
-import com.google.gerrit.server.notedb.NoteDbUpdateManager.TooManyUpdatesException;
 import com.google.gerrit.server.notedb.Sequences;
+import com.google.gerrit.server.notedb.TooManyUpdatesException;
 import com.google.gerrit.server.util.time.TimeUtil;
 import com.google.gerrit.testing.GerritBaseTests;
 import com.google.gerrit.testing.InMemoryTestEnvironment;