Merge changes from topic 'migrate-to-note-db'

* changes:
  Start implementing OnlineNoteDbMigrationIT
  TestNotesMigration: Configure readChangeSequence bit explicitly
  NoteDbMigrator: Refuse to rebuild if NoteDb writes are disabled
  Fix NoteDbMigrator#rebuild and add a real test

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/pgm/MigrateToNoteDbIT.java

@@ -1,89 +0,0 @@
-// Copyright (C) 2014 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.gerrit.acceptance.pgm;
-
-import static com.google.common.truth.Truth.assertThat;
-import static com.google.common.truth.Truth8.assertThat;
-
-import com.google.gerrit.launcher.GerritLauncher;
-import com.google.gerrit.server.config.SitePaths;
-import com.google.gerrit.server.notedb.ConfigNotesMigration;
-import com.google.gerrit.server.notedb.NotesMigrationState;
-import com.google.gerrit.testutil.TempFileUtil;
-import org.eclipse.jgit.lib.StoredConfig;
-import org.eclipse.jgit.storage.file.FileBasedConfig;
-import org.eclipse.jgit.util.FS;
-import org.junit.After;
-import org.junit.Before;
-import org.junit.Test;
-
-public class MigrateToNoteDbIT {
-  private String sitePath;
-  private StoredConfig gerritConfig;
-
-  @Before
-  public void setUp() throws Exception {
-    SitePaths sitePaths = new SitePaths(TempFileUtil.createTempDirectory().toPath());
-    sitePath = sitePaths.site_path.toString();
-    gerritConfig = new FileBasedConfig(sitePaths.gerrit_config.toFile(), FS.detect());
-    initSite();
-  }
-
-  @After
-  public void tearDown() throws Exception {
-    TempFileUtil.cleanup();
-  }
-
-  @Test
-  public void rebuildEmptySiteStartingWithNoteDbDisabed() throws Exception {
-    assertNotesMigrationState(NotesMigrationState.REVIEW_DB);
-    runGerrit("MigrateToNoteDb", "-d", sitePath, "--show-stack-trace");
-    assertNotesMigrationState(NotesMigrationState.READ_WRITE_NO_SEQUENCE);
-  }
-
-  @Test
-  public void rebuildEmptySiteStartingWithNoteDbEnabled() throws Exception {
-    setNotesMigrationState(NotesMigrationState.READ_WRITE_NO_SEQUENCE);
-    runGerrit("MigrateToNoteDb", "-d", sitePath, "--show-stack-trace");
-    assertNotesMigrationState(NotesMigrationState.READ_WRITE_NO_SEQUENCE);
-  }
-
-  private void initSite() throws Exception {
-    runGerrit(
-        "init",
-        "-d",
-        sitePath,
-        "--batch",
-        "--no-auto-start",
-        "--skip-plugins",
-        "--show-stack-trace");
-  }
-
-  private static void runGerrit(String... args) throws Exception {
-    assertThat(GerritLauncher.mainImpl(args)).isEqualTo(0);
-  }
-
-  private void setNotesMigrationState(NotesMigrationState state) throws Exception {
-    gerritConfig.load();
-    ConfigNotesMigration.setConfigValues(gerritConfig, state.migration());
-    gerritConfig.save();
-  }
-
-  private void assertNotesMigrationState(NotesMigrationState expected) throws Exception {
-    gerritConfig.load();
-    assertThat(NotesMigrationState.forNotesMigration(new ConfigNotesMigration(gerritConfig)))
-        .hasValue(expected);
-  }
-}

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/pgm/OfflineNoteDbMigrationIT.java

