ProjectControl: Trace time for canPerformOnAllRefs
From metric data we see that checking project permissions has a much
higher p99 latency than checking change permissions. The metric data
doesn't tell us which project permission check is slow, but we suspect
that it's checking the READ permission that triggers
canPerformOnAllRefs. canPerformOnAllRefs is known to be slow since it
iterates over all ref pattern. By having a performance log for this we
can see how badly it impacts the latency of requests.
Release-Notes: skip
Bug: Google b/335095952
Change-Id: Ie4d606514f422007a54b42f4a8a8505d23d9677a
Signed-off-by: Edwin Kempin <ekempin@google.com>
diff --git a/java/com/google/gerrit/server/permissions/ProjectControl.java b/java/com/google/gerrit/server/permissions/ProjectControl.java
index fab894e..466dd92 100644
--- a/java/com/google/gerrit/server/permissions/ProjectControl.java
+++ b/java/com/google/gerrit/server/permissions/ProjectControl.java
@@ -42,6 +42,9 @@
import com.google.gerrit.server.config.GitUploadPackGroups;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.group.SystemGroupBackend;
+import com.google.gerrit.server.logging.Metadata;
+import com.google.gerrit.server.logging.TraceContext;
+import com.google.gerrit.server.logging.TraceContext.TraceTimer;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.permissions.PermissionBackend.ForChange;
import com.google.gerrit.server.permissions.PermissionBackend.ForProject;
@@ -290,24 +293,29 @@
}
private boolean canPerformOnAllRefs(String permission, Set<String> ignore) {
- boolean canPerform = false;
- Set<String> patterns = allRefPatterns(permission);
- if (patterns.contains(ALL)) {
- // Only possible if granted on the pattern that
- // matches every possible reference. Check all
- // patterns also have the permission.
- //
- for (String pattern : patterns) {
- if (controlForRef(pattern).canPerform(permission)) {
- canPerform = true;
- } else if (ignore.contains(pattern)) {
- continue;
- } else {
- return false;
+ try (TraceTimer timer =
+ TraceContext.newTimer(
+ "ProjectControl#canPerformOnAllRefs",
+ Metadata.builder().projectName(getProject().getName()).build())) {
+ boolean canPerform = false;
+ Set<String> patterns = allRefPatterns(permission);
+ if (patterns.contains(ALL)) {
+ // Only possible if granted on the pattern that
+ // matches every possible reference. Check all
+ // patterns also have the permission.
+ //
+ for (String pattern : patterns) {
+ if (controlForRef(pattern).canPerform(permission)) {
+ canPerform = true;
+ } else if (ignore.contains(pattern)) {
+ continue;
+ } else {
+ return false;
+ }
}
}
+ return canPerform;
}
- return canPerform;
}
private Set<String> allRefPatterns(String permissionName) {
