Add latency metric to Sequences

The default choice of noteDb.changes.sequenceBatchSize=20 was not based
on data. Start collecting data.

Change-Id: I792557e6677fe9cbcc7943bf6fa9e1340ac941e7

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;
@@ -34,15 +39,20 @@ import org.eclipse.jgit.lib.Config;
 
 @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(
@@ -50,7 +60,8 @@ public class Sequences {
       Provider<ReviewDb> db,
       NotesMigration migration,
       GitRepositoryManager repoManager,
-      AllProjectsName allProjects) {
+      AllProjectsName allProjects,
+      MetricMaker metrics) {
     this.db = db;
     this.migration = migration;
 
@@ -58,19 +69,32 @@ public class Sequences {
     @SuppressWarnings("deprecation")
     RepoSequence.Seed seed = () -> db.get().nextChangeId() + gap;
     int batchSize = cfg.getInt("noteDb", "changes", "sequenceBatchSize", 20);
-    changeSeq = new RepoSequence(repoManager, allProjects, CHANGES, seed, batchSize);
+    changeSeq = new RepoSequence(repoManager, allProjects, NAME_CHANGES, seed, batchSize);
+
+    nextIdLatency =
+        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 nextChangeId(db.get());
     }
-    return changeSeq.next();
+    try (Timer2.Context timer = nextIdLatency.start(SequenceType.CHANGES, false)) {
+      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) {

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

@@ -438,7 +438,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();
       }
     }