diff --git a/app/finders/ci/runners_finder.rb b/app/finders/ci/runners_finder.rb index 8bc2a47a024fd9fc13b66a668fb0db04e7e4d92c..b38db61abdde80671875c8c7ede862c533cb6926 100644 --- a/app/finders/ci/runners_finder.rb +++ b/app/finders/ci/runners_finder.rb @@ -53,8 +53,7 @@ def group_runners Ci::Runner.belonging_to_group(@group.id) when :descendants, nil # Getting all runners from the group itself and all its descendant groups/projects - descendant_projects = Project.for_group_and_its_subgroups(@group) - Ci::Runner.belonging_to_group_or_project(@group.self_and_descendants, descendant_projects) + Ci::Runner.belonging_to_group_or_descendants(@group) else raise ArgumentError, 'Invalid membership filter' end diff --git a/app/models/ci/namespace_hierarchy.rb b/app/models/ci/namespace_hierarchy.rb new file mode 100644 index 0000000000000000000000000000000000000000..c48c7909055a00d380062b0e18f47e4f2820043c --- /dev/null +++ b/app/models/ci/namespace_hierarchy.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +module Ci + # This model represents a shadow table of the main database's namespaces table (though only represents namespaces containing runners). + # It allows us to navigate the namespace hierarchy on the ci database without resorting to a JOIN. + class NamespaceHierarchy < ApplicationRecord + scope :belonging_to_namespace, -> (namespace_id) { + return where('array_length(traversal_ids, 1) = 1') unless namespace_id + + where('traversal_ids && ARRAY[?]::int[]', namespace_id) + } + + class << self + def update_traversal_ids(old_self_and_ancestor_ids, new_self_and_ancestor_ids) + where('traversal_ids @> ARRAY[?]::int[]', old_self_and_ancestor_ids) + .update_all("traversal_ids = ARRAY#{new_self_and_ancestor_ids}::int[] || traversal_ids[#{old_self_and_ancestor_ids.length + 1}:]") + end + + def delete_traversal_ids(namespace_id) + belonging_to_namespace(namespace_id).delete_all + end + end + end +end diff --git a/app/models/ci/project_hierarchy.rb b/app/models/ci/project_hierarchy.rb new file mode 100644 index 0000000000000000000000000000000000000000..39e564db1b06b9712ac003d2c679918fb87c0e7b --- /dev/null +++ b/app/models/ci/project_hierarchy.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +module Ci + # This model represents a shadow table of the main database's namespaces table (though only represents projects containing runners). + # It allows us to navigate the namespace hierarchy on the ci database for a project without resorting to a JOIN. + class ProjectHierarchy < ApplicationRecord + scope :belonging_to_namespace, -> (namespace_id) { + where('traversal_ids && ARRAY[?]::int[]', namespace_id) + } + + class << self + def update_project_traversal_ids(project_id, old_self_and_ancestor_ids, new_self_and_ancestor_ids) + update_all_traversal_ids(where(project_xid: project_id), old_self_and_ancestor_ids, new_self_and_ancestor_ids) + end + + def delete_project_traversal_ids(project_id) + where(project_xid: project_id).delete_all + end + + def update_namespace_traversal_ids(old_self_and_ancestor_ids, new_self_and_ancestor_ids) + update_all_traversal_ids(belonging_to_namespace(old_self_and_ancestor_ids), old_self_and_ancestor_ids, new_self_and_ancestor_ids) + end + + private + + def update_all_traversal_ids(assoc, old_self_and_ancestor_ids, new_self_and_ancestor_ids) + assoc.update_all("traversal_ids = ARRAY#{new_self_and_ancestor_ids}::int[] || traversal_ids[#{old_self_and_ancestor_ids.length + 1}:]") + end + end + end +end diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb index 4c5e4837d0687af54b1effa975073ed3e3ff3648..afc9d30d54ca0c71db89e573ecb7574412697a43 100644 --- a/app/models/ci/runner.rb +++ b/app/models/ci/runner.rb @@ -85,13 +85,13 @@ class Runner < Ci::ApplicationRecord joins(:runner_namespaces).where(ci_runner_namespaces: { namespace_id: groups }) } - scope :belonging_to_group_or_project, -> (group_id, project_id) { - groups = ::Group.where(id: group_id) + scope :belonging_to_group_or_descendants, -> (group_id) { + groups = ::Group.where(id: group_id).ids - group_runners = joins(:runner_namespaces).where(ci_runner_namespaces: { namespace_id: groups }) - project_runners = joins(:runner_projects).where(ci_runner_projects: { project_id: project_id }) + runners_on_groups = joins(:runner_namespaces).where(ci_runner_namespaces: { namespace_id: Ci::NamespaceHierarchy.belonging_to_namespace(groups).select(:namespace_xid) }) + runners_on_projects = belonging_to_project(Ci::ProjectHierarchy.belonging_to_namespace(groups).select(:project_xid)) - union_sql = ::Gitlab::SQL::Union.new([group_runners, project_runners]).to_sql + union_sql = ::Gitlab::SQL::Union.new([runners_on_groups, runners_on_projects]).to_sql from("(#{union_sql}) #{table_name}") } diff --git a/app/models/ci/runner_namespace.rb b/app/models/ci/runner_namespace.rb index d1353b97ed9af950a39452bfb2caf74b1187d7f7..d128d6b115983aa5259a71721952f27c26edde06 100644 --- a/app/models/ci/runner_namespace.rb +++ b/app/models/ci/runner_namespace.rb @@ -14,6 +14,9 @@ class RunnerNamespace < Ci::ApplicationRecord belongs_to :namespace, inverse_of: :runner_namespaces, class_name: '::Namespace' belongs_to :group, class_name: '::Group', foreign_key: :namespace_id + after_create :insert_namespace_hierarchy + after_destroy :delete_namespace_hierarchy + validates :runner_id, uniqueness: { scope: :namespace_id } validate :group_runner_type @@ -26,5 +29,21 @@ def recent_runners def group_runner_type errors.add(:runner, 'is not a group runner') unless runner&.group_type? end + + def insert_namespace_hierarchy + Ci::NamespaceHierarchy.insert( + { + namespace_xid: namespace_id, + traversal_ids: namespace.self_and_ancestor_ids(hierarchy_order: :desc) + }, + unique_by: :namespace_xid + ) + end + + def delete_namespace_hierarchy + Ci::NamespaceHierarchy + .where(namespace_xid: namespace_id) + .delete_by('NOT EXISTS(?)', self.class.select(1).where(namespace_id: namespace_id)) + end end end diff --git a/app/models/ci/runner_project.rb b/app/models/ci/runner_project.rb index e1c435e9b1f0e7715f56d8762d3358589b2eb86c..d4874f69a7be47416c430a86630a38a42523b67c 100644 --- a/app/models/ci/runner_project.rb +++ b/app/models/ci/runner_project.rb @@ -13,10 +13,31 @@ class RunnerProject < Ci::ApplicationRecord belongs_to :runner, inverse_of: :runner_projects belongs_to :project, inverse_of: :runner_projects + after_create :insert_project_hierarchy + after_destroy :delete_project_hierarchy + + validates :runner_id, uniqueness: { scope: :project_id } + def recent_runners ::Ci::Runner.belonging_to_project(project_id).recent end - validates :runner_id, uniqueness: { scope: :project_id } + private + + def insert_project_hierarchy + Ci::ProjectHierarchy.insert( + { + project_xid: project_id, + traversal_ids: project.namespace&.self_and_ancestor_ids(hierarchy_order: :desc) + }, + unique_by: :project_xid + ) + end + + def delete_project_hierarchy + Ci::ProjectHierarchy + .where(project_xid: project_id) + .delete_by('NOT EXISTS(?)', self.class.select(1).where(project_id: project_id)) + end end end diff --git a/app/models/namespace.rb b/app/models/namespace.rb index 3ffd377251cd50380ed1afc94602a1b96a703272..eedad8e1d9e3a51265cceb548c678e52bf2fe6a0 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -90,6 +90,8 @@ class Namespace < ApplicationRecord before_create :sync_share_with_group_lock_with_parent before_update :sync_share_with_group_lock_with_parent, if: :parent_changed? after_update :force_share_with_group_lock_on_descendants, if: -> { saved_change_to_share_with_group_lock? && share_with_group_lock? } + after_update :update_ci_namespace_hierarchy, if: :saved_change_to_parent_id? + after_destroy :delete_ci_namespace_hierarchy # Legacy Storage specific hooks @@ -569,6 +571,23 @@ def force_share_with_group_lock_on_descendants .update_all(share_with_group_lock: true) end + def update_ci_namespace_hierarchy + old_parent = Namespace.find(parent_id_previously_was) if parent_id_previously_was + + old_traversal_ids = traversal_ids_from_parent_namespace(old_parent) + new_traversal_ids = traversal_ids_from_parent_namespace(parent) + ::Ci::NamespaceHierarchy.update_traversal_ids(old_traversal_ids, new_traversal_ids) + ::Ci::ProjectHierarchy.update_namespace_traversal_ids(old_traversal_ids, new_traversal_ids) + end + + def traversal_ids_from_parent_namespace(namespace) + (namespace&.self_and_ancestor_ids(hierarchy_order: :desc) || []) + [id] + end + + def delete_ci_namespace_hierarchy + ::Ci::NamespaceHierarchy.delete_traversal_ids(id) + end + def write_projects_repository_config all_projects.find_each do |project| project.set_full_path diff --git a/app/models/project.rb b/app/models/project.rb index 126fcc8d7e65583bda1b6802c0e9f3fa14457002..e45f0ff346ef4f01ef5236766c4b7953cddb0c9a 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -99,6 +99,9 @@ class Project < ApplicationRecord before_save :ensure_runners_token + after_update :update_ci_project_hierarchy, if: :saved_change_to_namespace_id? + after_destroy :delete_ci_project_hierarchy + after_save :update_project_statistics, if: :saved_change_to_namespace_id? after_save :create_import_state, if: ->(project) { project.import? && project.import_state.nil? } @@ -1719,6 +1722,20 @@ def update_forks_visibility_level end end + def update_ci_project_hierarchy + prev_namespace = Namespace.find_by(id: namespace_id_previously_was) + + ::Ci::ProjectHierarchy.update_project_traversal_ids( + project_id, + prev_namespace&.self_and_ancestor_ids || [namespace_id_previously_was], + namespace.self_and_ancestor_ids + ) + end + + def delete_ci_project_hierarchy + ::Ci::ProjectHierarchy.delete_project_traversal_ids(project_id) + end + def allowed_to_share_with_group? !namespace.share_with_group_lock end diff --git a/db/migrate/20210920131736_create_ci_namespace_hierarchies.rb b/db/migrate/20210920131736_create_ci_namespace_hierarchies.rb new file mode 100644 index 0000000000000000000000000000000000000000..f57b4a8f215c54a08bb03110e63100139f25026e --- /dev/null +++ b/db/migrate/20210920131736_create_ci_namespace_hierarchies.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class CreateCiNamespaceHierarchies < Gitlab::Database::Migration[1.0] + TABLE_NAME = :ci_namespace_hierarchies + INDEX_NAME = "index_gin_#{TABLE_NAME}" + + def change + create_table TABLE_NAME, id: false, primary_key: :namespace_xid do |t| + t.integer :namespace_xid, null: false, primary_key: true, default: nil + t.integer :traversal_ids, array: true, default: [], null: false + + t.index :traversal_ids, name: INDEX_NAME, using: :gin + end + end +end diff --git a/db/migrate/20210920134647_add_ci_namespace_hierarchies.rb b/db/migrate/20210920134647_add_ci_namespace_hierarchies.rb new file mode 100644 index 0000000000000000000000000000000000000000..cccb9c53d5879b70cde880a08d35fdc271bee1ac --- /dev/null +++ b/db/migrate/20210920134647_add_ci_namespace_hierarchies.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +class AddCiNamespaceHierarchies < Gitlab::Database::Migration[1.0] + TABLE_NAME = :ci_namespace_hierarchies + + disable_ddl_transaction! + + def up + with_lock_retries do + execute(<<~SQL) + INSERT INTO #{TABLE_NAME} (namespace_xid, traversal_ids) + SELECT + namespace_id AS namespace_xid, + namespaces.traversal_ids AS traversal_ids + FROM + ci_runner_namespaces + INNER JOIN namespaces ON ci_runner_namespaces.namespace_id = namespaces.id + ON CONFLICT(namespace_xid) DO NOTHING + SQL + end + end + + def down + with_lock_retries do + execute(<<~SQL) + DELETE FROM #{TABLE_NAME} + SQL + end + end +end diff --git a/db/migrate/20210921095325_create_ci_project_hierarchies.rb b/db/migrate/20210921095325_create_ci_project_hierarchies.rb new file mode 100644 index 0000000000000000000000000000000000000000..c99fade7c8402a5bd4df82e579ed3195ff87f061 --- /dev/null +++ b/db/migrate/20210921095325_create_ci_project_hierarchies.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class CreateCiProjectHierarchies < Gitlab::Database::Migration[1.0] + TABLE_NAME = :ci_project_hierarchies + INDEX_NAME = "index_gin_#{TABLE_NAME}" + + def change + create_table TABLE_NAME, id: false, primary_key: :project_xid do |t| + t.integer :project_xid, null: false, primary_key: true, default: nil + t.integer :traversal_ids, array: true, default: [], null: false + + t.index :traversal_ids, name: INDEX_NAME, using: :gin + end + end +end diff --git a/db/migrate/20210921095839_add_ci_project_hierarchies.rb b/db/migrate/20210921095839_add_ci_project_hierarchies.rb new file mode 100644 index 0000000000000000000000000000000000000000..5dac4589c345ce46dc84905e9e44975846949191 --- /dev/null +++ b/db/migrate/20210921095839_add_ci_project_hierarchies.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +class AddCiProjectHierarchies < Gitlab::Database::Migration[1.0] + disable_ddl_transaction! + + TABLE_NAME = :ci_project_hierarchies + + def up + with_lock_retries do + execute(<<~SQL) + INSERT INTO #{TABLE_NAME} (project_xid, traversal_ids) + SELECT + projects.id AS project_xid, + namespaces.traversal_ids AS traversal_ids + FROM + ci_runner_projects + INNER JOIN projects ON ci_runner_projects.project_id = projects.id + INNER JOIN namespaces ON projects.namespace_id = namespaces.id + ON CONFLICT(project_xid) DO NOTHING + SQL + end + end + + def down + with_lock_retries do + execute(<<~SQL) + DELETE FROM #{TABLE_NAME} + SQL + end + end +end diff --git a/db/schema_migrations/20210920131736 b/db/schema_migrations/20210920131736 new file mode 100644 index 0000000000000000000000000000000000000000..6585b2f698b80909b6995b28101d34954f79b869 --- /dev/null +++ b/db/schema_migrations/20210920131736 @@ -0,0 +1 @@ +cd745a4e843bc225e27d16108a13d3717fd10b7702c884fcd4efa756bfa54809 \ No newline at end of file diff --git a/db/schema_migrations/20210920134647 b/db/schema_migrations/20210920134647 new file mode 100644 index 0000000000000000000000000000000000000000..d39bf7151baafd9a268fdc9ec2dc4615d25946ce --- /dev/null +++ b/db/schema_migrations/20210920134647 @@ -0,0 +1 @@ +a4709d6afb563378d5fc76250b2a6e853997934dd16d5400cd33ef3b93dc29ec \ No newline at end of file diff --git a/db/schema_migrations/20210921095325 b/db/schema_migrations/20210921095325 new file mode 100644 index 0000000000000000000000000000000000000000..903fc27f9912bdeb4c485962e7667a2a4df2ddbc --- /dev/null +++ b/db/schema_migrations/20210921095325 @@ -0,0 +1 @@ +5b6df07bf86a45be69e755557cea730b756709faf7dea34fc6fc28cc2f395eb7 \ No newline at end of file diff --git a/db/schema_migrations/20210921095839 b/db/schema_migrations/20210921095839 new file mode 100644 index 0000000000000000000000000000000000000000..128bdb128327ed9b067b9d29461a9fe7108f78c6 --- /dev/null +++ b/db/schema_migrations/20210921095839 @@ -0,0 +1 @@ +32731b47d5979b4ec58a0d67149e428401d72e6ce477bef42d903fb61f9ba872 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 80689a24f2a5ddfded38678d466b2c001c197963..75b253c662852cc5e7a41051c9bbae2d94950c6c 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -11597,6 +11597,11 @@ CREATE SEQUENCE ci_minutes_additional_packs_id_seq ALTER SEQUENCE ci_minutes_additional_packs_id_seq OWNED BY ci_minutes_additional_packs.id; +CREATE TABLE ci_namespace_hierarchies ( + namespace_xid integer NOT NULL, + traversal_ids integer[] DEFAULT '{}'::integer[] NOT NULL +); + CREATE TABLE ci_namespace_monthly_usages ( id bigint NOT NULL, namespace_id bigint NOT NULL, @@ -11843,6 +11848,11 @@ CREATE SEQUENCE ci_platform_metrics_id_seq ALTER SEQUENCE ci_platform_metrics_id_seq OWNED BY ci_platform_metrics.id; +CREATE TABLE ci_project_hierarchies ( + project_xid integer NOT NULL, + traversal_ids integer[] DEFAULT '{}'::integer[] NOT NULL +); + CREATE TABLE ci_project_monthly_usages ( id bigint NOT NULL, project_id bigint NOT NULL, @@ -22463,6 +22473,9 @@ ALTER TABLE ONLY ci_job_variables ALTER TABLE ONLY ci_minutes_additional_packs ADD CONSTRAINT ci_minutes_additional_packs_pkey PRIMARY KEY (id); +ALTER TABLE ONLY ci_namespace_hierarchies + ADD CONSTRAINT ci_namespace_hierarchies_pkey PRIMARY KEY (namespace_xid); + ALTER TABLE ONLY ci_namespace_monthly_usages ADD CONSTRAINT ci_namespace_monthly_usages_pkey PRIMARY KEY (id); @@ -22496,6 +22509,9 @@ ALTER TABLE ONLY ci_pipelines ALTER TABLE ONLY ci_platform_metrics ADD CONSTRAINT ci_platform_metrics_pkey PRIMARY KEY (id); +ALTER TABLE ONLY ci_project_hierarchies + ADD CONSTRAINT ci_project_hierarchies_pkey PRIMARY KEY (project_xid); + ALTER TABLE ONLY ci_project_monthly_usages ADD CONSTRAINT ci_project_monthly_usages_pkey PRIMARY KEY (id); @@ -25173,8 +25189,12 @@ CREATE INDEX index_geo_reset_checksum_events_on_project_id ON geo_reset_checksum CREATE INDEX index_geo_upload_deleted_events_on_upload_id ON geo_upload_deleted_events USING btree (upload_id); +CREATE INDEX index_gin_ci_namespace_hierarchies ON ci_namespace_hierarchies USING gin (traversal_ids); + CREATE INDEX index_gin_ci_pending_builds_on_namespace_traversal_ids ON ci_pending_builds USING gin (namespace_traversal_ids); +CREATE INDEX index_gin_ci_project_hierarchies ON ci_project_hierarchies USING gin (traversal_ids); + CREATE INDEX index_gitlab_subscription_histories_on_gitlab_subscription_id ON gitlab_subscription_histories USING btree (gitlab_subscription_id); CREATE INDEX index_gitlab_subscriptions_on_end_date_and_namespace_id ON gitlab_subscriptions USING btree (end_date, namespace_id); diff --git a/ee/lib/analytics/devops_adoption/snapshot_calculator.rb b/ee/lib/analytics/devops_adoption/snapshot_calculator.rb index 0ad5d021102a2150941352f57ff1325836cf6983..620640fef14f3f222e57640e2235725bfdfc3587 100644 --- a/ee/lib/analytics/devops_adoption/snapshot_calculator.rb +++ b/ee/lib/analytics/devops_adoption/snapshot_calculator.rb @@ -61,9 +61,7 @@ def merge_request_approved # rubocop: enable CodeReuse/ActiveRecord def runner_configured - ::Gitlab::Database.allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/337541') do - Ci::Runner.active.belonging_to_group_or_project(snapshot_groups, snapshot_project_ids).exists? - end + Ci::Runner.active.belonging_to_group_or_descendants(enabled_namespace.namespace).exists? end def pipeline_succeeded diff --git a/spec/models/ci/namespace_hierarchy_spec.rb b/spec/models/ci/namespace_hierarchy_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..a7ac18e72cac98a438abe0d9ccd0fe99bcbc7b85 --- /dev/null +++ b/spec/models/ci/namespace_hierarchy_spec.rb @@ -0,0 +1,144 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::NamespaceHierarchy do + describe '#belonging_to_namespace' do + subject { described_class.belonging_to_namespace(group) } + + let_it_be(:parent_group) { create(:group) } + let_it_be(:group) { create(:group, parent: parent_group) } + + context 'with no runners registered' do + it 'returns no namespace records' do + is_expected.to be_empty + end + end + + context 'with 2 runners registered on same group' do + let_it_be(:group_runner1) { create(:ci_runner, :group, groups: [group]) } + let_it_be(:group_runner2) { create(:ci_runner, :group, groups: [group]) } + + it 'returns one namespace record' do + is_expected.to all(be_a(described_class)) + is_expected.to match_array([have_attributes(namespace_xid: group.id, traversal_ids: [parent_group.id, group.id])]) + end + end + + context 'with 2 runners registered on different groups' do + let_it_be(:group2) { create(:group, parent: parent_group) } + let_it_be(:group_runner1) { create(:ci_runner, :group, groups: [group]) } + let_it_be(:group_runner2) { create(:ci_runner, :group, groups: [group2]) } + + it 'returns one namespace record' do + is_expected.to all(be_a(described_class)) + is_expected.to match_array([ + have_attributes(namespace_xid: group.id, traversal_ids: [parent_group.id, group.id]) + ]) + end + + specify 'group2 should have its own namespace record' do + group2_self_and_ancestors = described_class.belonging_to_namespace(group2) + + expect(group2_self_and_ancestors).to match_array([ + have_attributes(namespace_xid: group2.id, traversal_ids: [parent_group.id, group2.id]) + ]) + end + end + end + + describe 'hierarchy change triggers' do + let(:records_from_old_and_new_parents) do + described_class.from(<