Merge "Merge branch 'stable-3.5' into stable-3.6" into stable-3.6

4 files changed, 48 insertions, 49 deletions

diff --git a/java/com/google/gerrit/index/QueryOptions.java b/java/com/google/gerrit/index/QueryOptions.java
index 91c8d1a5e5..29ab6d0119 100644
--- a/java/com/google/gerrit/index/QueryOptions.java
+++ b/java/com/google/gerrit/index/QueryOptions.java
@@ -73,7 +73,10 @@ public abstract class QueryOptions {
     int backendLimit = config().maxLimit();
     int limit = Ints.saturatedCast((long) limit() + start());
     limit = Math.min(limit, backendLimit);
-    int pageSize = Math.min(Ints.saturatedCast((long) pageSize() + start()), backendLimit);
+    int pageSize =
+        Math.min(
+            Math.min(Ints.saturatedCast((long) pageSize() + start()), config().maxPageSize()),
+            backendLimit);
     return create(config(), 0, null, pageSize, pageSizeMultiplier(), limit, fields());
   }
 
diff --git a/java/com/google/gerrit/server/index/change/AllChangesIndexer.java b/java/com/google/gerrit/server/index/change/AllChangesIndexer.java
index 99dacd95ff..ace3d6c1d2 100644
--- a/java/com/google/gerrit/server/index/change/AllChangesIndexer.java
+++ b/java/com/google/gerrit/server/index/change/AllChangesIndexer.java
@@ -21,6 +21,7 @@ import static com.google.gerrit.server.git.QueueProvider.QueueType.BATCH;
 
 import com.google.auto.value.AutoValue;
 import com.google.common.base.Stopwatch;
+import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSortedSet;
 import com.google.common.flogger.FluentLogger;
 import com.google.common.util.concurrent.ListenableFuture;
@@ -38,7 +39,6 @@ import com.google.gerrit.server.index.IndexExecutor;
 import com.google.gerrit.server.index.OnlineReindexMode;
 import com.google.gerrit.server.notedb.ChangeNotes;
 import com.google.gerrit.server.notedb.ChangeNotes.Factory.ChangeNotesResult;
-import com.google.gerrit.server.notedb.ChangeNotes.Factory.ScanResult;
 import com.google.gerrit.server.project.ProjectCache;
 import com.google.gerrit.server.query.change.ChangeData;
 import com.google.inject.Inject;
@@ -49,6 +49,7 @@ import java.util.concurrent.Callable;
 import java.util.concurrent.RejectedExecutionException;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
+import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.ProgressMonitor;
 import org.eclipse.jgit.lib.Repository;
 
@@ -107,10 +108,14 @@ public class AllChangesIndexer extends SiteIndexer<Change.Id, ChangeData, Change
 
     public abstract int slices();
 
-    public abstract ScanResult scanResult();
+    public abstract ImmutableMap<Change.Id, ObjectId> metaIdByChange();
 
-    private static ProjectSlice create(Project.NameKey name, int slice, int slices, ScanResult sr) {
-      return new AutoValue_AllChangesIndexer_ProjectSlice(name, slice, slices, sr);
+    private static ProjectSlice create(
+        Project.NameKey name,
+        int slice,
+        int slices,
+        ImmutableMap<Change.Id, ObjectId> metaIdByChange) {
+      return new AutoValue_AllChangesIndexer_ProjectSlice(name, slice, slices, metaIdByChange);
     }
   }
 
@@ -191,10 +196,10 @@ public class AllChangesIndexer extends SiteIndexer<Change.Id, ChangeData, Change
       Project.NameKey project,
       int slice,
       int slices,
