opendev/gerrit
Code Issues Proposed changes

Merge "Instrument two possibly long-running access computations"

Browse Source
This commit is contained in:
Patrick Hiesel
2018-09-25 07:49:44 +00:00
committed by Gerrit Code Review
parent 51cb2ea2ff f71bafe885
commit 2d35882d95
4 changed files with 89 additions and 43 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
Documentation/metrics.txt
View File

@@ -138,6 +138,13 @@ failed by table.
* `notedb/read_all_external_ids_latency`: Latency for reading all
external ID's from NoteDb.
=== Permissions
* `permissions/project_state/computation_latency`: Latency to compute current access
sections on a project by traversing it's parents.
* `permissions/permission_collection/filter_latency`: Latency to filter access sections
by user and ref.
=== Reviewer Suggestion
* `reviewer_suggestion/query_accounts`: Latency for querying accounts for

84
java/com/google/gerrit/server/permissions/PermissionCollection.java
View File

@@ -26,6 +26,10 @@ import com.google.gerrit.common.data.AccessSection;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.common.data.PermissionRule;
import com.google.gerrit.common.data.PermissionRule.Action;
import com.google.gerrit.metrics.Description;
import com.google.gerrit.metrics.Description.Units;
import com.google.gerrit.metrics.MetricMaker;
import com.google.gerrit.metrics.Timer0;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.CurrentUser;
@@ -54,10 +58,18 @@ public class PermissionCollection {
@Singleton
public static class Factory {
private final SectionSortCache sorter;
// TODO(hiesel): Remove this once we got production data
private final Timer0 filterLatency;
@Inject
Factory(SectionSortCache sorter) {
Factory(SectionSortCache sorter, MetricMaker metricMaker) {
this.sorter = sorter;
this.filterLatency =
metricMaker.newTimer(
"permissions/permission_collection/filter_latency",
new Description("Latency for access filter computations in PermissionCollection")
.setCumulative()
.setUnit(Units.NANOSECONDS));
}
/**
@@ -117,41 +129,43 @@ public class PermissionCollection {
*/
PermissionCollection filter(
Iterable<SectionMatcher> matcherList, String ref, CurrentUser user) {
if (isRE(ref)) {
ref = RefPattern.shortestExample(ref);
} else if (ref.endsWith("/*")) {
ref = ref.substring(0, ref.length() - 1);
try (Timer0.Context ignored = filterLatency.start()) {
if (isRE(ref)) {
ref = RefPattern.shortestExample(ref);
} else if (ref.endsWith("/*")) {
ref = ref.substring(0, ref.length() - 1);
}
// LinkedHashMap to maintain input ordering.
Map<AccessSection, Project.NameKey> sectionToProject = new LinkedHashMap<>();
boolean perUser = filterRefMatchingSections(matcherList, ref, user, sectionToProject);
List<AccessSection> sections = Lists.newArrayList(sectionToProject.keySet());
// Sort by ref pattern specificity. For equally specific patterns, the sections from the
// project closer to the current one come first.
sorter.sort(ref, sections);
// For block permissions, we want a different order: first, we want to go from parent to
// child.
List<Map.Entry<AccessSection, Project.NameKey>> accessDescending =
Lists.reverse(Lists.newArrayList(sectionToProject.entrySet()));
Map<Project.NameKey, List<AccessSection>> accessByProject =
accessDescending
.stream()
.collect(
Collectors.groupingBy(
Map.Entry::getValue,
LinkedHashMap::new,
mapping(Map.Entry::getKey, toList())));
// Within each project, sort by ref specificity.
for (List<AccessSection> secs : accessByProject.values()) {
sorter.sort(ref, secs);
}
return new PermissionCollection(
Lists.newArrayList(accessByProject.values()), sections, perUser);
}
// LinkedHashMap to maintain input ordering.
Map<AccessSection, Project.NameKey> sectionToProject = new LinkedHashMap<>();
boolean perUser = filterRefMatchingSections(matcherList, ref, user, sectionToProject);
List<AccessSection> sections = Lists.newArrayList(sectionToProject.keySet());
// Sort by ref pattern specificity. For equally specific patterns, the sections from the
// project closer to the current one come first.
sorter.sort(ref, sections);
// For block permissions, we want a different order: first, we want to go from parent to
// child.
List<Map.Entry<AccessSection, Project.NameKey>> accessDescending =
Lists.reverse(Lists.newArrayList(sectionToProject.entrySet()));
Map<Project.NameKey, List<AccessSection>> accessByProject =
accessDescending
.stream()
.collect(
Collectors.groupingBy(
Map.Entry::getValue,
LinkedHashMap::new,
mapping(Map.Entry::getKey, toList())));
// Within each project, sort by ref specificity.
for (List<AccessSection> secs : accessByProject.values()) {
sorter.sort(ref, secs);
}
return new PermissionCollection(
Lists.newArrayList(accessByProject.values()), sections, perUser);
}
}

36
java/com/google/gerrit/server/project/ProjectState.java
View File

@@ -36,6 +36,11 @@ import com.google.gerrit.extensions.api.projects.ThemeInfo;
import com.google.gerrit.extensions.client.SubmitType;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.index.project.ProjectData;
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.Timer1;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.BooleanProjectConfig;
import com.google.gerrit.reviewdb.client.Branch;
@@ -92,6 +97,9 @@ public class ProjectState {
private final long globalMaxObjectSizeLimit;
private final boolean inheritProjectMaxObjectSizeLimit;
// TODO(hiesel): Remove this once we got production data
private final Timer1<String> computationLatency;
/** Last system time the configuration's revision was examined. */
private volatile long lastCheckGeneration;
@@ -119,6 +127,7 @@ public class ProjectState {
List<CommentLinkInfo> commentLinks,
CapabilityCollection.Factory limitsFactory,
TransferConfig transferConfig,
MetricMaker metricMaker,
@Assisted ProjectConfig config) {
this.sitePaths = sitePaths;
this.projectCache = projectCache;
@@ -136,6 +145,14 @@ public class ProjectState {
this.globalMaxObjectSizeLimit = transferConfig.getMaxObjectSizeLimit();
this.inheritProjectMaxObjectSizeLimit = transferConfig.getInheritProjectMaxObjectSizeLimit();
this.computationLatency =
metricMaker.newTimer(
"permissions/project_state/computation_latency",
new Description("Latency for access computations in ProjectState")
.setCumulative()
.setUnit(Units.NANOSECONDS),
Field.ofString("method"));
if (isAllProjects && !Permission.canBeOnAllProjects(AccessSection.ALL, Permission.OWNER)) {
localOwners = Collections.emptySet();
} else {
@@ -354,15 +371,20 @@ public class ProjectState {
* cached. Callers should try to cache this result per-request as much as possible.
*/
public List<SectionMatcher> getAllSections() {
if (isAllProjects) {
return getLocalAccessSections();
}
try (Timer1.Context ignored = computationLatency.start("getAllSections")) {
if (isAllProjects) {
return getLocalAccessSections();
}
List<SectionMatcher> all = new ArrayList<>();
for (ProjectState s : tree()) {
all.addAll(s.getLocalAccessSections());
List<SectionMatcher> all = new ArrayList<>();
Iterable<ProjectState> tree = tree();
try (Timer1.Context ignored2 = computationLatency.start("getAllSections-parsing-only")) {
for (ProjectState s : tree) {
all.addAll(s.getLocalAccessSections());
}
}
return all;
}
return all;
}
/**

5
javatests/com/google/gerrit/server/permissions/RefControlTest.java
View File

@@ -43,6 +43,7 @@ import com.google.gerrit.common.data.PermissionRange;
import com.google.gerrit.common.data.PermissionRule;
import com.google.gerrit.common.errors.InvalidNameException;
import com.google.gerrit.extensions.api.projects.CommentLinkInfo;
import com.google.gerrit.metrics.MetricMaker;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
@@ -204,6 +205,7 @@ public class RefControlTest {
@Inject private ThreadLocalRequestContext requestContext;
@Inject private DefaultRefFilter.Factory refFilterFactory;
@Inject private TransferConfig transferConfig;
@Inject private MetricMaker metricMaker;
@Before
public void setUp() throws Exception {
@@ -291,7 +293,7 @@ public class RefControlTest {
Cache<SectionSortCache.EntryKey, SectionSortCache.EntryVal> c =
CacheBuilder.newBuilder().build();
sectionSorter = new PermissionCollection.Factory(new SectionSortCache(c));
sectionSorter = new PermissionCollection.Factory(new SectionSortCache(c), metricMaker);
parent = new ProjectConfig(parentKey);
parent.load(newRepository(parentKey));
@@ -972,6 +974,7 @@ public class RefControlTest {
commentLinks,
capabilityCollectionFactory,
transferConfig,
metricMaker,
pc));
return repo;
}