From 700e3e3bd742a372fa40235bcf960850a2a43b7e Mon Sep 17 00:00:00 2001
From: Dave Borowitz <dborowitz@google.com>
Date: Fri, 8 Jul 2016 10:31:28 -0400
Subject: [PATCH] ChangeIndexer: Disable auto-rebuilding changes in all cases

In I64000a57 we tried to do this, but only fixed one of the cases
where a ChangeData was created. Fix the remaining cases.

Also correct the NotesMigration check to just check that reads are
disabled. (The only difference between the enabled/disabled paths is
the value of autoRebuild passed to the ChangeNotes constructor, so
it doesn't matter which one we use if both reads and writes are
disabled.)

Change-Id: Ic55a2ce42782714700bfd156d98ebff24d2063cc
---
 .../server/index/change/ChangeIndexer.java    | 47 ++++++++++++-------
 .../gerrit/server/notedb/ChangeNotes.java     | 16 ++++++-
 2 files changed, 44 insertions(+), 19 deletions(-)

diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeIndexer.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeIndexer.java
index 4a331d9c75..8f324b3ed1 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeIndexer.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeIndexer.java
@@ -203,19 +203,7 @@ public class ChangeIndexer {
    */
   public void index(ReviewDb db, Change change)
       throws IOException, OrmException {
-    ChangeData cd;
-    if (notesMigration.commitChangeWrites()) {
-      // Auto-rebuilding when NoteDb reads are disabled just increases
-      // contention on the meta ref from a background indexing thread with
-      // little benefit. The next actual write to the entity may still incur a
-      // less-contentious rebuild.
-      ChangeNotes notes =
-          changeNotesFactory.createWithAutoRebuildingDisabled(change, null);
-      cd = changeDataFactory.create(db, notes);
-    } else {
-      cd = changeDataFactory.create(db, change);
-    }
-    index(cd);
+    index(newChangeData(db, change));
   }
 
   /**
@@ -226,8 +214,8 @@ public class ChangeIndexer {
    * @param changeId ID of the change to index.
    */
   public void index(ReviewDb db, Project.NameKey project, Change.Id changeId)
-      throws IOException {
-    index(changeDataFactory.create(db, project, changeId));
+      throws IOException, OrmException {
+    index(newChangeData(db, project, changeId));
   }
 
   /**
@@ -300,8 +288,8 @@ public class ChangeIndexer {
         };
         RequestContext oldCtx = context.setContext(newCtx);
         try {
-          ChangeData cd = changeDataFactory
-              .create(newCtx.getReviewDbProvider().get(), project, id);
+          ChangeData cd = newChangeData(
+              newCtx.getReviewDbProvider().get(), project, id);
           index(cd);
           return null;
         } finally  {
@@ -342,4 +330,29 @@ public class ChangeIndexer {
       return null;
     }
   }
+
+  // Avoid auto-rebuilding when reindexing if reading is disabled. This just
+  // increases contention on the meta ref from a background indexing thread
+  // with little benefit. The next actual write to the entity may still incur a
+  // less-contentious rebuild.
+  private ChangeData newChangeData(ReviewDb db, Change change)
+      throws OrmException {
+    if (!notesMigration.readChanges()) {
+      ChangeNotes notes = changeNotesFactory.createWithAutoRebuildingDisabled(
+          change, null);
+      return changeDataFactory.create(db, notes);
+    }
+    return changeDataFactory.create(db, change);
+  }
+
+  private ChangeData newChangeData(ReviewDb db, Project.NameKey project,
+      Change.Id changeId) throws OrmException {
+    if (!notesMigration.readChanges()) {
+      ChangeNotes notes = changeNotesFactory.createWithAutoRebuildingDisabled(
+          db, project, changeId);
+      return changeDataFactory.create(db, notes);
+    }
+    return changeDataFactory.create(db, project, changeId);
+  }
+
 }
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 6d87ae5b66..4737ba368d 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
@@ -149,7 +149,7 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
       return changes.get(0).notes();
     }
 
-    public ChangeNotes create(ReviewDb db, Project.NameKey project,
+    private Change loadChangeFromDb(ReviewDb db, Project.NameKey project,
         Change.Id changeId) throws OrmException {
       Change change = ReviewDbUtil.unwrapDb(db).changes().get(changeId);
       checkNotNull(change,
@@ -160,7 +160,19 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
           project, changeId, change.getProject());
       // TODO: Throw NoSuchChangeException when the change is not found in the
       // database
-      return new ChangeNotes(args, change).load();
+      return change;
+    }
+
+    public ChangeNotes create(ReviewDb db, Project.NameKey project,
+        Change.Id changeId) throws OrmException {
+      return new ChangeNotes(args, loadChangeFromDb(db, project, changeId))
+          .load();
+    }
+
+    public ChangeNotes createWithAutoRebuildingDisabled(ReviewDb db,
+        Project.NameKey project, Change.Id changeId) throws OrmException {
+      return new ChangeNotes(
+          args, loadChangeFromDb(db, project, changeId), false, null).load();
     }
 
     /**