-      ScanResult scanResult,
+      ImmutableMap<Change.Id, ObjectId> metaIdByChange,
       Task done,
       Task failed) {
-    return new ProjectIndexer(indexer, project, slice, slices, scanResult, done, failed);
+    return new ProjectIndexer(indexer, project, slice, slices, metaIdByChange, done, failed);
   }
 
   private class ProjectIndexer implements Callable<Void> {
@@ -202,7 +207,7 @@ public class AllChangesIndexer extends SiteIndexer<Change.Id, ChangeData, Change
     private final Project.NameKey project;
     private final int slice;
     private final int slices;
-    private final ScanResult scanResult;
+    private final ImmutableMap<Change.Id, ObjectId> metaIdByChange;
     private final ProgressMonitor done;
     private final ProgressMonitor failed;
 
@@ -211,14 +216,14 @@ public class AllChangesIndexer extends SiteIndexer<Change.Id, ChangeData, Change
         Project.NameKey project,
         int slice,
         int slices,
-        ScanResult scanResult,
+        ImmutableMap<Change.Id, ObjectId> metaIdByChange,
         ProgressMonitor done,
         ProgressMonitor failed) {
       this.indexer = indexer;
       this.project = project;
       this.slice = slice;
       this.slices = slices;
-      this.scanResult = scanResult;
+      this.metaIdByChange = metaIdByChange;
       this.done = done;
       this.failed = failed;
     }
@@ -232,7 +237,7 @@ public class AllChangesIndexer extends SiteIndexer<Change.Id, ChangeData, Change
       // but the goal is to invalidate that cache as infrequently as we possibly can. And besides,
       // we don't have concrete proof that improving packfile locality would help.
       notesFactory
-          .scan(scanResult, project, id -> (id.get() % slices) == slice)
+          .scan(metaIdByChange, project, id -> (id.get() % slices) == slice)
           .forEach(r -> index(r));
       OnlineReindexMode.end();
       return null;
