From 85bd0e6065d97319657535954d8a97c9ca161430 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Wed, 2 Nov 2016 14:38:31 +0100 Subject: [PATCH] SONAR-8325 Improve MeasureDao#selectByQuery performance It now accepts either a list of projects or a project with a list of components, but no more a list of components of any projects It's no more possible to query a list of any components, because when doing that the performance are very bad (it would probably requires to add a column PROJECT_MEASURES.PROJECT_UUID) --- .../measure/ws/ComponentTreeDataLoader.java | 4 +- .../sonar/server/measure/ws/SearchAction.java | 2 +- .../ws/SearchMyProjectsDataLoader.java | 2 +- .../qualitygate/ws/ProjectStatusAction.java | 3 +- .../java/org/sonar/db/measure/MeasureDao.java | 41 +++-- .../org/sonar/db/measure/MeasureQuery.java | 113 ++++++++++---- .../org/sonar/db/measure/MeasureMapper.xml | 80 +++++----- .../org/sonar/db/measure/MeasureDaoTest.java | 86 ++++++++--- .../sonar/db/measure/MeasureQueryTest.java | 140 ++++++++++++++++++ 9 files changed, 378 insertions(+), 93 deletions(-) create mode 100644 sonar-db/src/test/java/org/sonar/db/measure/MeasureQueryTest.java diff --git a/server/sonar-server/src/main/java/org/sonar/server/measure/ws/ComponentTreeDataLoader.java b/server/sonar-server/src/main/java/org/sonar/server/measure/ws/ComponentTreeDataLoader.java index 6304bc21367..8a2d059c215 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/measure/ws/ComponentTreeDataLoader.java +++ b/server/sonar-server/src/main/java/org/sonar/server/measure/ws/ComponentTreeDataLoader.java @@ -209,8 +209,8 @@ public class ComponentTreeDataLoader { Map metricsById = Maps.uniqueIndex(metrics, MetricDtoFunctions.toId()); MeasureQuery measureQuery = MeasureQuery.builder() .setPersonId(developerId) - .setComponentUuids(componentUuids) - .setMetricIds(metricsById.keySet()) + .setComponentUuids(baseComponent.projectUuid(), componentUuids) + .setMetricIds(new ArrayList<>(metricsById.keySet())) .build(); List measureDtos = dbClient.measureDao().selectByQuery(dbSession, measureQuery); diff --git a/server/sonar-server/src/main/java/org/sonar/server/measure/ws/SearchAction.java b/server/sonar-server/src/main/java/org/sonar/server/measure/ws/SearchAction.java index 61d6e70e5c8..2fc599297f0 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/measure/ws/SearchAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/measure/ws/SearchAction.java @@ -153,7 +153,7 @@ public class SearchAction implements MeasuresWsAction { private List searchMeasures() { return dbClient.measureDao().selectByQuery(dbSession, MeasureQuery.builder() - .setComponentUuids(projects.stream().map(ComponentDto::uuid).collect(toList())) + .setProjectUuids(projects.stream().map(ComponentDto::uuid).collect(toList())) .setMetricIds(metrics.stream().map(MetricDto::getId).collect(toList())) .build()); } diff --git a/server/sonar-server/src/main/java/org/sonar/server/project/ws/SearchMyProjectsDataLoader.java b/server/sonar-server/src/main/java/org/sonar/server/project/ws/SearchMyProjectsDataLoader.java index 879c9b3099d..648faeeb499 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/project/ws/SearchMyProjectsDataLoader.java +++ b/server/sonar-server/src/main/java/org/sonar/server/project/ws/SearchMyProjectsDataLoader.java @@ -62,7 +62,7 @@ public class SearchMyProjectsDataLoader { List snapshots = dbClient.snapshotDao().selectLastAnalysesByRootComponentUuids(dbSession, projectUuids); MetricDto gateStatusMetric = dbClient.metricDao().selectOrFailByKey(dbSession, CoreMetrics.ALERT_STATUS_KEY); MeasureQuery measureQuery = MeasureQuery.builder() - .setComponentUuids(projectUuids) + .setProjectUuids(projectUuids) .setMetricId(gateStatusMetric.getId()) .build(); List qualityGates = dbClient.measureDao().selectByQuery(dbSession, measureQuery); diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/ws/ProjectStatusAction.java b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/ws/ProjectStatusAction.java index 704474df243..fae8f46b7d4 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/ws/ProjectStatusAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/ws/ProjectStatusAction.java @@ -48,6 +48,7 @@ import org.sonarqube.ws.WsQualityGates.ProjectStatusWsResponse; import org.sonarqube.ws.client.qualitygate.ProjectStatusWsRequest; import static com.google.common.base.Strings.isNullOrEmpty; +import static java.util.Collections.singletonList; import static org.sonar.server.user.AbstractUserSession.insufficientPrivilegesException; import static org.sonar.server.ws.WsUtils.checkFoundWithOptional; import static org.sonar.server.ws.WsUtils.checkRequest; @@ -162,7 +163,7 @@ public class ProjectStatusAction implements QualityGatesWsAction { private Optional getQualityGateDetailsMeasureData(DbSession dbSession, ComponentDto project) { MeasureQuery measureQuery = MeasureQuery.builder() - .setComponentUuid(project.projectUuid()) + .setProjectUuids(singletonList(project.projectUuid())) .setMetricKey(CoreMetrics.QUALITY_GATE_DETAILS_KEY) .build(); List measures = dbClient.measureDao().selectByQuery(dbSession, measureQuery); diff --git a/sonar-db/src/main/java/org/sonar/db/measure/MeasureDao.java b/sonar-db/src/main/java/org/sonar/db/measure/MeasureDao.java index 0536a8389d6..c298bb18ff1 100644 --- a/sonar-db/src/main/java/org/sonar/db/measure/MeasureDao.java +++ b/sonar-db/src/main/java/org/sonar/db/measure/MeasureDao.java @@ -41,8 +41,11 @@ public class MeasureDao implements Dao { /** * Selects the measures of either the last analysis (when {@link MeasureQuery#analysisUuid} is {@code null}) or of the - * specified analysis (given by {@link MeasureQuery#analysisUuid}) for the component UUIDs specified in - * {@link MeasureQuery#componentUuids}. + * specified analysis (given by {@link MeasureQuery#analysisUuid}). + * The components can be specified either as : + * - A list of projects in {@link MeasureQuery#projectUuids} + * - A list of components in {@link MeasureQuery#componentUuids} with one mandatory project in {@link MeasureQuery#projectUuids} + * - One single component in {@link MeasureQuery#componentUuids} *

