NoteDb: Auto-rebuild when either reads or writes are enabled

We plan to switch from write-only to read-write in a running server
after doing a batch rebuild. For that to work, we need to assume that
changes that are being written consistently will stay up-to-date in
NoteDb.

Previously, auto-rebuilding only happened when reads were enabled;
after this change, it will happen when either reads or writes are
enabled.

Change-Id: I265ef862df84d81e6917400309193e0681dd56db

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

@@ -97,14 +97,16 @@ public abstract class AbstractChangeNotes<T> {
   }
 
   protected final Args args;
+  protected final boolean autoRebuild;
   private final Change.Id changeId;
 
   private ObjectId revision;
   private boolean loaded;
 
-  AbstractChangeNotes(Args args, Change.Id changeId) {
+  AbstractChangeNotes(Args args, Change.Id changeId, boolean autoRebuild) {
     this.args = checkNotNull(args);
     this.changeId = checkNotNull(changeId);
+    this.autoRebuild = autoRebuild;
   }
 
   public Change.Id getChangeId() {
@@ -120,7 +122,9 @@ public abstract class AbstractChangeNotes<T> {
     if (loaded) {
       return self();
     }
-    if (!args.migration.readChanges()) {
+    boolean read = args.migration.readChanges();
+    boolean readOrWrite = read || args.migration.writeChanges();
+    if (!readOrWrite || !autoRebuild) {
       loadDefaults();
       return self();
     }
@@ -129,9 +133,15 @@ public abstract class AbstractChangeNotes<T> {
     }
     try (Timer1.Context timer = args.metrics.readLatency.start(CHANGES);
         Repository repo = args.repoManager.openRepository(getProjectName());
+        // Call openHandle even if reading is disabled, to trigger
+        // auto-rebuilding before this object may get passed to a ChangeUpdate.
         LoadHandle handle = openHandle(repo)) {
+      if (read) {
         revision = handle.id();
         onLoad(handle);
+      } else {
+        loadDefaults();
+      }
       loaded = true;
     } catch (ConfigInvalidException | IOException e) {
       throw new OrmException(e);
@@ -145,7 +155,11 @@ public abstract class AbstractChangeNotes<T> {
   }
 
   protected LoadHandle openHandle(Repository repo) throws IOException {
-    return LoadHandle.create(ChangeNotesCommit.newRevWalk(repo), readRef(repo));
+    return openHandle(repo, readRef(repo));
+  }
+
+  protected LoadHandle openHandle(Repository repo, ObjectId id) {
+    return LoadHandle.create(ChangeNotesCommit.newRevWalk(repo), id);
   }
 
   public T reload() throws OrmException {

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

@@ -377,7 +377,6 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
 
   private final Project.NameKey project;
   private final Change change;
-  private final boolean autoRebuild;
   private final RefCache refs;
 
   private ImmutableSortedMap<PatchSet.Id, PatchSet> patchSets;
@@ -403,10 +402,9 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
 
   private ChangeNotes(Args args, Project.NameKey project, Change change,
       boolean autoRebuild, @Nullable RefCache refs) {
-    super(args, change.getId());
+    super(args, change.getId(), autoRebuild);
     this.project = project;
     this.change = new Change(change);
-    this.autoRebuild = autoRebuild;
     this.refs = refs;
   }
 
@@ -626,26 +624,31 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
   protected LoadHandle openHandle(Repository repo) throws IOException {
     if (autoRebuild) {
       NoteDbChangeState state = NoteDbChangeState.parse(change);
+      ObjectId id = readRef(repo);
+      if (state == null && id == null) {
+        return super.openHandle(repo, id);
+      }
       RefCache refs = this.refs != null ? this.refs : new RepoRefCache(repo);
       if (!NoteDbChangeState.isChangeUpToDate(state, refs, getChangeId())) {
-        return rebuildAndOpen(repo);
+        return rebuildAndOpen(repo, id);
       }
     }
     return super.openHandle(repo);
   }
 
-  private LoadHandle rebuildAndOpen(Repository repo) throws IOException {
+  private LoadHandle rebuildAndOpen(Repository repo, ObjectId oldId)
+      throws IOException {
     try {
       NoteDbChangeState newState =
           args.rebuilder.get().rebuild(args.db.get(), getChangeId());
       if (newState == null) {
-        return super.openHandle(repo); // May be null in tests.
+        return super.openHandle(repo, oldId); // May be null in tests.
       }
       repo.scanForRepoChanges();
       return LoadHandle.create(
           ChangeNotesCommit.newRevWalk(repo), newState.getChangeMetaId());
     } catch (NoSuchChangeException e) {
-      return super.openHandle(repo);
+      return super.openHandle(repo, oldId);
     } catch (OrmException | ConfigInvalidException e) {
       throw new IOException(e);
     }

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

@@ -52,7 +52,6 @@ public class DraftCommentNotes extends AbstractChangeNotes<DraftCommentNotes> {
 
   private final Change change;
   private final Account.Id author;
-  private final boolean autoRebuild;
 
   private ImmutableListMultimap<RevId, PatchLineComment> comments;
   private RevisionNoteMap revisionNoteMap;
@@ -70,10 +69,9 @@ public class DraftCommentNotes extends AbstractChangeNotes<DraftCommentNotes> {
       Args args,
       @Assisted Change.Id changeId,
       @Assisted Account.Id author) {
-    super(args, changeId);
+    super(args, changeId, true);
     this.change = null;
     this.author = author;
-    this.autoRebuild = true;
   }
 
   DraftCommentNotes(
@@ -81,10 +79,9 @@ public class DraftCommentNotes extends AbstractChangeNotes<DraftCommentNotes> {
       Change change,
       Account.Id author,
       boolean autoRebuild) {
-    super(args, change.getId());
+    super(args, change.getId(), autoRebuild);
     this.change = change;
     this.author = author;
-    this.autoRebuild = autoRebuild;
   }
 
   RevisionNoteMap getRevisionNoteMap() {