Merge changes from topic 'sequence-metric'

* changes:
  Add latency metric to Sequences
  Sequences: Push @SuppressWarnings down to call sites

Documentation/metrics.txt

@@ -111,6 +111,10 @@ reviewer suggestion.
 * `reviewer_suggestion/query_groups`: Latency for querying groups for reviewer
 suggestion.
 
+=== Repo Sequences
+
+* `sequence/next_id_latency`: Latency of requesting IDs from repo sequences.
+
 === Replication Plugin
 
 * `plugins/replication/replication_latency`: Time spent pushing to remote

gerrit-server/src/main/java/com/google/gerrit/server/Sequences.java

@@ -18,6 +18,11 @@ import static com.google.common.base.Preconditions.checkArgument;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableList;
+import com.google.gerrit.metrics.Description;
+import com.google.gerrit.metrics.Description.Units;
+import com.google.gerrit.metrics.Field;
+import com.google.gerrit.metrics.MetricMaker;
+import com.google.gerrit.metrics.Timer2;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.config.AllProjectsName;
 import com.google.gerrit.server.config.GerritServerConfig;
@@ -32,18 +37,22 @@ import java.util.ArrayList;
 import java.util.List;
 import org.eclipse.jgit.lib.Config;
 
-@SuppressWarnings("deprecation")
 @Singleton
 public class Sequences {
-  public static final String CHANGES = "changes";
+  public static final String NAME_CHANGES = "changes";
 
   public static int getChangeSequenceGap(Config cfg) {
     return cfg.getInt("noteDb", "changes", "initialSequenceGap", 1000);
   }
 
+  private enum SequenceType {
+    CHANGES;
+  }
+
   private final Provider<ReviewDb> db;
   private final NotesMigration migration;
   private final RepoSequence changeSeq;
+  private final Timer2<SequenceType, Boolean> nextIdLatency;
 
   @Inject
   Sequences(
@@ -51,30 +60,41 @@ public class Sequences {
       Provider<ReviewDb> db,
       NotesMigration migration,
       GitRepositoryManager repoManager,
-      AllProjectsName allProjects) {
+      AllProjectsName allProjects,
+      MetricMaker metrics) {
     this.db = db;
     this.migration = migration;
 
     int gap = getChangeSequenceGap(cfg);
-    changeSeq =
+    @SuppressWarnings("deprecation")
-        new RepoSequence(
+    RepoSequence.Seed seed = () -> db.get().nextChangeId() + gap;
-            repoManager,
+    int batchSize = cfg.getInt("noteDb", "changes", "sequenceBatchSize", 20);
-            allProjects,
+    changeSeq = new RepoSequence(repoManager, allProjects, NAME_CHANGES, seed, batchSize);
-            CHANGES,
+
-            () -> db.get().nextChangeId() + gap,
+    nextIdLatency =
-            cfg.getInt("noteDb", "changes", "sequenceBatchSize", 20));
+        metrics.newTimer(
+            "sequence/next_id_latency",
+            new Description("Latency of requesting IDs from repo sequences")
+                .setCumulative()
+                .setUnit(Units.MILLISECONDS),
+            Field.ofEnum(SequenceType.class, "sequence"),
+            Field.ofBoolean("multiple"));
   }
 
   public int nextChangeId() throws OrmException {
     if (!migration.readChangeSequence()) {
-      return db.get().nextChangeId();
+      return nextChangeId(db.get());
+    }
+    try (Timer2.Context timer = nextIdLatency.start(SequenceType.CHANGES, false)) {
+      return changeSeq.next();
     }
-    return changeSeq.next();
   }
 
   public ImmutableList<Integer> nextChangeIds(int count) throws OrmException {
     if (migration.readChangeSequence()) {
-      return changeSeq.next(count);
+      try (Timer2.Context timer = nextIdLatency.start(SequenceType.CHANGES, count > 1)) {
+        return changeSeq.next(count);
+      }
     }
 
     if (count == 0) {
@@ -84,7 +104,7 @@ public class Sequences {
     List<Integer> ids = new ArrayList<>(count);
     ReviewDb db = this.db.get();
     for (int i = 0; i < count; i++) {
-      ids.add(db.nextChangeId());
+      ids.add(nextChangeId(db));
     }
     return ImmutableList.copyOf(ids);
   }
@@ -93,4 +113,9 @@ public class Sequences {
   public RepoSequence getChangeIdRepoSequence() {
     return changeSeq;
   }
+
+  @SuppressWarnings("deprecation")
+  private static int nextChangeId(ReviewDb db) throws OrmException {
+    return db.nextChangeId();
+  }
 }

gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java

@@ -495,7 +495,7 @@ public class NoteDbMigrator implements AutoCloseable {
           new RepoSequence(
               repoManager,
               allProjects,
-              Sequences.CHANGES,
+              Sequences.NAME_CHANGES,
               // If sequenceGap is 0, this writes into the sequence ref the same ID that is returned
               // by the call to seq.next() below. If we actually used this as a change ID, that
               // would be a problem, but we just discard it, so this is safe.

gerrit-server/src/main/java/com/google/gerrit/server/schema/AllProjectsCreator.java

@@ -222,11 +222,11 @@ public class AllProjectsCreator {
 
   private void initSequences(Repository git, BatchRefUpdate bru) throws IOException {
     if (notesMigration.readChangeSequence()
-        && git.exactRef(REFS_SEQUENCES + Sequences.CHANGES) == null) {
+        && git.exactRef(REFS_SEQUENCES + Sequences.NAME_CHANGES) == null) {
       // Can't easily reuse the inserter from MetaDataUpdate, but this shouldn't slow down site
       // initialization unduly.
       try (ObjectInserter ins = git.newObjectInserter()) {
-        bru.addCommand(RepoSequence.storeNew(ins, Sequences.CHANGES, firstChangeId));
+        bru.addCommand(RepoSequence.storeNew(ins, Sequences.NAME_CHANGES, firstChangeId));
         ins.flush();
       }
     }