Merge changes from topic "notedb-pack-inserter" into stable-2.15

* changes:
  NoteDbMigrator: Use new pack-based inserter implementation
  Upgrade JGit to 4.9.0.201710071750-r.8-g678c99c05
  NoteDbMigrator: Use one NoteDbUpdateManager per project
  LockFailureException: Expose ref names

11 files changed, 304 insertions, 62 deletions

diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/OnlineNoteDbMigrationIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/OnlineNoteDbMigrationIT.java
index 51e2964d81..6b142635ca 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/OnlineNoteDbMigrationIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/OnlineNoteDbMigrationIT.java
@@ -60,8 +60,17 @@ import com.google.gwtorm.server.SchemaFactory;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 import java.io.IOException;
+import java.nio.file.FileVisitResult;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.SimpleFileVisitor;
+import java.nio.file.attribute.BasicFileAttributes;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Set;
+import java.util.SortedSet;
+import java.util.TreeSet;
+import org.eclipse.jgit.internal.storage.file.FileRepository;
 import org.eclipse.jgit.junit.TestRepository;
 import org.eclipse.jgit.lib.Config;
 import org.eclipse.jgit.lib.Constants;
@@ -374,10 +383,14 @@ public class OnlineNoteDbMigrationIT extends AbstractDaemonTest {
     Change.Id id1 = r1.getChange().getId();
     Change.Id id2 = r2.getChange().getId();
 
+    Set<String> objectFiles = getObjectFiles(project);
+    assertThat(objectFiles).isNotEmpty();
+
     migrate(b -> b.setThreads(threads));
-    assertNotesMigrationState(NOTE_DB, false, false);
 
+    assertNotesMigrationState(NOTE_DB, false, false);
     assertThat(sequences.nextChangeId()).isEqualTo(503);
+    assertThat(getObjectFiles(project)).containsExactlyElementsIn(objectFiles);
 
     ObjectId oldMetaId = null;
     int rowVersion = 0;
@@ -531,4 +544,23 @@ public class OnlineNoteDbMigrationIT extends AbstractDaemonTest {
   private void addListener(NotesMigrationStateListener listener) {
     addedListeners.add(listeners.add(listener));
   }
+
+  private SortedSet<String> getObjectFiles(Project.NameKey project) throws Exception {
+    SortedSet<String> files = new TreeSet<>();
+    try (Repository repo = repoManager.openRepository(project)) {
+      Files.walkFileTree(
+          ((FileRepository) repo).getObjectDatabase().getDirectory().toPath(),
+          new SimpleFileVisitor<Path>() {
+            @Override
+            public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) {
+              String name = file.getFileName().toString();
+              if (!attrs.isDirectory() && !name.endsWith(".pack") && !name.endsWith(".idx")) {
+                files.add(name);
+              }
+              return FileVisitResult.CONTINUE;
+            }
+          });
+    }
+    return files;
+  }
 }
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/externalids/ExternalIdsUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/externalids/ExternalIdsUpdate.java
index 00dc05a5f1..db37147fc5 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/externalids/ExternalIdsUpdate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/externalids/ExternalIdsUpdate.java
@@ -840,7 +840,7 @@ public class ExternalIdsUpdate {
       case FORCED:
         break;
       case LOCK_FAILURE:
-        throw new LockFailureException("Updating external IDs failed with " + res);
+        throw new LockFailureException("Updating external IDs failed with " + res, u);
       case IO_FAILURE:
       case NOT_ATTEMPTED:
       case REJECTED:
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/InMemoryInserter.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/InMemoryInserter.java
index 9c43bdbebc..b80f84669c 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/InMemoryInserter.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/InMemoryInserter.java
@@ -95,6 +95,10 @@ public class InMemoryInserter extends ObjectInserter {
     return ImmutableList.copyOf(inserted.values());
   }
 
+  public int getInsertedObjectCount() {
+    return inserted.values().size();
+  }
+
   public void clear() {
     inserted.clear();
   }
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/LockFailureException.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/LockFailureException.java
index 7380b0a686..02a30e098b 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/LockFailureException.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/LockFailureException.java
@@ -14,13 +14,38 @@
 
 package com.google.gerrit.server.git;
 
+import static com.google.common.collect.ImmutableList.toImmutableList;
+
+import com.google.common.collect.ImmutableList;
 import java.io.IOException;
+import org.eclipse.jgit.lib.BatchRefUpdate;
+import org.eclipse.jgit.lib.RefUpdate;
+import org.eclipse.jgit.transport.ReceiveCommand;
 
 /** Thrown when updating a ref in Git fails with LOCK_FAILURE. */
 public class LockFailureException extends IOException {
   private static final long serialVersionUID = 1L;
 
-  public LockFailureException(String message) {
+  private final ImmutableList<String> refs;
+
+  public LockFailureException(String message, RefUpdate refUpdate) {
     super(message);
+    refs = ImmutableList.of(refUpdate.getName());
+  }
+
+  public LockFailureException(String message, BatchRefUpdate batchRefUpdate) {
+    super(message);
+    refs =
+        batchRefUpdate
+            .getCommands()
+            .stream()
+            .filter(c -> c.getResult() == ReceiveCommand.Result.LOCK_FAILURE)
+            .map(ReceiveCommand::getRefName)
+            .collect(toImmutableList());
+  }
+
+  /** Subset of ref names that caused the lock failure. */
+  public ImmutableList<String> getFailedRefs() {
+    return refs;
   }
 }
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/VersionedMetaData.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/VersionedMetaData.java
index 1bf35864b0..92edc471c8 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/VersionedMetaData.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/VersionedMetaData.java
@@ -395,7 +395,8 @@ public abstract class VersionedMetaData {
                     + " in "
                     + db.getDirectory()
                     + ": "
-                    + ru.getResult());
+                    + ru.getResult(),
+                ru);
           case FORCED:
           case IO_FAILURE:
           case NOT_ATTEMPTED:
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java
index c9a28f4ce6..7799d2d6b7 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java
@@ -779,6 +779,7 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
           rebuildResult = checkNotNull(r);
           checkNotNull(r.newState());
           checkNotNull(r.staged());
+          checkNotNull(r.staged().changeObjects());
           return LoadHandle.create(
               ChangeNotesCommit.newStagedRevWalk(repo, r.staged().changeObjects()),
               r.newState().getChangeMetaId());
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java
index ea1f891376..3e36de93bb 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java
@@ -98,13 +98,13 @@ public class NoteDbUpdateManager implements AutoCloseable {
       ImmutableList<InsertedObject> changeObjects = ImmutableList.of();
       if (changeRepo != null) {
         changeCommands = changeRepo.getCommandsSnapshot();
-        changeObjects = changeRepo.tempIns.getInsertedObjects();
+        changeObjects = changeRepo.getInsertedObjects();
       }
       ImmutableList<ReceiveCommand> allUsersCommands = ImmutableList.of();
       ImmutableList<InsertedObject> allUsersObjects = ImmutableList.of();
       if (allUsersRepo != null) {
         allUsersCommands = allUsersRepo.getCommandsSnapshot();
-        allUsersObjects = allUsersRepo.tempIns.getInsertedObjects();
+        allUsersObjects = allUsersRepo.getInsertedObjects();
       }
       return new AutoValue_NoteDbUpdateManager_StagedResult(
           id, delta,
@@ -119,10 +119,32 @@ public class NoteDbUpdateManager implements AutoCloseable {
 
     public abstract ImmutableList<ReceiveCommand> changeCommands();
 
+    /**
+     * Objects inserted into the change repo for this change.
+     *
+     * <p>Includes all objects inserted for any change in this repo that may have been processed by
+     * the corresponding {@link NoteDbUpdateManager} instance, not just those objects that were
+     * inserted to handle this specific change's updates.
+     *
+     * @return inserted objects, or null if the corresponding {@link NoteDbUpdateManager} was
+     *     configured not to {@link NoteDbUpdateManager#setSaveObjects(boolean) save objects}.
+     */
+    @Nullable
     public abstract ImmutableList<InsertedObject> changeObjects();
 
     public abstract ImmutableList<ReceiveCommand> allUsersCommands();
 
+    /**
+     * Objects inserted into the All-Users repo for this change.
+     *
+     * <p>Includes all objects inserted into All-Users for any change that may have been processed
+     * by the corresponding {@link NoteDbUpdateManager} instance, not just those objects that were
+     * inserted to handle this specific change's updates.
+     *
+     * @return inserted objects, or null if the corresponding {@link NoteDbUpdateManager} was
+     *     configured not to {@link NoteDbUpdateManager#setSaveObjects(boolean) save objects}.
+     */
+    @Nullable
     public abstract ImmutableList<InsertedObject> allUsersObjects();
   }
 
@@ -144,17 +166,20 @@ public class NoteDbUpdateManager implements AutoCloseable {
     public final RevWalk rw;
     public final ChainedReceiveCommands cmds;
 
-    private final InMemoryInserter tempIns;
+    private final InMemoryInserter inMemIns;
+    private final ObjectInserter tempIns;
     @Nullable private final ObjectInserter finalIns;
 
     private final boolean close;
+    private final boolean saveObjects;
 
     private OpenRepo(
         Repository repo,
         RevWalk rw,
         @Nullable ObjectInserter ins,
         ChainedReceiveCommands cmds,
-        boolean close) {
+        boolean close,
+        boolean saveObjects) {
       ObjectReader reader = rw.getObjectReader();
       checkArgument(
           ins == null || reader.getCreatedFromInserter() == ins,
@@ -162,11 +187,21 @@ public class NoteDbUpdateManager implements AutoCloseable {
           ins,
           reader.getCreatedFromInserter());
       this.repo = checkNotNull(repo);
-      this.tempIns = new InMemoryInserter(rw.getObjectReader());
+
+      if (saveObjects) {
+        this.inMemIns = new InMemoryInserter(rw.getObjectReader());
+        this.tempIns = inMemIns;
+      } else {
+        checkArgument(ins != null);
+        this.inMemIns = null;
+        this.tempIns = ins;
+      }
+
       this.rw = new RevWalk(tempIns.newReader());
       this.finalIns = ins;
       this.cmds = checkNotNull(cmds);
       this.close = close;
+      this.saveObjects = saveObjects;
     }
 
     public Optional<ObjectId> getObjectId(String refName) throws IOException {
@@ -177,17 +212,25 @@ public class NoteDbUpdateManager implements AutoCloseable {
       return ImmutableList.copyOf(cmds.getCommands().values());
     }
 
+    @Nullable
+    ImmutableList<InsertedObject> getInsertedObjects() {
+      return saveObjects ? inMemIns.getInsertedObjects() : null;
+    }
+
     void flush() throws IOException {
       flushToFinalInserter();
       finalIns.flush();
     }
 
     void flushToFinalInserter() throws IOException {
+      if (!saveObjects) {
+        return;
+      }
       checkState(finalIns != null);
-      for (InsertedObject obj : tempIns.getInsertedObjects()) {
+      for (InsertedObject obj : inMemIns.getInsertedObjects()) {
         finalIns.insert(obj.type(), obj.data().toByteArray());
       }
-      tempIns.clear();
+      inMemIns.clear();
     }
 
     @Override
@@ -219,6 +262,8 @@ public class NoteDbUpdateManager implements AutoCloseable {
   private OpenRepo allUsersRepo;
   private Map<Change.Id, StagedResult> staged;
   private boolean checkExpectedState = true;
+  private boolean saveObjects = true;
+  private boolean atomicRefUpdates = true;
   private String refLogMessage;
   private PersonIdent refLogIdent;
   private PushCertificate pushCert;
@@ -264,14 +309,14 @@ public class NoteDbUpdateManager implements AutoCloseable {
   public NoteDbUpdateManager setChangeRepo(
       Repository repo, RevWalk rw, @Nullable ObjectInserter ins, ChainedReceiveCommands cmds) {
     checkState(changeRepo == null, "change repo already initialized");
-    changeRepo = new OpenRepo(repo, rw, ins, cmds, false);
+    changeRepo = new OpenRepo(repo, rw, ins, cmds, false, saveObjects);
     return this;
   }
 
   public NoteDbUpdateManager setAllUsersRepo(
       Repository repo, RevWalk rw, @Nullable ObjectInserter ins, ChainedReceiveCommands cmds) {
     checkState(allUsersRepo == null, "All-Users repo already initialized");
-    allUsersRepo = new OpenRepo(repo, rw, ins, cmds, false);
+    allUsersRepo = new OpenRepo(repo, rw, ins, cmds, false, saveObjects);
     return this;
   }
 
@@ -280,6 +325,37 @@ public class NoteDbUpdateManager implements AutoCloseable {
     return this;
   }
 
+  /**
+   * Set whether to save objects and make them available in {@link StagedResult}s.
+   *
+   * <p>If set, all objects inserted into all repos managed by this instance will be buffered in
+   * memory, and the {@link StagedResult}s will return non-null lists from {@link
+   * StagedResult#changeObjects()} and {@link StagedResult#allUsersObjects()}.
+   *
+   * <p>Not recommended if modifying a large number of changes with a single manager.
+   *
+   * @param saveObjects whether to save objects; defaults to true.
+   * @return this
+   */
+  public NoteDbUpdateManager setSaveObjects(boolean saveObjects) {
+    this.saveObjects = saveObjects;
+    return this;
+  }
+
+  /**
+   * Set whether to use atomic ref updates.
+   *
+   * <p>Can be set to false when the change updates represented by this manager aren't logically
+   * related, e.g. when the updater is only used to group objects together with a single inserter.
+   *
+   * @param atomicRefUpdates whether to use atomic ref updates; defaults to true.
+   * @return this
+   */
+  public NoteDbUpdateManager setAtomicRefUpdates(boolean atomicRefUpdates) {
+    this.atomicRefUpdates = atomicRefUpdates;
+    return this;
+  }
+
   public NoteDbUpdateManager setRefLogMessage(String message) {
     this.refLogMessage = message;
     return this;
@@ -336,7 +412,7 @@ public class NoteDbUpdateManager implements AutoCloseable {
     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) {
+      return new OpenRepo(repo, rw, ins, new ChainedReceiveCommands(repo), true, saveObjects) {
         @Override
         public void close() {
           reader.close();
@@ -543,6 +619,7 @@ public class NoteDbUpdateManager implements AutoCloseable {
     } else {
       // OpenRepo buffers objects separately; caller may assume that objects are available in the
       // inserter it previously passed via setChangeRepo.
+      checkState(saveObjects, "cannot use dryrun with saveObjects = false");
       or.flushToFinalInserter();
     }
 
@@ -554,6 +631,7 @@ public class NoteDbUpdateManager implements AutoCloseable {
       bru.setRefLogMessage(firstNonNull(guessRestApiHandler(), "Update NoteDb refs"), false);
     }
     bru.setRefLogIdent(refLogIdent != null ? refLogIdent : serverIdent.get());
+    bru.setAtomic(atomicRefUpdates);
     or.cmds.addTo(bru);
     bru.setAllowNonFastForwards(true);
 
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilderImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilderImpl.java
index 576a8dd41b..f96e96cffc 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilderImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilderImpl.java
@@ -187,7 +187,7 @@ public class ChangeRebuilderImpl extends ChangeRebuilder {
     }
     try (NoteDbUpdateManager manager = updateManagerFactory.create(change.getProject())) {
       buildUpdates(manager, bundleReader.fromReviewDb(db, changeId));
-      return execute(db, changeId, manager, checkReadOnly);
+      return execute(db, changeId, manager, checkReadOnly, true);
     }
   }
 
@@ -216,11 +216,15 @@ public class ChangeRebuilderImpl extends ChangeRebuilder {
   @Override
   public Result execute(ReviewDb db, Change.Id changeId, NoteDbUpdateManager manager)
       throws OrmException, IOException {
-    return execute(db, changeId, manager, true);
+    return execute(db, changeId, manager, true, true);
   }
 
   public Result execute(
-      ReviewDb db, Change.Id changeId, NoteDbUpdateManager manager, boolean checkReadOnly)
+      ReviewDb db,
+      Change.Id changeId,
+      NoteDbUpdateManager manager,
+      boolean checkReadOnly,
+      boolean executeManager)
       throws OrmException, IOException {
     db = ReviewDbUtil.unwrapDb(db);
     Change change = checkNoteDbState(ChangeNotes.readOneReviewDbChange(db, changeId));
@@ -277,11 +281,13 @@ public class ChangeRebuilderImpl extends ChangeRebuilder {
       // to the caller so they know to use the staged results instead of reading from the repo.
       throw new OrmException(NoteDbUpdateManager.CHANGES_READ_ONLY);
     }
-    manager.execute();
+    if (executeManager) {
+      manager.execute();
+    }
     return r;
   }
 
-  private static Change checkNoteDbState(Change c) throws OrmException {
+  static Change checkNoteDbState(Change c) throws OrmException {
     // Can only rebuild a change if its primary storage is ReviewDb.
     NoteDbChangeState s = NoteDbChangeState.parse(c);
     if (s != null && s.getPrimaryStorage() != PrimaryStorage.REVIEW_DB) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java
index add6d57a3b..29811745ae 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java
@@ -26,6 +26,7 @@ import static com.google.gerrit.server.notedb.NotesMigrationState.READ_WRITE_WIT
 import static com.google.gerrit.server.notedb.NotesMigrationState.WRITE;
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static java.util.Comparator.comparing;
+import static java.util.stream.Collectors.joining;
 import static java.util.stream.Collectors.toList;
 
 import com.google.common.annotations.VisibleForTesting;
@@ -56,12 +57,17 @@ import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
 import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.gerrit.server.git.LockFailureException;
 import com.google.gerrit.server.git.WorkQueue;
+import com.google.gerrit.server.notedb.ChangeBundleReader;
+import com.google.gerrit.server.notedb.ChangeNotes;
 import com.google.gerrit.server.notedb.MutableNotesMigration;
 import com.google.gerrit.server.notedb.NoteDbTable;
+import com.google.gerrit.server.notedb.NoteDbUpdateManager;
 import com.google.gerrit.server.notedb.NotesMigrationState;
 import com.google.gerrit.server.notedb.PrimaryStorageMigrator;
 import com.google.gerrit.server.notedb.RepoSequence;
 import com.google.gerrit.server.notedb.rebuild.ChangeRebuilder.NoPatchSetsException;
+import com.google.gerrit.server.project.NoSuchChangeException;
+import com.google.gerrit.server.update.ChainedReceiveCommands;
 import com.google.gerrit.server.util.ManualRequestContext;
 import com.google.gerrit.server.util.ThreadLocalRequestContext;
 import com.google.gwtorm.server.OrmException;
@@ -74,17 +80,24 @@ import java.io.OutputStreamWriter;
 import java.io.PrintWriter;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Optional;
+import java.util.Set;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeUnit;
 import java.util.function.Consumer;
 import java.util.function.Predicate;
 import org.eclipse.jgit.errors.ConfigInvalidException;
 import org.eclipse.jgit.errors.RepositoryNotFoundException;
+import org.eclipse.jgit.internal.storage.file.FileRepository;
 import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.lib.ObjectInserter;
+import org.eclipse.jgit.lib.ObjectReader;
 import org.eclipse.jgit.lib.ProgressMonitor;
+import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.lib.TextProgressMonitor;
+import org.eclipse.jgit.revwalk.RevWalk;
 import org.eclipse.jgit.storage.file.FileBasedConfig;
 import org.eclipse.jgit.util.FS;
 import org.eclipse.jgit.util.io.NullOutputStream;
@@ -119,10 +132,12 @@ public class NoteDbMigrator implements AutoCloseable {
     private final SitePaths sitePaths;
     private final SchemaFactory<ReviewDb> schemaFactory;
     private final GitRepositoryManager repoManager;
+    private final NoteDbUpdateManager.Factory updateManagerFactory;
+    private final ChangeBundleReader bundleReader;
     private final AllProjectsName allProjects;
     private final InternalUser.Factory userFactory;
     private final ThreadLocalRequestContext requestContext;
-    private final ChangeRebuilder rebuilder;
+    private final ChangeRebuilderImpl rebuilder;
     private final WorkQueue workQueue;
     private final MutableNotesMigration globalNotesMigration;
     private final PrimaryStorageMigrator primaryStorageMigrator;
@@ -144,10 +159,12 @@ public class NoteDbMigrator implements AutoCloseable {
         SitePaths sitePaths,
         SchemaFactory<ReviewDb> schemaFactory,
         GitRepositoryManager repoManager,
+        NoteDbUpdateManager.Factory updateManagerFactory,
+        ChangeBundleReader bundleReader,
         AllProjectsName allProjects,
         ThreadLocalRequestContext requestContext,
         InternalUser.Factory userFactory,
-        ChangeRebuilder rebuilder,
+        ChangeRebuilderImpl rebuilder,
         WorkQueue workQueue,
         MutableNotesMigration globalNotesMigration,
         PrimaryStorageMigrator primaryStorageMigrator,
@@ -159,6 +176,8 @@ public class NoteDbMigrator implements AutoCloseable {
       this.sitePaths = sitePaths;
       this.schemaFactory = schemaFactory;
       this.repoManager = repoManager;
+      this.updateManagerFactory = updateManagerFactory;
+      this.bundleReader = bundleReader;
       this.allProjects = allProjects;
       this.requestContext = requestContext;
       this.userFactory = userFactory;
@@ -318,6 +337,8 @@ public class NoteDbMigrator implements AutoCloseable {
           sitePaths,
           schemaFactory,
           repoManager,
+          updateManagerFactory,
+          bundleReader,
           allProjects,
           requestContext,
           userFactory,
@@ -343,10 +364,12 @@ public class NoteDbMigrator implements AutoCloseable {
   private final FileBasedConfig noteDbConfig;
   private final SchemaFactory<ReviewDb> schemaFactory;
   private final GitRepositoryManager repoManager;
+  private final NoteDbUpdateManager.Factory updateManagerFactory;
+  private final ChangeBundleReader bundleReader;
   private final AllProjectsName allProjects;
   private final ThreadLocalRequestContext requestContext;
   private final InternalUser.Factory userFactory;
-  private final ChangeRebuilder rebuilder;
+  private final ChangeRebuilderImpl rebuilder;
   private final MutableNotesMigration globalNotesMigration;
   private final PrimaryStorageMigrator primaryStorageMigrator;
   private final DynamicSet<NotesMigrationStateListener> listeners;
@@ -365,10 +388,12 @@ public class NoteDbMigrator implements AutoCloseable {
       SitePaths sitePaths,
       SchemaFactory<ReviewDb> schemaFactory,
       GitRepositoryManager repoManager,
+      NoteDbUpdateManager.Factory updateManagerFactory,
+      ChangeBundleReader bundleReader,
       AllProjectsName allProjects,
       ThreadLocalRequestContext requestContext,
       InternalUser.Factory userFactory,
-      ChangeRebuilder rebuilder,
+      ChangeRebuilderImpl rebuilder,
       MutableNotesMigration globalNotesMigration,
       PrimaryStorageMigrator primaryStorageMigrator,
       DynamicSet<NotesMigrationStateListener> listeners,
@@ -392,6 +417,8 @@ public class NoteDbMigrator implements AutoCloseable {
     this.schemaFactory = schemaFactory;
     this.rebuilder = rebuilder;
     this.repoManager = repoManager;
+    this.updateManagerFactory = updateManagerFactory;
+    this.bundleReader = bundleReader;
     this.allProjects = allProjects;
     this.requestContext = requestContext;
     this.userFactory = userFactory;
@@ -720,6 +747,12 @@ public class NoteDbMigrator implements AutoCloseable {
     return ImmutableListMultimap.copyOf(out);
   }
 
+  private static ObjectInserter newPackInserter(Repository repo) {
+    return repo instanceof FileRepository
+        ? ((FileRepository) repo).getObjectDatabase().newPackInserter()
+        : repo.newObjectInserter();
+  }
+
   private boolean rebuildProject(
       ReviewDb db,
       ImmutableListMultimap<Project.NameKey, Change.Id> allChanges,
@@ -729,43 +762,105 @@ public class NoteDbMigrator implements AutoCloseable {
     ProgressMonitor pm =
         new TextProgressMonitor(
             new PrintWriter(new BufferedWriter(new OutputStreamWriter(progressOut, UTF_8))));
-    pm.beginTask(FormatUtil.elide(project.get(), 50), allChanges.get(project).size());
-    try {
+    try (Repository changeRepo = repoManager.openRepository(project);
+        ObjectInserter changeIns = newPackInserter(changeRepo);
+        ObjectReader changeReader = changeIns.newReader();
+        RevWalk changeRw = new RevWalk(changeReader);
+        NoteDbUpdateManager manager =
+            updateManagerFactory
+                .create(project)
+                .setSaveObjects(false)
+                .setAtomicRefUpdates(false)
+                // Only use a PackInserter for the change repo, not All-Users.
+                //
+                // It's not possible to share a single inserter for All-Users across all project
+                // tasks, and we don't want to add one pack per project to All-Users. Adding many
+                // loose objects is preferable to many packs.
+                //
+                // Anyway, the number of objects inserted into All-Users is proportional to the
+                // number of pending draft comments, which should not be high (relative to the total
+                // number of changes), so the number of loose objects shouldn't be too unreasonable.
+                .setChangeRepo(
+                    changeRepo, changeRw, changeIns, new ChainedReceiveCommands(changeRepo))) {
+      Set<Change.Id> skipExecute = new HashSet<>();
       Collection<Change.Id> changes = allChanges.get(project);
-      for (Change.Id changeId : changes) {
-        // Update one change at a time, which ends up creating one NoteDbUpdateManager per change as
-        // well. This turns out to be no more expensive than batching, since each NoteDb operation
-        // is only writing single loose ref updates and loose objects. Plus we have to do one
-        // ReviewDb transaction per change due to the AtomicUpdate, so if we somehow batched NoteDb
-        // operations, ReviewDb would become the bottleneck.
-        try {
-          rebuilder.rebuild(db, changeId);
-        } catch (NoPatchSetsException e) {
-          log.warn(e.getMessage());
-        } catch (RepositoryNotFoundException e) {
-          log.warn("Repository {} not found while rebuilding change {}", project, changeId);
-        } catch (ConflictingUpdateException e) {
-          log.warn(
-              "Rebuilding detected a conflicting ReviewDb update for change {};"
-                  + " will be auto-rebuilt at runtime",
-              changeId);
-        } catch (LockFailureException e) {
-          log.warn(
-              "Rebuilding detected a conflicting NoteDb update for change {};"
-                  + " will be auto-rebuilt at runtime",
-              changeId);
-        } catch (Throwable t) {
-          log.error("Failed to rebuild change " + changeId, t);
-          ok = false;
+      pm.beginTask(FormatUtil.elide("Rebuilding " + project.get(), 50), changes.size());
+      try {
+        for (Change.Id changeId : changes) {
+          boolean staged = false;
+          try {
+            stage(db, changeId, manager);
+            staged = true;
+          } catch (NoPatchSetsException e) {
+            log.warn(e.getMessage());
+          } catch (Throwable t) {
+            log.error("Failed to rebuild change " + changeId, t);
+            ok = false;
+          }
+          pm.update(1);
+          if (!staged) {
+            skipExecute.add(changeId);
+          }
+        }
+      } finally {
+        pm.endTask();
+      }
+
+      pm.beginTask(
+          FormatUtil.elide("Saving " + project.get(), 50), changes.size() - skipExecute.size());
+      try {
+        for (Change.Id changeId : changes) {
+          if (skipExecute.contains(changeId)) {
+            continue;
+          }
+          try {
+            rebuilder.execute(db, changeId, manager, true, false);
+          } catch (ConflictingUpdateException e) {
+            log.warn(
+                "Rebuilding detected a conflicting ReviewDb update for change {};"
+                    + " will be auto-rebuilt at runtime",
+                changeId);
+          } catch (Throwable t) {
+            log.error("Failed to rebuild change " + changeId, t);
+            ok = false;
+          }
+          pm.update(1);
         }
-        pm.update(1);
+      } finally {
+        pm.endTask();
       }
-    } finally {
-      pm.endTask();
+
+      try {
+        manager.execute();
+      } catch (LockFailureException e) {
+        log.warn(
+            "Rebuilding detected a conflicting NoteDb update for the following refs, which will"
+                + " be auto-rebuilt at runtime: {}",
+            e.getFailedRefs().stream().distinct().sorted().collect(joining(", ")));
+      } catch (OrmException | IOException e) {
+        log.error("Failed to save NoteDb state for " + project, e);
+      }
+    } catch (RepositoryNotFoundException e) {
+      log.warn("Repository {} not found", project);
+    } catch (IOException e) {
+      log.error("Failed to rebuild project " + project, e);
     }
     return ok;
   }
 
+  private void stage(ReviewDb db, Change.Id changeId, NoteDbUpdateManager manager)
+      throws OrmException, IOException {
+    // Match ChangeRebuilderImpl#stage, but without calling manager.stage(), since that can only be
+    // called after building updates for all changes.
+    Change change =
+        ChangeRebuilderImpl.checkNoteDbState(ChangeNotes.readOneReviewDbChange(db, changeId));
+    if (change == null) {
+      // Could log here instead, but this matches the behavior of ChangeRebuilderImpl#stage.
+      throw new NoSuchChangeException(changeId);
+    }
+    rebuilder.buildUpdates(manager, bundleReader.fromReviewDb(db, changeId));
+  }
+
   private static boolean futuresToBoolean(List<ListenableFuture<Boolean>> futures, String errMsg) {
     try {
       return Futures.allAsList(futures).get().stream().allMatch(b -> b);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/update/RefUpdateUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/update/RefUpdateUtil.java
index ab0b78e2d5..86b4eef7c9 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/update/RefUpdateUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/update/RefUpdateUtil.java
@@ -74,7 +74,7 @@ public class RefUpdateUtil {
     }
 
     if (lockFailure + aborted == bru.getCommands().size()) {
-      throw new LockFailureException("Update aborted with one or more lock failures: " + bru);
+      throw new LockFailureException("Update aborted with one or more lock failures: " + bru, bru);
     } else if (failure > 0) {
       throw new IOException("Update failed: " + bru);
     }
diff --git a/lib/jgit/jgit.bzl b/lib/jgit/jgit.bzl
index 1d964b8291..27f53c7ec5 100644
--- a/lib/jgit/jgit.bzl
+++ b/lib/jgit/jgit.bzl
@@ -1,12 +1,12 @@
 load("//tools/bzl:maven_jar.bzl", "GERRIT", "MAVEN_LOCAL", "MAVEN_CENTRAL", "maven_jar")
 
-_JGIT_VERS = "4.9.0.201710071750-r"
+_JGIT_VERS = "4.9.0.201710071750-r.8-g678c99c05"
 
-_DOC_VERS = _JGIT_VERS # Set to _JGIT_VERS unless using a snapshot
+_DOC_VERS = "4.9.0.201710071750-r"  # Set to _JGIT_VERS unless using a snapshot
 
 JGIT_DOC_URL = "http://download.eclipse.org/jgit/site/" + _DOC_VERS + "/apidocs"
 
-_JGIT_REPO = MAVEN_CENTRAL # Leave here even if set to MAVEN_CENTRAL.
+_JGIT_REPO = GERRIT  # Leave here even if set to MAVEN_CENTRAL.
 
 # set this to use a local version.
 # "/home/<user>/projects/jgit"
@@ -26,28 +26,28 @@ def jgit_maven_repos():
         name = "jgit_lib",
         artifact = "org.eclipse.jgit:org.eclipse.jgit:" + _JGIT_VERS,
         repository = _JGIT_REPO,
-        sha1 = "69d8510b335d4d33d551a133505a4141311f970a",
-        src_sha1 = "6fd1eb331447b6163898b4d10aa769e2ac193740",
+        sha1 = "845fcaabaaf84f0924f78971def3adc962d3a69e",
+        src_sha1 = "8cc0627939d1bcd8fb313fd6bed03edfecc5011f",
         unsign = True,
     )
     maven_jar(
         name = "jgit_servlet",
         artifact = "org.eclipse.jgit:org.eclipse.jgit.http.server:" + _JGIT_VERS,
         repository = _JGIT_REPO,
-        sha1 = "93fb0075988b9c6bb97c725c03706f2341965b6b",
+        sha1 = "a0cb65ef0d9db387b010337b44ac4710be52b21e",
         unsign = True,
     )
     maven_jar(
         name = "jgit_archive",
         artifact = "org.eclipse.jgit:org.eclipse.jgit.archive:" + _JGIT_VERS,
         repository = _JGIT_REPO,
-        sha1 = "a15aee805c758516ad7e9fa3f16e27bb9f4a1c2e",
+        sha1 = "32c85ea349d19a0129d43ceda021a3bdd41a8242",
     )
     maven_jar(
         name = "jgit_junit",
         artifact = "org.eclipse.jgit:org.eclipse.jgit.junit:" + _JGIT_VERS,
         repository = _JGIT_REPO,
-        sha1 = "b6e712e743ea5798134f54547ae80456fad07f76",
+        sha1 = "549c3b338d66c2f8575394252a39e69d7fe71969",
         unsign = True,
     )