ChangeIndexer: Disable auto-rebuilding changes

When a DFS ref update of a NoteDb meta ref fails due to a transient
error in the storage backend, we don't want to incur an additional
write to the ref during reindexing, as it may just fail again. Worse,
while BatchUpdate knows to ignore NoteDb write updates when executing
its NoteDbUpdateManager, it can't as easily ignore them while getting
its index futures. Thus an error that was successfully ignored is
likely to be followed by one that can't be.

Work around this for the case of NoteDb writes enabled but reads
disabled, by explicitly turning off auto-rebuilding from
ChangeIndexer. This doesn't help us in the case of NoteDb reads being
enabled, where we have to be able to read the latest NoteDb data from
the ref in order to reindex.

Change-Id: I64000a57ffcf73a9cbef42a6130e1fb4bc8e98f0

gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java

@@ -420,7 +420,7 @@ class ChangeApiImpl implements ChangeApi {
   public void index() throws RestApiException {
     try {
       index.apply(change, new Index.Input());
-    } catch (IOException e) {
+    } catch (IOException | OrmException e) {
       throw new RestApiException("Cannot index change", e);
     }
   }

gerrit-server/src/main/java/com/google/gerrit/server/change/Index.java

@@ -21,6 +21,7 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.change.Index.Input;
 import com.google.gerrit.server.index.change.ChangeIndexer;
 import com.google.gerrit.server.project.ChangeControl;
+import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 import com.google.inject.Singleton;
@@ -44,7 +45,7 @@ public class Index implements RestModifyView<ChangeResource, Input> {
 
   @Override
   public Response<?> apply(ChangeResource rsrc, Input input)
-      throws IOException, AuthException {
+      throws IOException, AuthException, OrmException {
     ChangeControl ctl = rsrc.getControl();
     if (!ctl.isOwner()
         && !ctl.getUser().getCapabilities().canMaintainServer()) {

gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditModifier.java

@@ -36,6 +36,7 @@ import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.gerrit.server.index.change.ChangeIndexer;
 import com.google.gerrit.server.project.InvalidChangeOperationException;
+import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 import com.google.inject.Singleton;
@@ -119,9 +120,10 @@ public class ChangeEditModifier {
    * @throws IOException
    * @throws ResourceConflictException When change edit already
    * exists for the change
+   * @throws OrmException
    */
   public RefUpdate.Result createEdit(Change change, PatchSet ps)
-      throws AuthException, IOException, ResourceConflictException {
+      throws AuthException, IOException, ResourceConflictException, OrmException {
     if (!currentUser.get().isIdentifiedUser()) {
       throw new AuthException("Authentication required");
     }

gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditUtil.java

@@ -200,9 +200,10 @@ public class ChangeEditUtil {
    *
    * @param edit change edit to delete
    * @throws IOException
+   * @throws OrmException
    */
   public void delete(ChangeEdit edit)
-      throws IOException {
+      throws IOException, OrmException {
     Change change = edit.getChange();
     try (Repository repo = gitManager.openRepository(change.getProject())) {
       deleteRef(repo, edit);

gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeIndexer.java

@@ -27,6 +27,8 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.extensions.events.ChangeIndexedListener;
 import com.google.gerrit.server.index.Index;
+import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.notedb.NotesMigration;
 import com.google.gerrit.server.query.change.ChangeData;
 import com.google.gerrit.server.util.RequestContext;
 import com.google.gerrit.server.util.ThreadLocalRequestContext;
@@ -94,6 +96,8 @@ public class ChangeIndexer {
   private final ChangeIndexCollection indexes;
   private final ChangeIndex index;
   private final SchemaFactory<ReviewDb> schemaFactory;
+  private final NotesMigration notesMigration;
+  private final ChangeNotes.Factory changeNotesFactory;
   private final ChangeData.Factory changeDataFactory;
   private final ThreadLocalRequestContext context;
   private final ListeningExecutorService executor;
@@ -101,6 +105,8 @@ public class ChangeIndexer {
 
   @AssistedInject
   ChangeIndexer(SchemaFactory<ReviewDb> schemaFactory,
+      NotesMigration notesMigration,
+      ChangeNotes.Factory changeNotesFactory,
       ChangeData.Factory changeDataFactory,
       ThreadLocalRequestContext context,
       DynamicSet<ChangeIndexedListener> indexedListener,
@@ -108,6 +114,8 @@ public class ChangeIndexer {
       @Assisted ChangeIndex index) {
     this.executor = executor;
     this.schemaFactory = schemaFactory;
+    this.notesMigration = notesMigration;
+    this.changeNotesFactory = changeNotesFactory;
     this.changeDataFactory = changeDataFactory;
     this.context = context;
     this.index = index;
@@ -117,6 +125,8 @@ public class ChangeIndexer {
 
   @AssistedInject
   ChangeIndexer(SchemaFactory<ReviewDb> schemaFactory,
+      NotesMigration notesMigration,
+      ChangeNotes.Factory changeNotesFactory,
       ChangeData.Factory changeDataFactory,
       ThreadLocalRequestContext context,
       DynamicSet<ChangeIndexedListener> indexedListener,
@@ -124,6 +134,8 @@ public class ChangeIndexer {
       @Assisted ChangeIndexCollection indexes) {
     this.executor = executor;
     this.schemaFactory = schemaFactory;
+    this.notesMigration = notesMigration;
+    this.changeNotesFactory = changeNotesFactory;
     this.changeDataFactory = changeDataFactory;
     this.context = context;
     this.index = null;
@@ -189,8 +201,23 @@ public class ChangeIndexer {
    * @param db review database.
    * @param change change to index.
    */
-  public void index(ReviewDb db, Change change) throws IOException {
-    index(changeDataFactory.create(db, change));
+  public void index(ReviewDb db, Change change)
+      throws IOException, OrmException {
+    ChangeData cd;
+    if (notesMigration.readChanges()) {
+      cd = changeDataFactory.create(db, change);
+    } else if (notesMigration.writeChanges()) {
+      // 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);
   }
 
   /**

gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/IndexChangesCommand.java

@@ -58,7 +58,7 @@ final class IndexChangesCommand extends SshCommand {
     for (ChangeResource rsrc : changes.values()) {
       try {
         index.apply(rsrc, new Index.Input());
-      } catch (IOException | RestApiException e) {
+      } catch (IOException | RestApiException | OrmException e) {
         ok = false;
         writeError("error", String.format(
             "failed to index change %s: %s", rsrc.getId(), e.getMessage()));