Merge "Add config option setting default primary storage for new changes"

gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/GerritServer.java

@@ -25,6 +25,8 @@ import com.google.gerrit.pgm.Init;
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.git.AsyncReceiveCommits;
 import com.google.gerrit.server.ssh.NoSshModule;
+import com.google.gerrit.server.util.ManualRequestContext;
+import com.google.gerrit.server.util.OneOffRequestContext;
 import com.google.gerrit.server.util.SocketUtil;
 import com.google.gerrit.server.util.SystemLog;
 import com.google.gerrit.testutil.FakeEmailSender;
@@ -62,6 +64,7 @@ public class GerritServer {
     static Description forTestClass(org.junit.runner.Description testDesc,
         String configName) {
       return new AutoValue_GerritServer_Description(
+          testDesc,
           configName,
           true, // @UseLocalDisk is only valid on methods.
           !has(NoHttpd.class, testDesc.getTestClass()),
@@ -75,6 +78,7 @@ public class GerritServer {
     static Description forTestMethod(org.junit.runner.Description testDesc,
         String configName) {
       return new AutoValue_GerritServer_Description(
+          testDesc,
           configName,
           testDesc.getAnnotation(UseLocalDisk.class) == null,
           testDesc.getAnnotation(NoHttpd.class) == null
@@ -97,6 +101,7 @@ public class GerritServer {
       return false;
     }
 
+    abstract org.junit.runner.Description testDescription();
     @Nullable abstract String configName();
     abstract boolean memory();
     abstract boolean httpd();
@@ -297,10 +302,7 @@ public class GerritServer {
 
   void stop() throws Exception {
     try {
-      if (NoteDbMode.get().equals(NoteDbMode.CHECK)) {
+      checkNoteDbState();
-        testInjector.getInstance(NoteDbChecker.class)
-            .rebuildAndCheckAllChanges();
-      }
     } finally {
       daemon.getLifecycleManager().stop();
       if (daemonService != null) {
@@ -312,6 +314,23 @@ public class GerritServer {
     }
   }
 
+  private void checkNoteDbState() throws Exception {
+    NoteDbMode mode = NoteDbMode.get();
+    if (mode != NoteDbMode.CHECK && mode != NoteDbMode.PRIMARY) {
+      return;
+    }
+    NoteDbChecker checker = testInjector.getInstance(NoteDbChecker.class);
+    OneOffRequestContext oneOffRequestContext =
+        testInjector.getInstance(OneOffRequestContext.class);
+    try (ManualRequestContext ctx = oneOffRequestContext.open()) {
+      if (mode == NoteDbMode.CHECK) {
+        checker.rebuildAndCheckAllChanges();
+      } else if (mode == NoteDbMode.PRIMARY) {
+        checker.assertNoReviewDbChanges(desc.testDescription());
+      }
+    }
+  }
+
   @Override
   public String toString() {
     return MoreObjects.toStringHelper(this).addValue(desc).toString();

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java

@@ -99,6 +99,7 @@ import com.google.gerrit.server.git.BatchUpdate;
 import com.google.gerrit.server.git.ChangeMessageModifier;
 import com.google.gerrit.server.git.ProjectConfig;
 import com.google.gerrit.server.group.SystemGroupBackend;
+import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
 import com.google.gerrit.server.project.ChangeControl;
 import com.google.gerrit.server.project.Util;
 import com.google.gerrit.testutil.FakeEmailSender.Message;
@@ -1011,6 +1012,8 @@ public class ChangeIT extends AbstractDaemonTest {
   public void addReviewerWithNoteDbWhenDummyApprovalInReviewDbExists()
       throws Exception {
     assume().that(notesMigration.enabled()).isTrue();
+    assume().that(notesMigration.changePrimaryStorage())
+        .isEqualTo(PrimaryStorage.REVIEW_DB);
 
     PushOneCommit.Result r = createChange();
 

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/RefAdvertisementIT.java

@@ -43,6 +43,7 @@ import com.google.gerrit.server.git.SearchingChangeCacheImpl;
 import com.google.gerrit.server.git.TagCache;
 import com.google.gerrit.server.git.VisibleRefFilter;
 import com.google.gerrit.server.notedb.ChangeNoteUtil;
+import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
 import com.google.gerrit.server.project.ProjectControl;
 import com.google.gerrit.server.project.Util;
 import com.google.gerrit.server.query.change.ChangeData;
@@ -447,8 +448,6 @@ public class RefAdvertisementIT extends AbstractDaemonTest {
 
   @Test
   public void receivePackOmitsMissingObject() throws Exception {
-    // Use the tactic from ConsistencyCheckerIT to insert a new patch set with a
-    // missing object.
     String rev = "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef";
     try (Repository repo = repoManager.openRepository(project)) {
       TestRepository<?> tr = new TestRepository<>(repo);
@@ -457,9 +456,11 @@ public class RefAdvertisementIT extends AbstractDaemonTest {
       PatchSet.Id psId = new PatchSet.Id(c3.getId(), 2);
       c.setCurrentPatchSet(psId, subject, c.getOriginalSubject());
 
-      PatchSet ps = TestChanges.newPatchSet(psId, rev, admin.getId());
+      if (notesMigration.changePrimaryStorage() == PrimaryStorage.REVIEW_DB) {
-      db.patchSets().insert(Collections.singleton(ps));
+        PatchSet ps = TestChanges.newPatchSet(psId, rev, admin.getId());
-      db.changes().update(Collections.singleton(c));
+        db.patchSets().insert(Collections.singleton(ps));
+        db.changes().update(Collections.singleton(c));
+      }
 
       if (notesMigration.commitChangeWrites()) {
         PersonIdent committer = serverIdent.get();

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java

@@ -50,6 +50,7 @@ import com.google.gerrit.server.git.BatchUpdate.ChangeContext;
 import com.google.gerrit.server.git.BatchUpdate.RepoContext;
 import com.google.gerrit.server.git.validators.CommitValidators;
 import com.google.gerrit.server.notedb.ChangeNoteUtil;
+import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
 import com.google.gerrit.server.project.ChangeControl;
 import com.google.gerrit.testutil.InMemoryRepositoryManager;
 import com.google.gerrit.testutil.TestChanges;
@@ -297,8 +298,10 @@ public class ConsistencyCheckerIT extends AbstractDaemonTest {
     String rev = "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef";
     PatchSet ps = newPatchSet(psId, rev, adminId);
 
-    db.changes().insert(singleton(c));
+    if (notesMigration.changePrimaryStorage() == PrimaryStorage.REVIEW_DB) {
-    db.patchSets().insert(singleton(ps));
+      db.changes().insert(singleton(c));
+      db.patchSets().insert(singleton(ps));
+    }
     addNoteDbCommit(
         c.getId(),
         "Create change\n"
@@ -824,10 +827,12 @@ public class ConsistencyCheckerIT extends AbstractDaemonTest {
     Change c = new Change(ctl.getChange());
     PatchSet.Id psId = nextPatchSetId(ctl);
     c.setCurrentPatchSet(psId, subject, c.getOriginalSubject());
-
     PatchSet ps = newPatchSet(psId, rev, adminId);
-    db.patchSets().insert(singleton(ps));
+
-    db.changes().update(singleton(c));
+    if (PrimaryStorage.of(c) == PrimaryStorage.REVIEW_DB) {
+      db.patchSets().insert(singleton(ps));
+      db.changes().update(singleton(c));
+    }
 
     addNoteDbCommit(
         c.getId(),

gerrit-server/src/main/java/com/google/gerrit/server/git/BatchUpdate.java

@@ -1049,18 +1049,45 @@ public class BatchUpdate implements AutoCloseable {
     private ChangeContext newChangeContext(ReviewDb db, Repository repo,
         RevWalk rw, Change.Id id) throws OrmException {
       Change c = newChanges.get(id);
-      if (c == null) {
+      boolean isNew = c != null;
+      PrimaryStorage defaultStorage = notesMigration.changePrimaryStorage();
+      if (isNew) {
+        // New change: populate noteDbState.
+        checkState(c.getNoteDbState() == null,
+            "noteDbState should not be filled in by callers");
+        if (defaultStorage == PrimaryStorage.NOTE_DB) {
+          c.setNoteDbState(NoteDbChangeState.NOTE_DB_PRIMARY_STATE);
+        }
+      } else {
+        // Existing change.
         c = ChangeNotes.readOneReviewDbChange(db, id);
         if (c == null) {
-          logDebug("Failed to get change {} from unwrapped db", id);
+          if (defaultStorage == PrimaryStorage.REVIEW_DB) {
-          throw new NoSuchChangeException(id);
+            logDebug("Failed to get change {} from unwrapped db", id);
+            throw new NoSuchChangeException(id);
+          }
+          // Not in ReviewDb, but new changes are created with default primary
+          // storage as NOTE_DB, so we can assume that a missing change is
+          // NoteDb primary. Pass a synthetic change into ChangeNotes.Factory,
+          // which lets ChangeNotes take care of the existence check.
+          //
+          // TODO(dborowitz): This assumption is potentially risky, because
+          // it means once we turn this option on and start creating changes
+          // without writing anything to ReviewDb, we can't turn this option
+          // back off without making those changes inaccessible. The problem
+          // is we have no way of distinguishing a change that only exists in
+          // NoteDb because it only ever existed in NoteDb, from a change that
+          // only exists in NoteDb because it used to exist in ReviewDb and
+          // deleting from ReviewDb succeeded but deleting from NoteDb failed.
+          //
+          // TODO(dborowitz): We actually still have that problem anyway. Maybe
+          // we need a cutoff timestamp? Or maybe we need to start leaving
+          // tombstones in ReviewDb?
+          c = ChangeNotes.Factory.newNoteDbOnlyChange(project, id);
         }
         NoteDbChangeState.checkNotReadOnly(c, skewMs);
       }
-      // Pass in preloaded change to controlFor, to avoid:
+      ChangeNotes notes = changeNotesFactory.createForBatchUpdate(c, !isNew);
-      //  - reading from a db that does not belong to this update
-      //  - attempting to read a change that doesn't exist yet
-      ChangeNotes notes = changeNotesFactory.createForBatchUpdate(c);
       ChangeControl ctl = changeControlFactory.controlFor(notes, user);
       return new ChangeContext(ctl, new BatchUpdateReviewDb(db), repo, rw);
     }

gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java

@@ -29,6 +29,7 @@ import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.gerrit.server.notedb.ChangeNotesCommit.ChangeNotesRevWalk;
 import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
 import com.google.gerrit.server.notedb.rebuild.ChangeRebuilder;
+import com.google.gerrit.server.project.NoSuchChangeException;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
@@ -174,7 +175,20 @@ public abstract class AbstractChangeNotes<T> {
     return ref != null ? ref.getObjectId() : null;
   }
 
-  protected LoadHandle openHandle(Repository repo) throws IOException {
+  /**
+   * Open a handle for reading this entity from a repository.
+   * <p>
+   * Implementations may override this method to provide auto-rebuilding
+   * behavior.
+   *
+   * @param repo open repository.
+   * @return handle for reading the entity.
+   *
+   * @throws NoSuchChangeException change does not exist.
+   * @throws IOException a repo-level error occurred.
+   */
+  protected LoadHandle openHandle(Repository repo)
+      throws NoSuchChangeException, IOException {
     return openHandle(repo, readRef(repo));
   }
 
@@ -182,7 +196,7 @@ public abstract class AbstractChangeNotes<T> {
     return LoadHandle.create(ChangeNotesCommit.newRevWalk(repo), id);
   }
 
-  public T reload() throws OrmException {
+  public T reload() throws NoSuchChangeException, OrmException {
     loaded = false;
     return load();
   }
@@ -215,7 +229,7 @@ public abstract class AbstractChangeNotes<T> {
 
   /** Set up the metadata, parsing any state from the loaded revision. */
   protected abstract void onLoad(LoadHandle handle)
-      throws IOException, ConfigInvalidException;
+      throws NoSuchChangeException, IOException, ConfigInvalidException;
 
   @SuppressWarnings("unchecked")
   protected final T self() {

gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java

@@ -36,6 +36,7 @@ import com.google.gerrit.common.Nullable;
 import com.google.gerrit.common.data.SubmitRecord;
 import com.google.gerrit.metrics.Timer1;
 import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.Branch;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.ChangeMessage;
 import com.google.gerrit.reviewdb.client.Comment;
@@ -124,7 +125,14 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
     public ChangeNotes createChecked(ReviewDb db, Project.NameKey project,
         Change.Id changeId) throws OrmException {
       Change change = readOneReviewDbChange(db, changeId);
-      if (change == null || !change.getProject().equals(project)) {
+      if (change == null) {
+        if (!args.migration.readChanges()) {
+          throw new NoSuchChangeException(changeId);
+        }
+        // Change isn't in ReviewDb, but its primary storage might be in NoteDb.
+        // Prepopulate the change exists with proper noteDbState field.
+        change = newNoteDbOnlyChange(project, changeId);
+      } else if (!change.getProject().equals(project)) {
         throw new NoSuchChangeException(changeId);
       }
       return new ChangeNotes(args, change).load();
@@ -144,16 +152,33 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
       return changes.get(0).notes();
     }
 
+    public static Change newNoteDbOnlyChange(
+        Project.NameKey project, Change.Id changeId) {
+      Change change = new Change(
+          null, changeId, null,
+          new Branch.NameKey(project, "INVALID_NOTE_DB_ONLY"),
+          null);
+      change.setNoteDbState(NoteDbChangeState.NOTE_DB_PRIMARY_STATE);
+      return change;
+    }
+
     private Change loadChangeFromDb(ReviewDb db, Project.NameKey project,
         Change.Id changeId) throws OrmException {
-      Change change = readOneReviewDbChange(db, changeId);
       checkArgument(project != null, "project is required");
-      checkNotNull(change,
+      Change change = readOneReviewDbChange(db, changeId);
-          "change %s not found in ReviewDb", changeId);
+
-      checkArgument(change.getProject().equals(project),
+      if (change == null && args.migration.readChanges()) {
-          "passed project %s when creating ChangeNotes for %s, but actual"
+        // Change isn't in ReviewDb, but its primary storage might be in NoteDb.
-          + " project is %s",
+        // Prepopulate the change exists with proper noteDbState field.
-          project, changeId, change.getProject());
+        change = newNoteDbOnlyChange(project, changeId);
+      } else {
+        checkNotNull(change, "change %s not found in ReviewDb", changeId);
+        checkArgument(change.getProject().equals(project),
+            "passed project %s when creating ChangeNotes for %s, but actual"
+            + " project is %s",
+            project, changeId, change.getProject());
+      }
+
       // TODO: Throw NoSuchChangeException when the change is not found in the
       // database
       return change;
@@ -168,7 +193,7 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
     public ChangeNotes createWithAutoRebuildingDisabled(ReviewDb db,
         Project.NameKey project, Change.Id changeId) throws OrmException {
       return new ChangeNotes(
-          args, loadChangeFromDb(db, project, changeId), false, null).load();
+          args, loadChangeFromDb(db, project, changeId), true, false, null).load();
     }
 
     /**
@@ -183,13 +208,14 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
       return new ChangeNotes(args, change);
     }
 
-    public ChangeNotes createForBatchUpdate(Change change) throws OrmException {
+    public ChangeNotes createForBatchUpdate(Change change, boolean shouldExist)
-      return new ChangeNotes(args, change, false, null).load();
+        throws OrmException {
+      return new ChangeNotes(args, change, shouldExist, false, null).load();
     }
 
     public ChangeNotes createWithAutoRebuildingDisabled(Change change,
         RefCache refs) throws OrmException {
-      return new ChangeNotes(args, change, false, refs).load();
+      return new ChangeNotes(args, change, true, false, refs).load();
     }
 
     // TODO(ekempin): Remove when database backend is deleted
@@ -226,7 +252,7 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
 
     public List<ChangeNotes> create(ReviewDb db, Project.NameKey project,
         Collection<Change.Id> changeIds, Predicate<ChangeNotes> predicate)
-            throws OrmException {
+        throws OrmException {
       List<ChangeNotes> notes = new ArrayList<>();
       if (args.migration.enabled()) {
         for (Change.Id cid : changeIds) {
@@ -302,13 +328,18 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
         Project.NameKey project) throws OrmException, IOException {
       Set<Change.Id> ids = scan(repo);
       List<ChangeNotes> changeNotes = new ArrayList<>(ids.size());
+      PrimaryStorage defaultStorage = args.migration.changePrimaryStorage();
       for (Change.Id id : ids) {
         Change change = readOneReviewDbChange(db, id);
         if (change == null) {
-          log.warn("skipping change {} found in project {} " +
+          if (defaultStorage == PrimaryStorage.REVIEW_DB) {
-              "but not in ReviewDb",
+            log.warn("skipping change {} found in project {} " +
-              id, project);
+                "but not in ReviewDb",
-          continue;
+                id, project);
+            continue;
+          }
+          // TODO(dborowitz): See discussion in BatchUpdate#newChangeContext.
+          change = newNoteDbOnlyChange(project, id);
         } else if (!change.getProject().equals(project)) {
           log.error(
               "skipping change {} found in project {} " +
@@ -337,6 +368,7 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
     }
   }
 
+  private final boolean shouldExist;
   private final RefCache refs;
 
   private Change change;
@@ -358,13 +390,14 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
 
   @VisibleForTesting
   public ChangeNotes(Args args, Change change) {
-    this(args, change, true, null);
+    this(args, change, true, true, null);
   }
 
-  private ChangeNotes(Args args, Change change, boolean autoRebuild,
+  private ChangeNotes(Args args, Change change, boolean shouldExist,
-      @Nullable RefCache refs) {
+      boolean autoRebuild, @Nullable RefCache refs) {
     super(args, change.getId(), PrimaryStorage.of(change), autoRebuild);
     this.change = new Change(change);
+    this.shouldExist = shouldExist;
     this.refs = refs;
   }
 
@@ -555,9 +588,14 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
 
   @Override
   protected void onLoad(LoadHandle handle)
-      throws IOException, ConfigInvalidException {
+      throws NoSuchChangeException, IOException, ConfigInvalidException {
     ObjectId rev = handle.id();
     if (rev == null) {
+      if (args.migration.readChanges()
+          && PrimaryStorage.of(change) == PrimaryStorage.NOTE_DB
+          && shouldExist) {
+        throw new NoSuchChangeException(getChangeId());
+      }
       loadDefaults();
       return;
     }
@@ -587,12 +625,17 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
   }
 
   @Override
-  protected LoadHandle openHandle(Repository repo) throws IOException {
+  protected LoadHandle openHandle(Repository repo)
+      throws NoSuchChangeException, IOException {
     if (autoRebuild) {
       NoteDbChangeState state = NoteDbChangeState.parse(change);
       ObjectId id = readRef(repo);
-      if (state == null && id == null) {
+      if (id == null) {
-        return super.openHandle(repo, id);
+        if (state == null) {
+          return super.openHandle(repo, id);
+        } else if (shouldExist) {
+          throw new NoSuchChangeException(getChangeId());
+        }
       }
       RefCache refs = this.refs != null ? this.refs : new RepoRefCache(repo);
       if (!NoteDbChangeState.isChangeUpToDate(state, refs, getChangeId())) {

gerrit-server/src/main/java/com/google/gerrit/server/notedb/ConfigNotesMigration.java

@@ -20,6 +20,7 @@ import static com.google.gerrit.server.notedb.NoteDbTable.CHANGES;
 
 import com.google.common.collect.ImmutableSet;
 import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
 import com.google.inject.AbstractModule;
 import com.google.inject.Inject;
 import com.google.inject.Singleton;
@@ -49,9 +50,11 @@ public class ConfigNotesMigration extends NotesMigration {
   }
 
   private static final String NOTE_DB = "noteDb";
+
+  private static final String PRIMARY_STORAGE = "primaryStorage";
   private static final String READ = "read";
-  private static final String WRITE = "write";
   private static final String SEQUENCE = "sequence";
+  private static final String WRITE = "write";
 
   private static void checkConfig(Config cfg) {
     Set<String> keys = new HashSet<>();
@@ -81,6 +84,7 @@ public class ConfigNotesMigration extends NotesMigration {
   private final boolean writeChanges;
   private final boolean readChanges;
   private final boolean readChangeSequence;
+  private final PrimaryStorage changePrimaryStorage;
 
   private final boolean writeAccounts;
   private final boolean readAccounts;
@@ -98,6 +102,9 @@ public class ConfigNotesMigration extends NotesMigration {
     // NoteDb. This decision for the default may be reevaluated later.
     readChangeSequence = cfg.getBoolean(NOTE_DB, CHANGES.key(), SEQUENCE, false);
 
+    changePrimaryStorage = cfg.getEnum(
+        NOTE_DB, CHANGES.key(), PRIMARY_STORAGE, PrimaryStorage.REVIEW_DB);
+
     writeAccounts = cfg.getBoolean(NOTE_DB, ACCOUNTS.key(), WRITE, false);
     readAccounts = cfg.getBoolean(NOTE_DB, ACCOUNTS.key(), READ, false);
   }
@@ -117,6 +124,11 @@ public class ConfigNotesMigration extends NotesMigration {
     return readChangeSequence;
   }
 
+  @Override
+  public PrimaryStorage changePrimaryStorage() {
+    return changePrimaryStorage;
+  }
+
   @Override
   public boolean writeAccounts() {
     return writeAccounts;

gerrit-server/src/main/java/com/google/gerrit/server/notedb/DraftCommentNotes.java

@@ -188,7 +188,8 @@ public class DraftCommentNotes extends AbstractChangeNotes<DraftCommentNotes> {
   }
 
   @Override
-  protected LoadHandle openHandle(Repository repo) throws IOException {
+  protected LoadHandle openHandle(Repository repo)
+      throws NoSuchChangeException, IOException {
     if (rebuildResult != null) {
       StagedResult sr = checkNotNull(rebuildResult.staged());
       return LoadHandle.create(
@@ -216,7 +217,8 @@ public class DraftCommentNotes extends AbstractChangeNotes<DraftCommentNotes> {
     return null;
   }
 
-  private LoadHandle rebuildAndOpen(Repository repo) throws IOException {
+  private LoadHandle rebuildAndOpen(Repository repo)
+      throws NoSuchChangeException, IOException {
     Timer1.Context timer = args.metrics.autoRebuildLatency.start(CHANGES);
     try {
       Change.Id cid = getChangeId();

gerrit-server/src/main/java/com/google/gerrit/server/notedb/NotesMigration.java

@@ -14,6 +14,8 @@
 
 package com.google.gerrit.server.notedb;
 
+import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
+
 /**
  * Holds the current state of the NoteDb migration.
  * <p>
@@ -71,6 +73,9 @@ public abstract class NotesMigration {
    */
   public abstract boolean readChangeSequence();
 
+  /** @return default primary storage for new changes. */
+  public abstract PrimaryStorage changePrimaryStorage();
+
   public abstract boolean readAccounts();
 
   public abstract boolean writeAccounts();

gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java

@@ -60,6 +60,7 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.reviewdb.server.ReviewDbUtil;
 import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.PatchSetUtil;
 import com.google.gerrit.server.Sequences;
 import com.google.gerrit.server.StarredChangesUtil;
 import com.google.gerrit.server.account.AccountManager;
@@ -146,6 +147,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
   @Inject protected InternalChangeQuery internalChangeQuery;
   @Inject protected ChangeNotes.Factory notesFactory;
   @Inject protected PatchSetInserter.Factory patchSetFactory;
+  @Inject protected PatchSetUtil psUtil;
   @Inject protected ChangeControl.GenericFactory changeControlFactory;
   @Inject protected ChangeQueryProcessor queryProcessor;
   @Inject protected SchemaCreator schemaCreator;
@@ -1579,9 +1581,13 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
     Account.Id user2 = createAccount("user2");
     TestRepository<Repo> repo = createProject("repo");
     Change change1 = insert(repo, newChange(repo));
-    PatchSet ps1 = db.patchSets().get(change1.currentPatchSetId());
+    ChangeNotes notes1 =
+        notesFactory.create(db, change1.getProject(), change1.getId());
+    PatchSet ps1 = psUtil.get(db, notes1, change1.currentPatchSetId());
     Change change2 = insert(repo, newChange(repo));
-    PatchSet ps2 = db.patchSets().get(change2.currentPatchSetId());
+    ChangeNotes notes2 =
+        notesFactory.create(db, change2.getProject(), change2.getId());
+    PatchSet ps2 = psUtil.get(db, notes2, change2.currentPatchSetId());
 
     requestContext.setContext(newRequestContext(user1));
     assertQuery("has:edit");
@@ -1692,7 +1698,9 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
     Project.NameKey project = new Project.NameKey("repo");
     TestRepository<Repo> repo = createProject(project.get());
     Change change = insert(repo, newChange(repo));
-    PatchSet ps = db.patchSets().get(change.currentPatchSetId());
+    ChangeNotes notes =
+        notesFactory.create(db, change.getProject(), change.getId());
+    PatchSet ps = psUtil.get(db, notes, change.currentPatchSetId());
 
     requestContext.setContext(newRequestContext(user));
     assertThat(changeEditModifier.createEdit(change, ps))
@@ -1714,6 +1722,9 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
 
   @Test
   public void refStateFields() throws Exception {
+    // This test method manages primary storage manually.
+    assume().that(notesMigration.changePrimaryStorage())
+        .isEqualTo(PrimaryStorage.REVIEW_DB);
     Account.Id user = createAccount("user");
     Project.NameKey project = new Project.NameKey("repo");
     TestRepository<Repo> repo = createProject(project.get());
@@ -1723,7 +1734,9 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
     Change change = insert(repo, newChangeForCommit(repo, commit));
     Change.Id id = change.getId();
     int c = id.get();
-    PatchSet ps = db.patchSets().get(change.currentPatchSetId());
+    ChangeNotes notes =
+        notesFactory.create(db, change.getProject(), change.getId());
+    PatchSet ps = psUtil.get(db, notes, change.currentPatchSetId());
     requestContext.setContext(newRequestContext(user));
 
     // Ensure one of each type of supported ref is present for the change. If

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

@@ -39,6 +39,7 @@ import com.google.inject.Singleton;
 
 import org.eclipse.jgit.errors.RepositoryNotFoundException;
 import org.eclipse.jgit.lib.Repository;
+import org.junit.runner.Description;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -124,6 +125,25 @@ public class NoteDbChecker {
     }
   }
 
+  public void assertNoReviewDbChanges(Description desc) throws Exception {
+    ReviewDb db = getUnwrappedDb();
+    assertThat(db.changes().all().toList())
+        .named("Changes in " + desc.getTestClass())
+        .isEmpty();
+    assertThat(db.changeMessages().all().toList())
+        .named("ChangeMessages in " + desc.getTestClass())
+        .isEmpty();
+    assertThat(db.patchSets().all().toList())
+        .named("PatchSets in " + desc.getTestClass())
+        .isEmpty();
+    assertThat(db.patchSetApprovals().all().toList())
+        .named("PatchSetApprovals in " + desc.getTestClass())
+        .isEmpty();
+    assertThat(db.patchComments().all().toList())
+        .named("PatchLineComments in " + desc.getTestClass())
+        .isEmpty();
+  }
+
   private List<ChangeBundle> readExpected(Stream<Change.Id> changeIds)
       throws Exception {
     boolean old = notesMigration.readChanges();

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

@@ -29,6 +29,9 @@ public enum NoteDbMode {
   /** Reading and writing all data to NoteDb is enabled. */
   READ_WRITE,
 
+  /** Changes are created with their primary storage as NoteDb. */
+  PRIMARY,
+
   /**
    * Run tests with NoteDb disabled, then convert ReviewDb to NoteDb and check
    * that the results match.
@@ -59,6 +62,6 @@ public enum NoteDbMode {
   }
 
   public static boolean readWrite() {
-    return get() == READ_WRITE;
+    return get() == READ_WRITE || get() == PRIMARY;
   }
 }

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

@@ -14,6 +14,9 @@
 
 package com.google.gerrit.testutil;
 
+import static com.google.common.base.Preconditions.checkNotNull;
+
+import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
 import com.google.gerrit.server.notedb.NotesMigration;
 import com.google.inject.Singleton;
 
@@ -22,6 +25,8 @@ import com.google.inject.Singleton;
 public class TestNotesMigration extends NotesMigration {
   private volatile boolean readChanges;
   private volatile boolean writeChanges;
+  private volatile PrimaryStorage changePrimaryStorage =
+      PrimaryStorage.REVIEW_DB;
   private volatile boolean failOnLoad;
 
   @Override
@@ -36,6 +41,11 @@ public class TestNotesMigration extends NotesMigration {
     return readChanges;
   }
 
+  @Override
+  public PrimaryStorage changePrimaryStorage() {
+    return changePrimaryStorage;
+  }
+
   // Increase visbility from superclass, as tests may want to check whether
   // NoteDb data is written in specific migration scenarios.
   @Override
@@ -68,6 +78,12 @@ public class TestNotesMigration extends NotesMigration {
     return this;
   }
 
+  public TestNotesMigration setChangePrimaryStorage(
+      PrimaryStorage changePrimaryStorage) {
+    this.changePrimaryStorage = checkNotNull(changePrimaryStorage);
+    return this;
+  }
+
   public TestNotesMigration setFailOnLoad(boolean failOnLoad) {
     this.failOnLoad = failOnLoad;
     return this;
@@ -82,16 +98,24 @@ public class TestNotesMigration extends NotesMigration {
       case READ_WRITE:
         setWriteChanges(true);
         setReadChanges(true);
+        setChangePrimaryStorage(PrimaryStorage.REVIEW_DB);
         break;
       case WRITE:
         setWriteChanges(true);
         setReadChanges(false);
+        setChangePrimaryStorage(PrimaryStorage.REVIEW_DB);
+        break;
+      case PRIMARY:
+        setWriteChanges(true);
+        setReadChanges(true);
+        setChangePrimaryStorage(PrimaryStorage.NOTE_DB);
         break;
       case CHECK:
       case OFF:
       default:
         setWriteChanges(false);
         setReadChanges(false);
+        setChangePrimaryStorage(PrimaryStorage.REVIEW_DB);
         break;
     }
     return this;