@@ -0,0 +1,190 @@
+// Copyright (C) 2014 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.acceptance.pgm;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth8.assertThat;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.gerrit.acceptance.AccountCreator;
+import com.google.gerrit.acceptance.GerritServer;
+import com.google.gerrit.acceptance.NoHttpd;
+import com.google.gerrit.acceptance.TestAccount;
+import com.google.gerrit.acceptance.UseLocalDisk;
+import com.google.gerrit.extensions.api.GerritApi;
+import com.google.gerrit.extensions.common.ChangeInput;
+import com.google.gerrit.launcher.GerritLauncher;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.reviewdb.server.ReviewDbUtil;
+import com.google.gerrit.server.config.SitePaths;
+import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.notedb.ConfigNotesMigration;
+import com.google.gerrit.server.notedb.NoteDbChangeState;
+import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
+import com.google.gerrit.server.notedb.NoteDbChangeState.RefState;
+import com.google.gerrit.server.notedb.NotesMigrationState;
+import com.google.gerrit.server.util.ManualRequestContext;
+import com.google.gerrit.server.util.OneOffRequestContext;
+import com.google.gerrit.testutil.ConfigSuite;
+import com.google.gerrit.testutil.TempFileUtil;
+import com.google.inject.Injector;
+import java.nio.file.Path;
+import java.util.stream.Stream;
+import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.lib.StoredConfig;
+import org.eclipse.jgit.storage.file.FileBasedConfig;
+import org.eclipse.jgit.util.FS;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TestWatcher;
+import org.junit.runner.Description;
+
+/**
+ * Tests for offline {@code migrate-to-note-db} program.
+ *
+ * <p><strong>Note:</strong> These tests are very slow due to the repeated daemon startup. Prefer
+ * adding tests to {@link com.google.gerrit.acceptance.server.notedb.OnlineNoteDbMigrationIT} if
+ * possible.
+ */
+@UseLocalDisk
+@NoHttpd
+public class OfflineNoteDbMigrationIT {
+  @Rule
+  public TestWatcher testWatcher =
+      new TestWatcher() {
+        @Override
+        protected void starting(Description description) {
+          serverDesc = GerritServer.Description.forTestMethod(description, ConfigSuite.DEFAULT);
+        }
+      };
+
+  private GerritServer.Description serverDesc;
+
+  private Path site;
+  private StoredConfig gerritConfig;
+
+  @Before
+  public void setUp() throws Exception {
+    site = TempFileUtil.createTempDirectory().toPath();
+    GerritServer.init(serverDesc, new Config(), site);
+    gerritConfig = new FileBasedConfig(new SitePaths(site).gerrit_config.toFile(), FS.detect());
+  }
+
+  @After
+  public void tearDown() throws Exception {
+    TempFileUtil.cleanup();
+  }
+
+  @Test
+  public void rebuildEmptySiteStartingWithNoteDbDisabed() throws Exception {
+    assertNotesMigrationState(NotesMigrationState.REVIEW_DB);
+    migrate();
+    assertNotesMigrationState(NotesMigrationState.READ_WRITE_NO_SEQUENCE);
+  }
+
+  @Test
+  public void rebuildEmptySiteStartingWithNoteDbEnabled() throws Exception {
+    setNotesMigrationState(NotesMigrationState.READ_WRITE_NO_SEQUENCE);
+    migrate();
+    assertNotesMigrationState(NotesMigrationState.READ_WRITE_NO_SEQUENCE);
+  }
+
+  @Test
+  public void rebuildOneChangeTrialMode() throws Exception {
+    assertNotesMigrationState(NotesMigrationState.REVIEW_DB);
+    Project.NameKey project = new Project.NameKey("project");
+
+    Account.Id accountId;
+    Change.Id id;
+    try (GerritServer server = startServer();
+        ManualRequestContext ctx = openContext(server)) {
+      accountId = ctx.getUser().getAccountId();
+      GerritApi gApi = server.getTestInjector().getInstance(GerritApi.class);
+      gApi.projects().create("project");
+
+      ChangeInput in = new ChangeInput(project.get(), "master", "Test change");
+      in.newBranch = true;
+      id = new Change.Id(gApi.changes().create(in).info()._number);
+    }
+
+    migrate("--trial");
+    assertNotesMigrationState(NotesMigrationState.READ_WRITE_NO_SEQUENCE);
+
+    try (GerritServer server = startServer();
+        ManualRequestContext ctx = openContext(server, accountId)) {
+      GitRepositoryManager repoManager =
+          server.getTestInjector().getInstance(GitRepositoryManager.class);
+      ObjectId metaId;
+      try (Repository repo = repoManager.openRepository(project)) {
+        Ref ref = repo.exactRef(RefNames.changeMetaRef(id));
+        assertThat(ref).isNotNull();
+        metaId = ref.getObjectId();
+      }
+
+      ReviewDb db = ReviewDbUtil.unwrapDb(ctx.getReviewDbProvider().get());
+      Change c = db.changes().get(id);
+      assertThat(c).isNotNull();
+      NoteDbChangeState state = NoteDbChangeState.parse(c);
+      assertThat(state).isNotNull();
+      assertThat(state.getPrimaryStorage()).isEqualTo(PrimaryStorage.REVIEW_DB);
+      assertThat(state.getRefState()).hasValue(RefState.create(metaId, ImmutableMap.of()));
+    }
+  }
+
+  private GerritServer startServer() throws Exception {
+    return GerritServer.start(serverDesc, new Config(), site);
+  }
+
+  private ManualRequestContext openContext(GerritServer server) throws Exception {
+    Injector i = server.getTestInjector();
+    TestAccount a = i.getInstance(AccountCreator.class).admin();
+    return openContext(server, a.getId());
+  }
+
+  private ManualRequestContext openContext(GerritServer server, Account.Id accountId)
+      throws Exception {
+    return server.getTestInjector().getInstance(OneOffRequestContext.class).openAs(accountId);
+  }
+
+  private void migrate(String... additionalArgs) throws Exception {
+    String[] args =
+        Stream.concat(
+                Stream.of("migrate-to-note-db", "-d", site.toString(), "--show-stack-trace"),
+                Stream.of(additionalArgs))
+            .toArray(String[]::new);
+    assertThat(GerritLauncher.mainImpl(args)).isEqualTo(0);
+  }
+
+  private void setNotesMigrationState(NotesMigrationState state) throws Exception {
+    gerritConfig.load();
+    ConfigNotesMigration.setConfigValues(gerritConfig, state.migration());
+    gerritConfig.save();
+  }
+
+  private void assertNotesMigrationState(NotesMigrationState expected) throws Exception {
+    gerritConfig.load();
+    assertThat(NotesMigrationState.forNotesMigration(new ConfigNotesMigration(gerritConfig)))
+        .hasValue(expected);
+  }
+}

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/OnlineNoteDbMigrationIT.java