* In addition, this method returns measures which are not associated to any developer, unless one is specified in * {@link MeasureQuery#personId}. @@ -56,22 +59,29 @@ public class MeasureDao implements Dao { if (query.returnsEmpty()) { return Collections.emptyList(); } - if (query.getComponentUuids() == null) { - return mapper(dbSession).selectByQuery(query); + if (query.getComponentUuids() != null) { + return executeLargeInputs( + query.getComponentUuids(), + componentUuids -> { + MeasureQuery pageQuery = MeasureQuery.copyWithSubsetOfComponentUuids(query, componentUuids); + return mapper(dbSession).selectByQuery(pageQuery); + }); + } else if (query.getProjectUuids() != null) { + return executeLargeInputs( + query.getProjectUuids(), + projectUuids -> { + MeasureQuery pageQuery = MeasureQuery.copyWithSubsetOfProjectUuids(query, projectUuids); + return mapper(dbSession).selectByQuery(pageQuery); + }); } - return executeLargeInputs(query.getComponentUuids(), componentUuids -> { - MeasureQuery pageQuery = MeasureQuery.copyWithSubsetOfComponentUuids(query, componentUuids); - return mapper(dbSession).selectByQuery(pageQuery); - }); + return mapper(dbSession).selectByQuery(query); } public void selectByQuery(DbSession dbSession, MeasureQuery query, ResultHandler resultHandler) { if (query.returnsEmpty()) { return; } - if (query.getComponentUuids() == null) { - mapper(dbSession).selectByQuery(query, resultHandler); - } else { + if (query.getComponentUuids() != null) { executeLargeInputsWithoutOutput( query.getComponentUuids(), componentUuids -> { @@ -79,7 +89,16 @@ public class MeasureDao implements Dao { mapper(dbSession).selectByQuery(pageQuery, resultHandler); return null; }); + } else if (query.getProjectUuids() != null) { + executeLargeInputsWithoutOutput( + query.getProjectUuids(), + projectUuids -> { + MeasureQuery pageQuery = MeasureQuery.copyWithSubsetOfProjectUuids(query, projectUuids); + mapper(dbSession).selectByQuery(pageQuery, resultHandler); + return null; + }); } + mapper(dbSession).selectByQuery(query, resultHandler); } public List selectPastMeasures(DbSession dbSession, diff --git a/sonar-db/src/main/java/org/sonar/db/measure/MeasureQuery.java b/sonar-db/src/main/java/org/sonar/db/measure/MeasureQuery.java index 80b49427fcc..00b724df175 100644 --- a/sonar-db/src/main/java/org/sonar/db/measure/MeasureQuery.java +++ b/sonar-db/src/main/java/org/sonar/db/measure/MeasureQuery.java @@ -19,39 +19,50 @@ */ package org.sonar.db.measure; -import java.util.Collection; import java.util.List; import java.util.Objects; import javax.annotation.CheckForNull; import javax.annotation.Nullable; -import static com.google.common.base.Preconditions.checkState; -import static java.util.Collections.singleton; +import static com.google.common.base.Preconditions.checkArgument; import static java.util.Collections.singletonList; import static java.util.Objects.requireNonNull; public class MeasureQuery { private final String analysisUuid; + + @CheckForNull + private final List projectUuids; + + @CheckForNull private final List componentUuids; + @CheckForNull - private final Collection metricIds; + private final List metricIds; + @CheckForNull - private final Collection metricKeys; + private final List metricKeys; + @CheckForNull private final Long personId; private MeasureQuery(Builder builder) { - this(builder.analysisUuid, builder.componentUuids, builder.metricIds, builder.metricKeys, builder.personId); + this(builder.analysisUuid, builder.projectUuids, builder.componentUuids, builder.metricIds, builder.metricKeys, builder.personId); } private MeasureQuery(@Nullable String analysisUuid, - List componentUuids, - @Nullable Collection metricIds, - @Nullable Collection metricKeys, + @Nullable List projectUuids, + @Nullable List componentUuids, + @Nullable List metricIds, + @Nullable List metricKeys, @Nullable Long personId) { - requireNonNull(componentUuids, "Component UUIDs must be set"); - checkState(metricIds == null || metricKeys == null, "Metric IDs and keys must not be set both"); + checkArgument(metricIds == null || metricKeys == null, "Metric IDs and keys must not be set both"); + checkArgument(projectUuids != null || componentUuids != null, "At least one filter on component UUID is expected"); + checkArgument(componentUuids == null || componentUuids.size() == 1 || (projectUuids != null && projectUuids.size() == 1), + "Component UUIDs can only be used when a single project UUID is set"); + this.analysisUuid = analysisUuid; + this.projectUuids = projectUuids; this.componentUuids = componentUuids; this.metricIds = metricIds; this.metricKeys = metricKeys; @@ -62,17 +73,33 @@ public class MeasureQuery { return analysisUuid; } + @CheckForNull + public List getProjectUuids() { + return projectUuids; + } + + @CheckForNull + public String getProjectUuid() { + return isOnComponents() ? projectUuids.get(0) : null; + } + + @CheckForNull public List getComponentUuids() { return componentUuids; } @CheckForNull - public Collection getMetricIds() { + public String getComponentUuid() { + return isOnSingleComponent() ? componentUuids.get(0) : null; + } + + @CheckForNull + public List getMetricIds() { return metricIds; } @CheckForNull - public Collection getMetricKeys() { + public List getMetricKeys() { return metricKeys; } @@ -82,11 +109,24 @@ public class MeasureQuery { } public boolean returnsEmpty() { - return componentUuids.isEmpty() + return (projectUuids != null && projectUuids.isEmpty()) + || (componentUuids != null && componentUuids.isEmpty()) || (metricIds != null && metricIds.isEmpty()) || (metricKeys != null && metricKeys.isEmpty()); } + public boolean isOnProjects() { + return projectUuids != null && componentUuids == null; + } + + public boolean isOnComponents() { + return projectUuids != null && projectUuids.size() == 1 && componentUuids != null; + } + + public boolean isOnSingleComponent() { + return projectUuids == null && componentUuids != null && componentUuids.size() == 1; + } + @Override public boolean equals(@Nullable Object o) { if (this == o) { @@ -97,10 +137,11 @@ public class MeasureQuery { } MeasureQuery that = (MeasureQuery) o; return Objects.equals(analysisUuid, that.analysisUuid) && - Objects.equals(componentUuids, that.componentUuids) && - Objects.equals(metricIds, that.metricIds) && - Objects.equals(metricKeys, that.metricKeys) && - Objects.equals(personId, that.personId); + Objects.equals(projectUuids, that.projectUuids) && + Objects.equals(componentUuids, that.componentUuids) && + Objects.equals(metricIds, that.metricIds) && + Objects.equals(metricKeys, that.metricKeys) && + Objects.equals(personId, that.personId); } @Override @@ -112,15 +153,20 @@ public class MeasureQuery { return new Builder(); } + static MeasureQuery copyWithSubsetOfProjectUuids(MeasureQuery query, List projectUuids) { + return new MeasureQuery(query.analysisUuid, projectUuids, query.componentUuids, query.metricIds, query.metricKeys, query.personId); + } + static MeasureQuery copyWithSubsetOfComponentUuids(MeasureQuery query, List componentUuids) { - return new MeasureQuery(query.analysisUuid, componentUuids, query.metricIds, query.metricKeys, query.personId); + return new MeasureQuery(query.analysisUuid, query.projectUuids, componentUuids, query.metricIds, query.metricKeys, query.personId); } public static final class Builder { private String analysisUuid; + private List projectUuids; private List componentUuids; - private Collection metricIds; - private Collection metricKeys; + private List metricIds; + private List metricKeys; private Long personId; private Builder() { @@ -132,11 +178,26 @@ public class MeasureQuery { return this; } - public Builder setComponentUuids(List componentUuids) { + /** + * List of projects + */ + public Builder setProjectUuids(@Nullable List projectUuids) { + this.projectUuids = projectUuids; + return this; + } + + /** + * List of components of a project + */ + public Builder setComponentUuids(String projectUuid, List componentUuids) { + setProjectUuids(singletonList(requireNonNull(projectUuid))); this.componentUuids = componentUuids; return this; } + /** + * Single component + */ public Builder setComponentUuid(String componentUuid) { this.componentUuids = singletonList(componentUuid); return this; @@ -145,26 +206,26 @@ public class MeasureQuery { /** * All the measures are returned if parameter is {@code null}. */ - public Builder setMetricIds(@Nullable Collection metricIds) { + public Builder setMetricIds(@Nullable List metricIds) { this.metricIds = metricIds; return this; } public Builder setMetricId(int metricId) { - this.metricIds = singleton(metricId); + this.metricIds = singletonList(metricId); return this; } /** * All the measures are returned if parameter is {@code null}. */ - public Builder setMetricKeys(@Nullable Collection s) { + public Builder setMetricKeys(@Nullable List s) { this.metricKeys = s; return this; } public Builder setMetricKey(String s) { - this.metricKeys = singleton(s); + this.metricKeys = singletonList(s); return this; } diff --git a/sonar-db/src/main/resources/org/sonar/db/measure/MeasureMapper.xml b/sonar-db/src/main/resources/org/sonar/db/measure/MeasureMapper.xml index ab9bbcc31c5..0ed2c37a4ed 100644 --- a/sonar-db/src/main/resources/org/sonar/db/measure/MeasureMapper.xml +++ b/sonar-db/src/main/resources/org/sonar/db/measure/MeasureMapper.xml @@ -29,41 +29,55 @@ select from - project_measures pm - inner join projects p on p.uuid=pm.component_uuid - inner join snapshots analysis on analysis.component_uuid = p.project_uuid and analysis.uuid = pm.analysis_uuid - - inner join metrics m on m.id = pm.metric_id - + project_measures pm + inner join snapshots analysis on analysis.uuid = pm.analysis_uuid + + inner join projects p on p.project_uuid=analysis.component_uuid and p.uuid=pm.component_uuid + and p.project_uuid=#{query.projectUuid} + and p.uuid in + + #{componentUuid} + + + + inner join projects p on p.project_uuid=analysis.component_uuid and p.uuid=pm.component_uuid + and p.uuid=#{query.componentUuid} + + + inner join metrics m on m.id = pm.metric_id + where - - analysis.islast=${_true} - - - analysis.uuid = #{query.analysisUuid} - - and p.uuid in - - #{componentUuid} + + analysis.islast=${_true} + + + analysis.uuid = #{query.analysisUuid} + + + and analysis.component_uuid=pm.component_uuid + and analysis.component_uuid in + + #{projectUuid} - - and pm.metric_id in - #{metricId} - - - and m.name in - - #{metricKey} - - - - - and person_id = #{query.personId} - - - and person_id is null - - + + + and pm.metric_id in + #{metricId} + + + and m.name in + + #{metricKey} + + + + + and person_id = #{query.personId} + + + and person_id is null + +