@@ -337,8 +342,9 @@ public class AllChangesIndexer extends SiteIndexer<Change.Id, ChangeData, Change
       @Override
       public Void call() throws IOException {
         try (Repository repo = repoManager.openRepository(name)) {
-          ScanResult sr = ChangeNotes.Factory.scanChangeIds(repo);
-          int size = sr.all().size();
+          ImmutableMap<Change.Id, ObjectId> metaIdByChange =
+              ChangeNotes.Factory.scanChangeIds(repo);
+          int size = metaIdByChange.size();
           if (size > 0) {
             changeCount.addAndGet(size);
             int slices = 1 + size / PROJECT_SLICE_MAX_REFS;
@@ -351,7 +357,7 @@ public class AllChangesIndexer extends SiteIndexer<Change.Id, ChangeData, Change
             projTask.updateTotal(slices);
 
             for (int slice = 0; slice < slices; slice++) {
-              ProjectSlice projectSlice = ProjectSlice.create(name, slice, slices, sr);
+              ProjectSlice projectSlice = ProjectSlice.create(name, slice, slices, metaIdByChange);
               ListenableFuture<?> future =
                   executor.submit(
                       reindexProject(
@@ -359,7 +365,7 @@ public class AllChangesIndexer extends SiteIndexer<Change.Id, ChangeData, Change
                           name,
                           slice,
                           slices,
-                          projectSlice.scanResult(),
+                          projectSlice.metaIdByChange(),
                           doneTask,
                           failedTask));
               String description = "project " + name + " (" + slice + "/" + slices + ")";
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotes.java b/java/com/google/gerrit/server/notedb/ChangeNotes.java
index 3095cd2b63..00de505600 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotes.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotes.java
@@ -26,6 +26,7 @@ import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableListMultimap;
+import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.ImmutableSortedMap;
 import com.google.common.collect.ImmutableSortedSet;
@@ -33,8 +34,6 @@ import com.google.common.collect.ListMultimap;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Multimaps;
 import com.google.common.collect.Ordering;
-import com.google.common.collect.Sets;
-import com.google.common.collect.Sets.SetView;
 import com.google.common.flogger.FluentLogger;
 import com.google.errorprone.annotations.FormatMethod;
 import com.google.gerrit.common.Nullable;
@@ -70,6 +69,7 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
+import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.function.Predicate;
@@ -114,27 +114,18 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
       this.projectCache = projectCache;
     }
 
-    @AutoValue
-    public abstract static class ScanResult {
-      abstract ImmutableSet<Change.Id> fromPatchSetRefs();
-
-      abstract ImmutableSet<Change.Id> fromMetaRefs();
-
-      public SetView<Change.Id> all() {
-        return Sets.union(fromPatchSetRefs(), fromMetaRefs());
-      }
-    }
-
-    public static ScanResult scanChangeIds(Repository repo) throws IOException {
-      ImmutableSet.Builder<Change.Id> fromPs = ImmutableSet.builder();
-      ImmutableSet.Builder<Change.Id> fromMeta = ImmutableSet.builder();
+    public static ImmutableMap<Change.Id, ObjectId> scanChangeIds(Repository repo)
+        throws IOException {
+      ImmutableMap.Builder<Change.Id, ObjectId> metaIdByChange = ImmutableMap.builder();
       for (Ref r : repo.getRefDatabase().getRefsByPrefix(RefNames.REFS_CHANGES)) {
-        Change.Id id = Change.Id.fromRef(r.getName());
-        if (id != null) {
-          (r.getName().endsWith(RefNames.META_SUFFIX) ? fromMeta : fromPs).add(id);
+        if (r.getName().endsWith(RefNames.META_SUFFIX)) {
+          Change.Id id = Change.Id.fromRef(r.getName());
+          if (id != null) {
+            metaIdByChange.put(id, r.getObjectId());
+          }
         }
       }
-      return new AutoValue_ChangeNotes_Factory_ScanResult(fromPs.build(), fromMeta.build());
+      return metaIdByChange.build();
     }
 
     public ChangeNotes createChecked(Change c) {
@@ -300,27 +291,25 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
     }
 
     public Stream<ChangeNotesResult> scan(
-        ScanResult sr, Project.NameKey project, Predicate<Change.Id> changeIdPredicate) {
-      Stream<Change.Id> idStream = sr.all().stream();
+        ImmutableMap<Change.Id, ObjectId> metaIdByChange,
+        Project.NameKey project,
+        Predicate<Change.Id> changeIdPredicate) {
+      Stream<Map.Entry<Change.Id, ObjectId>> metaByIdStream = metaIdByChange.entrySet().stream();
       if (changeIdPredicate != null) {
-        idStream = idStream.filter(changeIdPredicate);
+        metaByIdStream = metaByIdStream.filter(e -> changeIdPredicate.test(e.getKey()));
       }
-      return idStream.map(id -> scanOneChange(project, sr, id)).filter(Objects::nonNull);
+      return metaByIdStream.map(e -> scanOneChange(project, e)).filter(Objects::nonNull);
     }
 
     @Nullable
-    private ChangeNotesResult scanOneChange(Project.NameKey project, ScanResult sr, Change.Id id) {
-      if (!sr.fromMetaRefs().contains(id)) {
-        // Stray patch set refs can happen due to normal error conditions, e.g. failed
-        // push processing, so aren't worth even a warning.
-        return null;
-      }
-
+    private ChangeNotesResult scanOneChange(
+        Project.NameKey project, Map.Entry<Change.Id, ObjectId> metaIdByChangeId) {
+      Change.Id id = metaIdByChangeId.getKey();
       // TODO(dborowitz): See discussion in BatchUpdate#newChangeContext.
       try {
         Change change = ChangeNotes.Factory.newChange(project, id);
         logger.atFine().log("adding change %s found in project %s", id, project);
-        return toResult(change);
+        return toResult(change, metaIdByChangeId.getValue());
       } catch (InvalidServerIdException ise) {
         logger.atWarning().withCause(ise).log(
             "skipping change %d in project %s because of an invalid server id", id.get(), project);
@@ -329,8 +318,8 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
     }
 
     @Nullable
-    private ChangeNotesResult toResult(Change rawChangeFromNoteDb) {
-      ChangeNotes n = new ChangeNotes(args, rawChangeFromNoteDb, true, null);
+    private ChangeNotesResult toResult(Change rawChangeFromNoteDb, ObjectId metaId) {
+      ChangeNotes n = new ChangeNotes(args, rawChangeFromNoteDb, true, null, metaId);
       try {
         n.load();
       } catch (Exception e) {
diff --git a/tools/BUILD b/tools/BUILD
index 4e4e5f0d32..26db1faf9b 100644
--- a/tools/BUILD
+++ b/tools/BUILD
@@ -156,6 +156,7 @@ java_package_configuration(
         "-Xep:FloatingPointLiteralPrecision:ERROR",
         "-Xep:FloggerArgumentToString:ERROR",
         "-Xep:FloggerFormatString:ERROR",
+        "-Xep:FloggerLogString:WARN",
         "-Xep:FloggerLogVarargs:ERROR",
         "-Xep:FloggerSplitLogStatement:ERROR",
         "-Xep:FloggerStringConcatenation:ERROR",