@@ -0,0 +1,82 @@
+// Copyright (C) 2017 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.acceptance.server.notedb;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth8.assertThat;
+import static com.google.common.truth.TruthJUnit.assume;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.NoHttpd;
+import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.acceptance.Sandboxed;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.reviewdb.server.ReviewDbUtil;
+import com.google.gerrit.server.notedb.NoteDbChangeState;
+import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
+import com.google.gerrit.server.notedb.NoteDbChangeState.RefState;
+import com.google.gerrit.server.notedb.NotesMigrationState;
+import com.google.gerrit.server.notedb.rebuild.NoteDbMigrator;
+import com.google.gerrit.testutil.NoteDbMode;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.Repository;
+import org.junit.Before;
+import org.junit.Test;
+
+@Sandboxed
+@NoHttpd
+public class OnlineNoteDbMigrationIT extends AbstractDaemonTest {
+  @Inject private Provider<NoteDbMigrator.Builder> migratorBuilderProvider;
+
+  @Before
+  public void setUp() {
+    assume().that(NoteDbMode.get()).isEqualTo(NoteDbMode.OFF);
+    assertNotesMigrationState(NotesMigrationState.REVIEW_DB);
+  }
+
+  @Test
+  public void rebuildOneChangeTrialMode() throws Exception {
+    PushOneCommit.Result r = createChange();
+    Change.Id id = r.getChange().getId();
+
+    try (NoteDbMigrator migrator = migratorBuilderProvider.get().setTrialMode(true).build()) {
+      migrator.migrate();
+    }
+    assertNotesMigrationState(NotesMigrationState.READ_WRITE_NO_SEQUENCE);
+
+    ObjectId metaId;
+    try (Repository repo = repoManager.openRepository(project)) {
+      Ref ref = repo.exactRef(RefNames.changeMetaRef(id));
+      assertThat(ref).isNotNull();
+      metaId = ref.getObjectId();
+    }
+
+    Change c = ReviewDbUtil.unwrapDb(db).changes().get(id);
+    assertThat(c).isNotNull();
+    NoteDbChangeState state = NoteDbChangeState.parse(c);
+    assertThat(state).isNotNull();
+    assertThat(state.getPrimaryStorage()).isEqualTo(PrimaryStorage.REVIEW_DB);
+    assertThat(state.getRefState()).hasValue(RefState.create(metaId, ImmutableMap.of()));
+  }
+
+  private void assertNotesMigrationState(NotesMigrationState expected) {
+    assertThat(NotesMigrationState.forNotesMigration(notesMigration)).hasValue(expected);
+  }
+}

gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilderImpl.java

@@ -247,17 +247,17 @@ public class ChangeRebuilderImpl extends ChangeRebuilder {
                     throw new AbortUpdateException();
                   } else if (!Objects.equals(oldNoteDbState, currNoteDbState)) {
                     // Another thread updated the state to something else.
-                    throw new ConflictingUpdateException(change, oldNoteDbState);
+                    throw new ConflictingUpdateRuntimeException(change, oldNoteDbState);
                   }
                   change.setNoteDbState(newNoteDbState);
                   return change;
                 }
               });
-    } catch (ConflictingUpdateException e) {
+    } catch (ConflictingUpdateRuntimeException e) {
       // Rethrow as an OrmException so the caller knows to use staged results. Strictly speaking
       // they are not completely up to date, but result we send to the caller is the same as if this
       // rebuild had executed before the other thread.
-      throw new OrmException(e.getMessage());
+      throw new ConflictingUpdateException(e);
     } catch (AbortUpdateException e) {
       if (NoteDbChangeState.parse(changeId, newNoteDbState)
           .isUpToDate(

gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ConflictingUpdateException.java

@@ -1,4 +1,4 @@
-// Copyright (C) 2016 The Android Open Source Project
+// Copyright (C) 2017 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.
@@ -14,16 +14,19 @@
 
 package com.google.gerrit.server.notedb.rebuild;
 
-import com.google.gerrit.reviewdb.client.Change;
+import com.google.gwtorm.server.OrmException;
-import com.google.gwtorm.server.OrmRuntimeException;
 
-class ConflictingUpdateException extends OrmRuntimeException {
+/**
+ * {@link com.google.gwtorm.server.OrmException} thrown by {@link ChangeRebuilder} when rebuilding a
+ * change failed because another operation modified its {@link
+ * com.google.gerrit.server.notedb.NoteDbChangeState}.
+ */
+public class ConflictingUpdateException extends OrmException {
   private static final long serialVersionUID = 1L;
 
-  ConflictingUpdateException(Change change, String expectedNoteDbState) {
+  // Always created from a ConflictingUpdateRuntimeException because it originates from an
-    super(
+  // AtomicUpdate, which cannot throw checked exceptions.
-        String.format(
+  ConflictingUpdateException(ConflictingUpdateRuntimeException cause) {
-            "Expected change %s to have noteDbState %s but was %s",
+    super(cause.getMessage(), cause);
-            change.getId(), expectedNoteDbState, change.getNoteDbState()));
   }
 }

gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ConflictingUpdateRuntimeException.java

@@ -0,0 +1,29 @@
+// Copyright (C) 2016 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.rebuild;
+
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gwtorm.server.OrmRuntimeException;
+
+class ConflictingUpdateRuntimeException extends OrmRuntimeException {
+  private static final long serialVersionUID = 1L;
+
+  ConflictingUpdateRuntimeException(Change change, String expectedNoteDbState) {
+    super(
+        String.format(
+            "Expected change %s to have noteDbState %s but was %s",
+            change.getId(), expectedNoteDbState, change.getNoteDbState()));
+  }
+}

gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java

@@ -44,10 +44,9 @@ import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.config.SitePaths;
+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.ConfigNotesMigration;
-import com.google.gerrit.server.notedb.NoteDbUpdateManager;
 import com.google.gerrit.server.notedb.NotesMigration;
 import com.google.gerrit.server.notedb.NotesMigrationState;
 import com.google.gerrit.server.notedb.rebuild.ChangeRebuilder.NoPatchSetsException;
@@ -82,9 +81,7 @@ public class NoteDbMigrator implements AutoCloseable {
   public static class Builder {
     private final SitePaths sitePaths;
     private final SchemaFactory<ReviewDb> schemaFactory;
-    private final NoteDbUpdateManager.Factory updateManagerFactory;
     private final ChangeRebuilder rebuilder;
-    private final ChangeBundleReader bundleReader;
     private final WorkQueue workQueue;
     private final NotesMigration globalNotesMigration;
 
@@ -99,16 +96,12 @@ public class NoteDbMigrator implements AutoCloseable {
     Builder(
         SitePaths sitePaths,
         SchemaFactory<ReviewDb> schemaFactory,
-        NoteDbUpdateManager.Factory updateManagerFactory,
         ChangeRebuilder rebuilder,
-        ChangeBundleReader bundleReader,
         WorkQueue workQueue,
         NotesMigration globalNotesMigration) {
       this.sitePaths = sitePaths;
       this.schemaFactory = schemaFactory;
-      this.updateManagerFactory = updateManagerFactory;
       this.rebuilder = rebuilder;
-      this.bundleReader = bundleReader;
       this.workQueue = workQueue;
       this.globalNotesMigration = globalNotesMigration;
     }
@@ -208,9 +201,7 @@ public class NoteDbMigrator implements AutoCloseable {
       return new NoteDbMigrator(
           sitePaths,
           schemaFactory,
-          updateManagerFactory,
           rebuilder,
-          bundleReader,
           globalNotesMigration,
           threads > 1
               ? MoreExecutors.listeningDecorator(workQueue.createQueue(threads, "RebuildChange"))
@@ -225,9 +216,7 @@ public class NoteDbMigrator implements AutoCloseable {
 
   private final FileBasedConfig gerritConfig;
   private final SchemaFactory<ReviewDb> schemaFactory;
-  private final NoteDbUpdateManager.Factory updateManagerFactory;
   private final ChangeRebuilder rebuilder;
-  private final ChangeBundleReader bundleReader;
   private final NotesMigration globalNotesMigration;
 
   private final ListeningExecutorService executor;
@@ -240,9 +229,7 @@ public class NoteDbMigrator implements AutoCloseable {
   private NoteDbMigrator(
       SitePaths sitePaths,
       SchemaFactory<ReviewDb> schemaFactory,
-      NoteDbUpdateManager.Factory updateManagerFactory,
       ChangeRebuilder rebuilder,
-      ChangeBundleReader bundleReader,
       NotesMigration globalNotesMigration,
       ListeningExecutorService executor,
       ImmutableList<Project.NameKey> projects,
@@ -251,9 +238,7 @@ public class NoteDbMigrator implements AutoCloseable {
       boolean trial,
       boolean forceRebuild) {
     this.schemaFactory = schemaFactory;
-    this.updateManagerFactory = updateManagerFactory;
     this.rebuilder = rebuilder;
-    this.bundleReader = bundleReader;
     this.globalNotesMigration = globalNotesMigration;
 
     boolean hasChanges = !changes.isEmpty();
@@ -399,6 +384,9 @@ public class NoteDbMigrator implements AutoCloseable {
   }
 
   public void rebuild() throws MigrationException, OrmException {
+    checkState(
+        globalNotesMigration.commitChangeWrites(),
+        "cannot rebuild without noteDb.changes.write=true");
     boolean ok;
     Stopwatch sw = Stopwatch.createStarted();
     log.info("Rebuilding changes in NoteDb");
@@ -468,27 +456,40 @@ public class NoteDbMigrator implements AutoCloseable {
   private boolean rebuildProject(
       ReviewDb db,
       ImmutableListMultimap<Project.NameKey, Change.Id> allChanges,
-      Project.NameKey project)
+      Project.NameKey project) {
-      throws IOException, OrmException {
     checkArgument(allChanges.containsKey(project));
     boolean ok = true;
     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 (NoteDbUpdateManager manager = updateManagerFactory.create(project)) {
+    try {
       for (Change.Id changeId : allChanges.get(project)) {
+        // 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.buildUpdates(manager, bundleReader.fromReviewDb(db, changeId));
+          rebuilder.rebuild(db, changeId);
         } catch (NoPatchSetsException e) {
           log.warn(e.getMessage());
+        } 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.update(1);
       }
-      manager.execute();
     } finally {
       pm.endTask();
     }

gerrit-server/src/test/java/com/google/gerrit/testutil/ConfigSuite.java

@@ -104,7 +104,7 @@ import org.junit.runners.model.InitializationError;
  * field annotated with {@code @ConfigSuite.Name}.
  */
 public class ConfigSuite extends Suite {
-  private static final String DEFAULT = "default";
+  public static final String DEFAULT = "default";
 
   @Target({METHOD})
   @Retention(RUNTIME)

gerrit-server/src/test/java/com/google/gerrit/testutil/TestNotesMigration.java

@@ -25,6 +25,7 @@ import com.google.inject.Singleton;
 public class TestNotesMigration extends NotesMigration {
   private volatile boolean readChanges;
   private volatile boolean writeChanges;
+  private volatile boolean readChangeSequence;
   private volatile PrimaryStorage changePrimaryStorage = PrimaryStorage.REVIEW_DB;
   private volatile boolean disableChangeReviewDb;
   private volatile boolean fuseUpdates;
@@ -41,9 +42,7 @@ public class TestNotesMigration extends NotesMigration {
 
   @Override
   public boolean readChangeSequence() {
-    // Unlike ConfigNotesMigration, read change numbers from NoteDb by default
+    return readChangeSequence;
-    // when reads are enabled, to improve test coverage.
-    return readChanges;
   }
 
   @Override
@@ -83,6 +82,11 @@ public class TestNotesMigration extends NotesMigration {
     return this;
   }
 
+  public TestNotesMigration setReadChangeSequence(boolean readChangeSequence) {
+    this.readChangeSequence = readChangeSequence;
+    return this;
+  }
+
   public TestNotesMigration setChangePrimaryStorage(PrimaryStorage changePrimaryStorage) {
     this.changePrimaryStorage = checkNotNull(changePrimaryStorage);
     return this;
@@ -115,6 +119,7 @@ public class TestNotesMigration extends NotesMigration {
   public TestNotesMigration setFrom(NotesMigration other) {
     setWriteChanges(other.rawWriteChangesSetting());
     setReadChanges(other.readChanges());
+    setReadChangeSequence(other.readChangeSequence());
     setChangePrimaryStorage(other.changePrimaryStorage());
     setDisableChangeReviewDb(other.disableChangeReviewDb());
     setFuseUpdates(other.fuseUpdates());