Add latency metric to Sequences
The default choice of noteDb.changes.sequenceBatchSize=20 was not based
on data. Start collecting data.
Change-Id: I792557e6677fe9cbcc7943bf6fa9e1340ac941e7
diff --git a/Documentation/metrics.txt b/Documentation/metrics.txt
index 7440634..8cddce2 100644
--- a/Documentation/metrics.txt
+++ b/Documentation/metrics.txt
@@ -111,6 +111,10 @@
* `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
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/Sequences.java b/gerrit-server/src/main/java/com/google/gerrit/server/Sequences.java
index 391df11..010ed32 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/Sequences.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/Sequences.java
@@ -18,6 +18,11 @@
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 @@
@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 @@
Provider<ReviewDb> db,
NotesMigration migration,
GitRepositoryManager repoManager,
- AllProjectsName allProjects) {
+ AllProjectsName allProjects,
+ MetricMaker metrics) {
this.db = db;
this.migration = migration;
@@ -58,19 +69,32 @@
@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) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java
index a866314..bd1ccec 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java
@@ -438,7 +438,7 @@
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.
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/AllProjectsCreator.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/AllProjectsCreator.java
index 9a56aa4..dfcacb7 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/schema/AllProjectsCreator.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/AllProjectsCreator.java
@@ -222,11 +222,11 @@
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();
}
}
