Skip to content

G
gitlab-ce
Commit aad1b85d authored by David Fernandez's avatar David Fernandez
Browse files
Options

Remove the packages_remove_cross_joins_to_pipelines feature flag 

Removes cross join queries between package tables and the ci_pipelines
table.

Changelog: other
parent 8591f104
Hide whitespace changes
Inline Side-by-side
Showing with 7 additions and 192 deletions
app/finders/packages/group_packages_finder.rb
View file @ aad1b85d
......@@ -22,7 +22,7 @@ module Packages
def packages_for_group_projects(installable_only: false)
packages = ::Packages::Package
.load_pipelines
.preload_pipelines
.including_project_route
.including_tags
.for_projects(group_projects_visible_to_current_user.select(:id))
......
app/finders/packages/package_finder.rb
View file @ aad1b85d
......@@ -9,7 +9,7 @@ module Packages
def execute
@project
.packages
.load_pipelines
.preload_pipelines
.including_project_route
.including_tags
.displayable
......
app/finders/packages/packages_finder.rb
View file @ aad1b85d
......@@ -14,7 +14,7 @@ module Packages
def execute
packages = project.packages
.load_pipelines
.preload_pipelines
.including_project_route
.including_tags
......
app/models/packages/package.rb
View file @ aad1b85d
......@@ -40,7 +40,7 @@ class Packages::Package < ApplicationRecord
has_one :composer_metadatum, inverse_of: :package, class_name: 'Packages::Composer::Metadatum'
has_one :rubygems_metadatum, inverse_of: :package, class_name: 'Packages::Rubygems::Metadatum'
has_many :build_infos, inverse_of: :package
has_many :pipelines, through: :build_infos, disable_joins: -> { disable_cross_joins_to_pipelines? }
has_many :pipelines, through: :build_infos, disable_joins: true
has_one :debian_publication, inverse_of: :package, class_name: 'Packages::Debian::Publication'
has_one :debian_distribution, through: :debian_publication, source: :distribution, inverse_of: :packages, class_name: 'Packages::Debian::ProjectDistribution'
......@@ -102,7 +102,6 @@ class Packages::Package < ApplicationRecord
scope :with_status, ->(status) { where(status: status) }
scope :displayable, -> { with_status(DISPLAYABLE_STATUSES) }
scope :installable, -> { with_status(INSTALLABLE_STATUSES) }
scope :including_build_info, -> { includes(pipelines: :user) }
scope :including_project_route, -> { includes(project: { namespace: :route }) }
scope :including_tags, -> { includes(:tags) }
scope :including_dependency_links, -> { includes(dependency_links: :dependency) }
......@@ -135,7 +134,6 @@ class Packages::Package < ApplicationRecord
scope :last_of_each_version, -> { where(id: all.select('MAX(id) AS id').group(:version)) }
scope :limit_recent, ->(limit) { order_created_desc.limit(limit) }
scope :select_distinct_name, -> { select(:name).distinct }
scope :load_pipelines, -> { disable_cross_joins_to_pipelines? ? preload_pipelines : including_build_info }
# Sorting
scope :order_created, -> { reorder(created_at: :asc) }
......@@ -162,10 +160,6 @@ class Packages::Package < ApplicationRecord
joins(:project).reorder(keyset_order)
end
def self.disable_cross_joins_to_pipelines?
::Feature.enabled?(:packages_remove_cross_joins_to_pipelines, default_enabled: :yaml)
end
def self.only_maven_packages_with_path(path, use_cte: false)
if use_cte
# This is an optimization fence which assumes that looking up the Metadatum record by path (globally)
......@@ -251,7 +245,7 @@ class Packages::Package < ApplicationRecord
def versions
project.packages
.load_pipelines
.preload_pipelines
.including_tags
.with_name(name)
.where.not(version: version)
......
app/models/packages/package_file.rb
View file @ aad1b85d
......@@ -15,7 +15,7 @@ class Packages::PackageFile < ApplicationRecord
has_one :conan_file_metadatum, inverse_of: :package_file, class_name: 'Packages::Conan::FileMetadatum'
has_many :package_file_build_infos, inverse_of: :package_file, class_name: 'Packages::PackageFileBuildInfo'
has_many :pipelines, through: :package_file_build_infos, disable_joins: -> { Packages::Package.disable_cross_joins_to_pipelines? }
has_many :pipelines, through: :package_file_build_infos, disable_joins: true
has_one :debian_file_metadatum, inverse_of: :package_file, class_name: 'Packages::Debian::FileMetadatum'
has_one :helm_file_metadatum, inverse_of: :package_file, class_name: 'Packages::Helm::FileMetadatum'
......
config/feature_flags/development/packages_remove_cross_joins_to_pipelines.yml deleted 100644 → 0
View file @ 8591f104
---
name: packages_remove_cross_joins_to_pipelines
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/70624
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/342921
milestone: '14.5'
type: development
group: group::package
default_enabled: false
lib/api/package_files.rb
View file @ aad1b85d
......@@ -29,7 +29,7 @@ module API
.new(user_project, params[:package_id]).execute
files = package.package_files
files = files.preload_pipelines if Packages::Package.disable_cross_joins_to_pipelines?
.preload_pipelines
present paginate(files), with: ::API::Entities::PackageFile
end
......
spec/finders/packages/group_packages_finder_spec.rb
View file @ aad1b85d
......@@ -160,30 +160,6 @@ RSpec.describe Packages::GroupPackagesFinder do
end
end
context 'with pipelines' do
let_it_be(:build_info_1) { create(:package_build_info, :with_pipeline, package: package1) }
let_it_be(:build_info_2) { create(:package_build_info, :with_pipeline, package: package2) }
it 'preloads the pipelines' do
expect(::Packages::Package).to receive(:preload_pipelines).and_call_original
expect(::Packages::Package).not_to receive(:including_build_info)
expect(subject).to match_array([package1, package2])
end
context 'with packages_remove_cross_joins_to_pipelines disabled' do
before do
stub_feature_flags(packages_remove_cross_joins_to_pipelines: false)
end
it 'includes the pipelines' do
expect(::Packages::Package).to receive(:including_build_info).and_call_original
expect(::Packages::Package).not_to receive(:preload_pipelines)
expect(subject).to match_array([package1, package2])
end
end
end
it_behaves_like 'concerning versionless param'
it_behaves_like 'concerning package statuses'
end
......
spec/finders/packages/package_finder_spec.rb
View file @ aad1b85d
......@@ -32,29 +32,5 @@ RSpec.describe ::Packages::PackageFinder do
expect { subject }.to raise_exception(ActiveRecord::RecordNotFound)
end
end
context 'with pipelines' do
let_it_be(:build_info) { create(:package_build_info, :with_pipeline, package: maven_package) }
it 'preloads the pipelines' do
expect(::Packages::Package).to receive(:preload_pipelines).and_call_original
expect(::Packages::Package).not_to receive(:including_build_info)
expect(subject).to eq(maven_package)
end
context 'with packages_remove_cross_joins_to_pipelines disabled' do
before do
stub_feature_flags(packages_remove_cross_joins_to_pipelines: false)
end
it 'includes the pipelines' do
expect(::Packages::Package).to receive(:including_build_info).and_call_original
expect(::Packages::Package).not_to receive(:preload_pipelines)
expect(subject).to eq(maven_package)
end
end
end
end
end
spec/finders/packages/packages_finder_spec.rb
View file @ aad1b85d
......@@ -81,31 +81,6 @@ RSpec.describe ::Packages::PackagesFinder do
it { is_expected.to match_array([conan_package, maven_package]) }
end
context 'with pipelines' do
let_it_be(:build_info1) { create(:package_build_info, :with_pipeline, package: conan_package) }
let_it_be(:build_info2) { create(:package_build_info, :with_pipeline, package: maven_package) }
it 'preloads the pipelines' do
expect(::Packages::Package).to receive(:preload_pipelines).and_call_original
expect(::Packages::Package).not_to receive(:including_build_info)
expect(subject).to match_array([conan_package, maven_package])
end
context 'with packages_remove_cross_joins_to_pipelines disabled' do
before do
stub_feature_flags(packages_remove_cross_joins_to_pipelines: false)
end
it 'includes the pipelines' do
expect(::Packages::Package).to receive(:including_build_info).and_call_original
expect(::Packages::Package).not_to receive(:preload_pipelines)
expect(subject).to match_array([conan_package, maven_package])
end
end
end
it_behaves_like 'concerning versionless param'
it_behaves_like 'concerning package statuses'
end
......
spec/models/packages/package_spec.rb
View file @ aad1b85d
......@@ -961,33 +961,6 @@ RSpec.describe Packages::Package, type: :model do
end
end
describe '.load_pipelines' do
let_it_be(:package) { create(:maven_package) }
let_it_be(:build_info) { create(:package_build_info, :with_pipeline, package: package) }
subject { described_class.load_pipelines }
it 'uses preload', :aggregate_failures do
expect(described_class).to receive(:preload_pipelines).and_call_original
expect(described_class).not_to receive(:including_build_info)
subject
end
context 'with packages_remove_cross_joins_to_pipelines disabled' do
before do
stub_feature_flags(packages_remove_cross_joins_to_pipelines: false)
end
it 'uses includes', :aggregate_failures do
expect(described_class).to receive(:including_build_info).and_call_original
expect(described_class).not_to receive(:preload_pipelines)
subject
end
end
end
describe '#versions' do
let_it_be(:project) { create(:project) }
let_it_be(:package) { create(:maven_package, project: project) }
......@@ -1001,30 +974,6 @@ RSpec.describe Packages::Package, type: :model do
it 'does not return different packages' do
expect(package.versions).not_to include(package3)
end
context 'with pipelines' do
let_it_be(:build_info) { create(:package_build_info, :with_pipeline, package: package2) }
it 'preloads the pipelines' do
expect(::Packages::Package).to receive(:preload_pipelines).and_call_original
expect(::Packages::Package).not_to receive(:including_build_info)
expect(package.versions).to contain_exactly(package2)
end
context 'with packages_remove_cross_joins_to_pipelines disabled' do
before do
stub_feature_flags(packages_remove_cross_joins_to_pipelines: false)
end
it 'includes the pipelines' do
expect(::Packages::Package).to receive(:including_build_info).and_call_original
expect(::Packages::Package).not_to receive(:preload_pipelines)
expect(package.versions).to contain_exactly(package2)
end
end
end
end
describe '#pipeline' do
......
spec/requests/api/package_files_spec.rb
View file @ aad1b85d
......@@ -27,34 +27,6 @@ RSpec.describe API::PackageFiles do
expect(response).to have_gitlab_http_status(:not_found)
end
context 'with pipelines' do
let(:package_file_build_info) { create(:package_file_build_info, :with_pipeline, package_file: package.package_files.first) }
it 'preloads the pipelines' do
expect(::Packages::PackageFile).to receive(:preload_pipelines).and_call_original
get api(url)
expect(response).to have_gitlab_http_status(:ok)
end
context 'with packages_remove_cross_joins_to_pipelines disabled' do
before do
stub_feature_flags(packages_remove_cross_joins_to_pipelines: false)
end
it 'does not preload the pipelines' do
expect(::Packages::PackageFile).not_to receive(:preload_pipelines)
::Gitlab::Database.allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/342921') do
get api(url)
end
expect(response).to have_gitlab_http_status(:ok)
end
end
end
end
context 'project is private' do
......
spec/support/shared_examples/requests/api/graphql/packages/package_details_shared_examples.rb
View file @ aad1b85d
......@@ -13,25 +13,6 @@ RSpec.shared_examples 'a package detail' do
it_behaves_like 'a working graphql query' do
it_behaves_like 'matching the package details schema'
end
context 'with packages_remove_cross_joins_to_pipelines disabled' do
# subject is called in a before callback that is executed before the one below
# the callback below MUST be executed before the subject
# solution: nullify subject and manually call #post_graphql
subject {}
before do
stub_feature_flags(packages_remove_cross_joins_to_pipelines: false)
::Gitlab::Database.allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/342921') do
post_graphql(query, current_user: user)
end
end
it_behaves_like 'a working graphql query' do
it_behaves_like 'matching the package details schema'
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7