During online reindexing of all changes skip changes already present

During online reindexing after an upgrade, all changes were reindexed,
even if they were already present in the new index. If the Gerrit
instance was restarted before the index was ready, this caused a lot
of repeated indexing.

Now, changes that are already present in the newest index version
won't be reindexed anymore. However, they will still be checked
for staleness and reindexed if necessary.

Revert the change "Remove unused method IndexedChangeQuery#oneResult"
(commit 585022e176ad31a82ab3fc0b7fe049f4bb13c6ff) since we need it here.

Release-Notes: Skip already reindexed changes during reindexing
Change-Id: I1fd78958f6fa0848f81630abab0428520f412baa
(cherry picked from commit 366ae421f263acf201adbcc1f13c69f03fef40cf)
diff --git a/java/com/google/gerrit/server/index/OnlineReindexer.java b/java/com/google/gerrit/server/index/OnlineReindexer.java
index eef394d..e3b8e7c 100644
--- a/java/com/google/gerrit/server/index/OnlineReindexer.java
+++ b/java/com/google/gerrit/server/index/OnlineReindexer.java
@@ -106,9 +106,6 @@
         "Starting online reindex of %s from schema version %s to %s",
         name, version(indexes.getSearchIndex()), version(index));
 
-    if (oldVersion != newVersion) {
-      index.deleteAll();
-    }
     SiteIndexer.Result result = batchIndexer.indexAll(index);
     if (!result.success()) {
       logger.atSevere().log(
diff --git a/java/com/google/gerrit/server/index/change/AllChangesIndexer.java b/java/com/google/gerrit/server/index/change/AllChangesIndexer.java
index 3935108..b98bae0 100644
--- a/java/com/google/gerrit/server/index/change/AllChangesIndexer.java
+++ b/java/com/google/gerrit/server/index/change/AllChangesIndexer.java
@@ -47,6 +47,7 @@
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.Callable;
 import java.util.concurrent.RejectedExecutionException;
@@ -197,15 +198,20 @@
     return Result.create(sw, ok.get(), nDone, nFailed);
   }
 
+  /**
+   * Reindexes all changes in a given project, even if they already exist in the index. Changes will
+   * not be sliced to allow multithreaded reindexing.
+   */
   @Nullable
   public Callable<Void> reindexProject(
       ChangeIndexer indexer, Project.NameKey project, Task done, Task failed) {
     try (Repository repo = repoManager.openRepository(project)) {
-      return reindexProjectSlice(
+      return new ProjectSliceIndexer(
           indexer,
           ProjectSlice.oneSlice(project, ChangeNotes.Factory.scanChangeIds(repo)),
           done,
-          failed);
+          failed,
+          true);
     } catch (IOException e) {
       logger.atSevere().log("%s", e.getMessage());
       return null;
@@ -214,7 +220,7 @@
 
   public Callable<Void> reindexProjectSlice(
       ChangeIndexer indexer, ProjectSlice projectSlice, Task done, Task failed) {
-    return new ProjectSliceIndexer(indexer, projectSlice, done, failed);
+    return new ProjectSliceIndexer(indexer, projectSlice, done, failed, false);
   }
 
   private class ProjectSliceIndexer implements Callable<Void> {
@@ -222,16 +228,19 @@
     private final ProjectSlice projectSlice;
     private final ProgressMonitor done;
     private final ProgressMonitor failed;
+    private final boolean forceReindex;
 
     private ProjectSliceIndexer(
         ChangeIndexer indexer,
         ProjectSlice projectSlice,
         ProgressMonitor done,
-        ProgressMonitor failed) {
+        ProgressMonitor failed,
+        boolean forceReindex) {
       this.indexer = indexer;
       this.projectSlice = projectSlice;
       this.done = done;
       this.failed = failed;
+      this.forceReindex = forceReindex;
     }
 
     @Override
@@ -247,6 +256,10 @@
                     + projectSlice.slice()
                     + "]");
         OnlineReindexMode.begin();
+        Optional<ChangeIndex> newestIndex = indexer.getNewestIndex();
+        if (newestIndex.isEmpty()) {
+          logger.atWarning().log("No change index available yet");
+        }
         // Order of scanning changes is undefined. This is ok if we assume that packfile locality is
         // not important for indexing, since sites should have a fully populated DiffSummary cache.
         // It does mean that reindexing after invalidating the DiffSummary cache will be expensive,
@@ -257,7 +270,7 @@
                 projectSlice.metaIdByChange(),
                 projectSlice.name(),
                 id -> (id.get() % projectSlice.slices()) == projectSlice.slice())
-            .forEach(r -> index(r));
+            .forEach(r -> index(r, newestIndex));
         OnlineReindexMode.end();
       } finally {
         Thread.currentThread().setName(oldThreadName);
@@ -265,16 +278,23 @@
       return null;
     }
 
-    private void index(ChangeNotesResult r) {
+    private void index(ChangeNotesResult r, Optional<ChangeIndex> newestIndex) {
       if (r.error().isPresent()) {
         fail("Failed to read change " + r.id() + " for indexing", true, r.error().get());
         return;
       }
       try {
-        indexer.index(changeDataFactory.create(r.notes()));
+        if (forceReindex || !indexer.isChangeAlreadyIndexed(r.id(), newestIndex)) {
+          indexer.index(changeDataFactory.create(r.notes()));
+          verboseWriter.format(
+              "Reindexed change %d (project: %s)\n",
+              r.id().get(), r.notes().getProjectName().get());
+
+        } else {
+          verboseWriter.format(
+              "Skipped change %d (project: %s)\n", r.id().get(), r.notes().getProjectName().get());
+        }
         done.update(1);
-        verboseWriter.format(
-            "Reindexed change %d (project: %s)\n", r.id().get(), r.notes().getProjectName().get());
       } catch (RejectedExecutionException e) {
         // Server shutdown, don't spam the logs.
         failSilently();
diff --git a/java/com/google/gerrit/server/index/change/ChangeIndexer.java b/java/com/google/gerrit/server/index/change/ChangeIndexer.java
index 517809a..17401ec 100644
--- a/java/com/google/gerrit/server/index/change/ChangeIndexer.java
+++ b/java/com/google/gerrit/server/index/change/ChangeIndexer.java
@@ -45,6 +45,7 @@
 import com.google.inject.assistedinject.AssistedInject;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.Comparator;
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
@@ -244,6 +245,17 @@
     fireChangeIndexedEvent(cd.project().get(), cd.getId().get());
   }
 
+  public boolean isChangeAlreadyIndexed(Change.Id id, Optional<ChangeIndex> newestIndex) {
+    if (newestIndex.isEmpty()) {
+      return false;
+    }
+    return newestIndex.get().get(id, IndexedChangeQuery.oneResult()).isPresent();
+  }
+
+  public Optional<ChangeIndex> getNewestIndex() {
+    return getWriteIndexes().stream().max(Comparator.comparingInt(i -> i.getSchema().getVersion()));
+  }
+
   private void fireChangeScheduledForIndexingEvent(String projectName, int id) {
     indexedListeners.runEach(l -> l.onChangeScheduledForIndexing(projectName, id));
   }
diff --git a/java/com/google/gerrit/server/index/change/IndexedChangeQuery.java b/java/com/google/gerrit/server/index/change/IndexedChangeQuery.java
index 00642a9..f7ff13c 100644
--- a/java/com/google/gerrit/server/index/change/IndexedChangeQuery.java
+++ b/java/com/google/gerrit/server/index/change/IndexedChangeQuery.java
@@ -21,6 +21,7 @@
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.gerrit.entities.Change;
 import com.google.gerrit.index.IndexConfig;
@@ -51,6 +52,10 @@
  */
 public class IndexedChangeQuery extends IndexedQuery<Change.Id, ChangeData>
     implements ChangeDataSource, Matchable<ChangeData> {
+  public static QueryOptions oneResult() {
+    IndexConfig config = IndexConfig.createDefault();
+    return createOptions(config, 0, 1, config.pageSizeMultiplier(), 1, ImmutableSet.of());
+  }
 
   public static QueryOptions createOptions(
       IndexConfig config, int start, int limit, Set<String> fields) {