Commit 41f87e9e authored by Thong Kuah's avatar Thong Kuah

Removes potentially incorrect, and slow fallback

Deployment_platform is relatively expensive and calling this after the
fact means that this may not be the cluster that was deployed to.

Correspondingly reduce the leeway given in the related N+1 spec
parent 7fa0c766
...@@ -44,18 +44,7 @@ class DeploymentMetrics ...@@ -44,18 +44,7 @@ class DeploymentMetrics
end end
end end
# TODO remove fallback case to deployment_platform_cluster.
# Otherwise we will continue to pay the performance penalty described in
# https://gitlab.com/gitlab-org/gitlab-ce/issues/63475
#
# Removal issue: https://gitlab.com/gitlab-org/gitlab-ce/issues/64105
def cluster_prometheus def cluster_prometheus
cluster_with_fallback = cluster || deployment_platform_cluster cluster.application_prometheus if cluster&.application_prometheus_available?
cluster_with_fallback.application_prometheus if cluster_with_fallback&.application_prometheus_available?
end
def deployment_platform_cluster
deployment.environment.deployment_platform&.cluster
end end
end end
---
title: Remove incorrect fallback when determining which cluster to use when retrieving
MR performance metrics
merge_request: 31126
author:
type: changed
...@@ -885,10 +885,9 @@ describe Projects::MergeRequestsController do ...@@ -885,10 +885,9 @@ describe Projects::MergeRequestsController do
environment2 = create(:environment, project: forked) environment2 = create(:environment, project: forked)
create(:deployment, :succeed, environment: environment2, sha: sha, ref: 'master', deployable: build) create(:deployment, :succeed, environment: environment2, sha: sha, ref: 'master', deployable: build)
# TODO address the last 11 queries # TODO address the last 5 queries
# See https://gitlab.com/gitlab-org/gitlab-ce/issues/63952 (5 queries) # See https://gitlab.com/gitlab-org/gitlab-ce/issues/63952 (5 queries)
# And https://gitlab.com/gitlab-org/gitlab-ce/issues/64105 (6 queries) leeway = 5
leeway = 11
expect { get_ci_environments_status }.not_to exceed_all_query_limit(control_count + leeway) expect { get_ci_environments_status }.not_to exceed_all_query_limit(control_count + leeway)
end end
end end
......
...@@ -49,18 +49,6 @@ describe DeploymentMetrics do ...@@ -49,18 +49,6 @@ describe DeploymentMetrics do
it { is_expected.to be_truthy } it { is_expected.to be_truthy }
end end
context 'fallback deployment platform' do
let(:cluster) { create(:cluster, :provided_by_user, environment_scope: '*', projects: [deployment.project]) }
let!(:prometheus) { create(:clusters_applications_prometheus, :installed, cluster: cluster) }
before do
expect(deployment.project).to receive(:deployment_platform).and_return(cluster.platform)
expect(cluster.application_prometheus).to receive(:can_query?).and_return(true)
end
it { is_expected.to be_truthy }
end
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment