diff --git a/ee/app/finders/geo/legacy_project_registry_verification_failed_finder.rb b/ee/app/finders/geo/legacy_project_registry_verification_failed_finder.rb new file mode 100644 index 0000000000000000000000000000000000000000..6a829367eea6c6a6d9934c4434356aa7275f7bb7 --- /dev/null +++ b/ee/app/finders/geo/legacy_project_registry_verification_failed_finder.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +# Finder for retrieving project registries that verification have +# failed scoped to a type (repository or wiki) using cross-database +# joins for selective sync. +# +# Basic usage: +# +# Geo::LegacyProjectRegistryVerificationFailedFinder.new(current_node: Gitlab::Geo.current_node, :repository).execute +# +# Valid `type` values are: +# +# * `:repository` +# * `:wiki` +# +# Any other value will be ignored. +module Geo + class LegacyProjectRegistryVerificationFailedFinder < RegistryFinder + def initialize(current_node: nil, type:) + super(current_node: current_node) + @type = type.to_s.to_sym + end + + def execute + if selective_sync? + failed_registries_for_selective_sync + else + failed_registries + end + end + + private + + attr_reader :type + + def failed_registries + Geo::ProjectRegistry.verification_failed(type) + end + + # rubocop: disable CodeReuse/ActiveRecord + def failed_registries_for_selective_sync + legacy_inner_join_registry_ids( + failed_registries, + current_node.projects.pluck(:id), + Geo::ProjectRegistry, + foreign_key: :project_id + ) + end + # rubocop: enable CodeReuse/ActiveRecord + end +end diff --git a/ee/app/finders/geo/project_registry_finder.rb b/ee/app/finders/geo/project_registry_finder.rb index 344ac0c9bf4c1b6c7606a0fab7d4199e566371b4..18ebc4c9f75796547d1885747c9b02e5a5b9adf0 100644 --- a/ee/app/finders/geo/project_registry_finder.rb +++ b/ee/app/finders/geo/project_registry_finder.rb @@ -34,6 +34,18 @@ def count_verified_wikis registries_for_verified_projects(:wiki).count end + def count_verification_failed_repositories + registries_for_verification_failed_projects(:repository).count + end + + def count_verification_failed_wikis + registries_for_verification_failed_projects(:wiki).count + end + + def find_verification_failed_project_registries(type = nil) + registries_for_verification_failed_projects(type) + end + def count_repositories_checksum_mismatch Geo::ProjectRegistry.repository_checksum_mismatch.count end @@ -50,22 +62,6 @@ def count_wikis_retrying_verification Geo::ProjectRegistry.wikis_retrying_verification.count end - def count_verification_failed_repositories - find_verification_failed_project_registries('repository').count - end - - def count_verification_failed_wikis - find_verification_failed_project_registries('wiki').count - end - - def find_verification_failed_project_registries(type = nil) - if use_legacy_queries? - legacy_find_filtered_verification_failed_projects(type) - else - find_filtered_verification_failed_project_registries(type) - end - end - def find_checksum_mismatch_project_registries(type = nil) if use_legacy_queries? legacy_find_filtered_checksum_mismatch_projects(type) @@ -111,17 +107,6 @@ def find_projects_updated_recently(batch_size:) protected - def find_filtered_verification_failed_project_registries(type = nil) - case type - when 'repository' - Geo::ProjectRegistry.verification_failed_repos - when 'wiki' - Geo::ProjectRegistry.verification_failed_wikis - else - Geo::ProjectRegistry.verification_failed - end - end - def find_filtered_checksum_mismatch_project_registries(type = nil) case type when 'repository' @@ -229,18 +214,6 @@ def quote_value(value) ::Gitlab::SQL::Glob.q(value) end - # @return [ActiveRecord::Relation] list of projects that verification has failed - # rubocop: disable CodeReuse/ActiveRecord - def legacy_find_filtered_verification_failed_projects(type = nil) - legacy_inner_join_registry_ids( - find_filtered_verification_failed_project_registries(type), - current_node.projects.pluck(:id), - Geo::ProjectRegistry, - foreign_key: :project_id - ) - end - # rubocop: enable CodeReuse/ActiveRecord - # @return [ActiveRecord::Relation] list of projects where there is a checksum_mismatch # rubocop: disable CodeReuse/ActiveRecord def legacy_find_filtered_checksum_mismatch_projects(type = nil) @@ -372,5 +345,19 @@ def registries_for_verified_projects(type) .new(current_node: current_node, type: type) .execute end + + def finder_klass_for_verification_failed_registries + if Gitlab::Geo::Fdw.enabled_for_selective_sync? + Geo::ProjectRegistryVerificationFailedFinder + else + Geo::LegacyProjectRegistryVerificationFailedFinder + end + end + + def registries_for_verification_failed_projects(type) + finder_klass_for_verification_failed_registries + .new(current_node: current_node, type: type) + .execute + end end end diff --git a/ee/app/finders/geo/project_registry_verification_failed_finder.rb b/ee/app/finders/geo/project_registry_verification_failed_finder.rb new file mode 100644 index 0000000000000000000000000000000000000000..552bea3385de46627c6378b00861ce5d28840edb --- /dev/null +++ b/ee/app/finders/geo/project_registry_verification_failed_finder.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +# Finder for retrieving project registries that have verification failed +# scoped to a type (repository or wiki) using FDW queries. +# +# Basic usage: +# +# Geo::ProjectRegistryVerificationFailedFinder.new(current_node: Gitlab::Geo.current_node, :repository).execute +# +# Valid `type` values are: +# +# * `:repository` +# * `:wiki` +# +# Any other value will be ignored. +module Geo + class ProjectRegistryVerificationFailedFinder + def initialize(current_node:, type:) + @current_node = Geo::Fdw::GeoNode.find(current_node.id) + @type = type.to_s.to_sym + end + + def execute + current_node.project_registries.verification_failed(type) + end + + private + + attr_reader :current_node, :type + end +end diff --git a/ee/app/finders/geo/project_registry_verified_finder.rb b/ee/app/finders/geo/project_registry_verified_finder.rb index 5aacf2309a33bd6626569a8c2eff5f7814042d77..943120c6c6d08e323dd1001d0b8c612bc5a6bf4d 100644 --- a/ee/app/finders/geo/project_registry_verified_finder.rb +++ b/ee/app/finders/geo/project_registry_verified_finder.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Finder for retrieving project registries that have been verified +# Finder for retrieving project registries that have verification succeeded # scoped to a type (repository or wiki) using FDW queries. # # Basic usage: diff --git a/ee/app/models/geo/project_registry.rb b/ee/app/models/geo/project_registry.rb index d2d802114efd32d1e7fcfd63242016bec74a45b4..eb519c6b7024c3b544f2170adf27e0644324b80a 100644 --- a/ee/app/models/geo/project_registry.rb +++ b/ee/app/models/geo/project_registry.rb @@ -46,13 +46,6 @@ def self.failed where(repository_sync_failed.or(wiki_sync_failed)) end - def self.verification_failed - repository_verification_failed = arel_table[:last_repository_verification_failure].not_eq(nil) - wiki_verification_failed = arel_table[:last_wiki_verification_failure].not_eq(nil) - - where(repository_verification_failed.or(wiki_verification_failed)) - end - def self.checksum_mismatch repository_checksum_mismatch = arel_table[:repository_checksum_mismatch].eq(true) wiki_checksum_mismatch = arel_table[:wiki_checksum_mismatch].eq(true) @@ -123,6 +116,17 @@ def self.verified(type) end end + def self.verification_failed(type) + case type + when :repository + verification_failed_repos + when :wiki + verification_failed_wikis + else + verification_failed_repos.or(verification_failed_wikis) + end + end + def self.flag_repositories_for_resync! update_all( resync_repository: true, diff --git a/ee/changelogs/unreleased/8798-geo-implement-selective-sync-support-for-fdw-queries-to-find-registries-verification-failed.yml b/ee/changelogs/unreleased/8798-geo-implement-selective-sync-support-for-fdw-queries-to-find-registries-verification-failed.yml new file mode 100644 index 0000000000000000000000000000000000000000..6a94d93e4d0cde651734a3781c070fe60d1b2770 --- /dev/null +++ b/ee/changelogs/unreleased/8798-geo-implement-selective-sync-support-for-fdw-queries-to-find-registries-verification-failed.yml @@ -0,0 +1,5 @@ +--- +title: Geo - Add selective sync support for FDW queries to find registries where verification has failed +merge_request: 10266 +author: +type: changed diff --git a/ee/spec/finders/geo/legacy_project_registry_verification_failed_finder_spec.rb b/ee/spec/finders/geo/legacy_project_registry_verification_failed_finder_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..4868100066eb9135ab25d42ffff1815d2770fb42 --- /dev/null +++ b/ee/spec/finders/geo/legacy_project_registry_verification_failed_finder_spec.rb @@ -0,0 +1,123 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Geo::LegacyProjectRegistryVerificationFailedFinder, :geo do + include EE::GeoHelpers + + describe '#execute' do + let(:node) { create(:geo_node) } + let(:group_1) { create(:group) } + let(:group_2) { create(:group) } + let(:nested_group_1) { create(:group, parent: group_1) } + let(:project_1) { create(:project, group: group_1) } + let(:project_2) { create(:project, group: nested_group_1) } + let(:project_3) { create(:project, group: nested_group_1) } + let(:project_4) { create(:project, :broken_storage, group: group_2) } + let(:project_5) { create(:project, :broken_storage, group: group_2) } + let!(:registry_verification_failed) { create(:geo_project_registry, :repository_verification_failed, :wiki_verification_failed, project: project_1) } + let!(:registry_repository_verification_failed) { create(:geo_project_registry, :repository_verification_failed, :wiki_verified, project: project_2) } + let!(:registry_wiki_verification_failed) { create(:geo_project_registry, :repository_verified, :wiki_verification_failed, project: project_3) } + let!(:registry_wiki_verification_failed_broken_shard) { create(:geo_project_registry, :repository_verified, :wiki_verification_failed, project: project_4) } + let!(:registry_repository_verification_failed_broken_shard) { create(:geo_project_registry, :repository_verification_failed, :wiki_verified, project: project_5) } + let!(:registry_verified) { create(:geo_project_registry, :repository_verified, :wiki_verified) } + + shared_examples 'finds registries that verification failed' do + context 'with repository type' do + subject { described_class.new(current_node: node, type: :repository) } + + context 'without selective sync' do + it 'returns all failed registries' do + expect(subject.execute).to contain_exactly(registry_verification_failed, registry_repository_verification_failed, registry_repository_verification_failed_broken_shard) + end + end + + context 'with selective sync by namespace' do + it 'returns failed registries where projects belongs to the namespaces' do + node.update!(selective_sync_type: 'namespaces', namespaces: [group_1, nested_group_1]) + + expect(subject.execute).to contain_exactly(registry_verification_failed, registry_repository_verification_failed) + end + end + + context 'with selective sync by shard' do + it 'returns failed registries where projects belongs to the shards' do + node.update!(selective_sync_type: 'shards', selective_sync_shards: ['broken']) + + expect(subject.execute).to contain_exactly(registry_repository_verification_failed_broken_shard) + end + end + end + + context 'with wiki type' do + subject { described_class.new(current_node: node, type: :wiki) } + + context 'without selective sync' do + it 'returns all failed registries' do + expect(subject.execute).to contain_exactly(registry_verification_failed, registry_wiki_verification_failed, registry_wiki_verification_failed_broken_shard) + end + end + + context 'with selective sync by namespace' do + it 'returns failed registries where projects belongs to the namespaces' do + node.update!(selective_sync_type: 'namespaces', namespaces: [group_1, nested_group_1]) + + expect(subject.execute).to contain_exactly(registry_verification_failed, registry_wiki_verification_failed) + end + end + + context 'with selective sync by shard' do + it 'returns failed registries where projects belongs to the shards' do + node.update!(selective_sync_type: 'shards', selective_sync_shards: ['broken']) + + expect(subject.execute).to contain_exactly(registry_wiki_verification_failed_broken_shard) + end + end + end + + context 'with no type' do + subject { described_class.new(current_node: node, type: :invalid) } + + context 'without selective sync' do + it 'returns all failed registries' do + expect(subject.execute).to contain_exactly(registry_verification_failed, registry_repository_verification_failed, registry_wiki_verification_failed, registry_repository_verification_failed_broken_shard, registry_wiki_verification_failed_broken_shard) + end + end + + context 'with selective sync by namespace' do + it 'returns all failed registries where projects belongs to the namespaces' do + node.update!(selective_sync_type: 'namespaces', namespaces: [group_1, nested_group_1]) + + expect(subject.execute).to contain_exactly(registry_verification_failed, registry_repository_verification_failed, registry_wiki_verification_failed) + end + end + + context 'with selective sync by shard' do + it 'returns all failed registries where projects belongs to the shards' do + node.update!(selective_sync_type: 'shards', selective_sync_shards: ['broken']) + + expect(subject.execute).to contain_exactly(registry_repository_verification_failed_broken_shard, registry_wiki_verification_failed_broken_shard) + end + end + end + end + + # Disable transactions via :delete method because a foreign table + # can't see changes inside a transaction of a different connection. + context 'FDW', :delete do + before do + skip('FDW is not configured') unless Gitlab::Geo::Fdw.enabled? + end + + include_examples 'finds registries that verification failed' + end + + context 'Legacy' do + before do + stub_fdw_disabled + end + + include_examples 'finds registries that verification failed' + end + end +end diff --git a/ee/spec/finders/geo/project_registry_finder_spec.rb b/ee/spec/finders/geo/project_registry_finder_spec.rb index 1712eae22e2be29f8fd3b9b035bbe4f5d7fb7fce..1d92c8c4ff2d1fd5d76d35aeb520e12163766457 100644 --- a/ee/spec/finders/geo/project_registry_finder_spec.rb +++ b/ee/spec/finders/geo/project_registry_finder_spec.rb @@ -166,28 +166,6 @@ end describe '#count_verification_failed_repositories' do - it 'delegates to #find_verification_failed_project_registries' do - expect(subject).to receive(:find_verification_failed_project_registries).with('repository').and_call_original - - subject.count_verification_failed_repositories - end - - it 'delegates to #legacy_find_filtered_verification_failed_projects when use_legacy_queries is true' do - expect(subject).to receive(:use_legacy_queries?).and_return(true) - - expect(subject).to receive(:legacy_find_filtered_verification_failed_projects).with('repository').and_call_original - - subject.count_verification_failed_repositories - end - - it 'delegates to #find_filtered_verification_failed_project_registries when use_legacy_queries is false' do - expect(subject).to receive(:use_legacy_queries?).and_return(false) - - expect(subject).to receive(:find_filtered_verification_failed_project_registries).with('repository').and_call_original - - subject.count_verification_failed_repositories - end - it 'counts projects that verification has failed' do create(:geo_project_registry, :repository_verified, project: project_repository_verified) create(:geo_project_registry, :repository_verification_failed, project: project_repository_verification_failed) @@ -208,28 +186,6 @@ end describe '#count_verification_failed_wikis' do - it 'delegates to #find_verification_failed_project_registries' do - expect(subject).to receive(:find_verification_failed_project_registries).with('wiki').and_call_original - - subject.count_verification_failed_wikis - end - - it 'delegates to #legacy_find_filtered_verification_failed_projects when use_legacy_queries is true' do - expect(subject).to receive(:use_legacy_queries?).and_return(true) - - expect(subject).to receive(:legacy_find_filtered_verification_failed_projects).with('wiki').and_call_original - - subject.count_verification_failed_wikis - end - - it 'delegates to #find_filtered_verification_failed_project_registries when use_legacy_queries is false' do - expect(subject).to receive(:use_legacy_queries?).and_return(false) - - expect(subject).to receive(:find_filtered_verification_failed_project_registries).with('wiki').and_call_original - - subject.count_verification_failed_wikis - end - it 'counts projects that verification has failed' do create(:geo_project_registry, :repository_verified, project: project_repository_verified) create(:geo_project_registry, :repository_verification_failed, project: project_repository_verification_failed) diff --git a/ee/spec/finders/geo/project_registry_verification_failed_finder_spec.rb b/ee/spec/finders/geo/project_registry_verification_failed_finder_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..8811a73b4329138e6d68b3df5c97a849a521ef34 --- /dev/null +++ b/ee/spec/finders/geo/project_registry_verification_failed_finder_spec.rb @@ -0,0 +1,107 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Geo::ProjectRegistryVerificationFailedFinder, :geo do + # Disable transactions via :delete method because a foreign table + # can't see changes inside a transaction of a different connection. + describe '#execute', :delete do + let(:node) { create(:geo_node) } + let(:group_1) { create(:group) } + let(:group_2) { create(:group) } + let(:nested_group_1) { create(:group, parent: group_1) } + let(:project_1) { create(:project, group: group_1) } + let(:project_2) { create(:project, group: nested_group_1) } + let(:project_3) { create(:project, group: nested_group_1) } + let(:project_4) { create(:project, :broken_storage, group: group_2) } + let(:project_5) { create(:project, :broken_storage, group: group_2) } + let!(:registry_verification_failed) { create(:geo_project_registry, :repository_verification_failed, :wiki_verification_failed, project: project_1) } + let!(:registry_repository_verification_failed) { create(:geo_project_registry, :repository_verification_failed, :wiki_verified, project: project_2) } + let!(:registry_wiki_verification_failed) { create(:geo_project_registry, :repository_verified, :wiki_verification_failed, project: project_3) } + let!(:registry_wiki_verification_failed_broken_shard) { create(:geo_project_registry, :repository_verified, :wiki_verification_failed, project: project_4) } + let!(:registry_repository_verification_failed_broken_shard) { create(:geo_project_registry, :repository_verification_failed, :wiki_verified, project: project_5) } + let!(:registry_verified) { create(:geo_project_registry, :repository_verified, :wiki_verified) } + + before do + skip('FDW is not configured') unless Gitlab::Geo::Fdw.enabled? + end + + context 'with repository type' do + subject { described_class.new(current_node: node, type: :repository) } + + context 'without selective sync' do + it 'returns all failed registries' do + expect(subject.execute).to contain_exactly(registry_verification_failed, registry_repository_verification_failed, registry_repository_verification_failed_broken_shard) + end + end + + context 'with selective sync by namespace' do + it 'returns failed registries where projects belongs to the namespaces' do + node.update!(selective_sync_type: 'namespaces', namespaces: [group_1, nested_group_1]) + + expect(subject.execute).to contain_exactly(registry_verification_failed, registry_repository_verification_failed) + end + end + + context 'with selective sync by shard' do + it 'returns failed registries where projects belongs to the shards' do + node.update!(selective_sync_type: 'shards', selective_sync_shards: ['broken']) + + expect(subject.execute).to contain_exactly(registry_repository_verification_failed_broken_shard) + end + end + end + + context 'with wiki type' do + subject { described_class.new(current_node: node, type: :wiki) } + + context 'without selective sync' do + it 'returns all failed registries' do + expect(subject.execute).to contain_exactly(registry_verification_failed, registry_wiki_verification_failed, registry_wiki_verification_failed_broken_shard) + end + end + + context 'with selective sync by namespace' do + it 'returns failed registries where projects belongs to the namespaces' do + node.update!(selective_sync_type: 'namespaces', namespaces: [group_1, nested_group_1]) + + expect(subject.execute).to contain_exactly(registry_verification_failed, registry_wiki_verification_failed) + end + end + + context 'with selective sync by shard' do + it 'returns failed registries where projects belongs to the shards' do + node.update!(selective_sync_type: 'shards', selective_sync_shards: ['broken']) + + expect(subject.execute).to contain_exactly(registry_wiki_verification_failed_broken_shard) + end + end + end + + context 'with no type' do + subject { described_class.new(current_node: node, type: :invalid) } + + context 'without selective sync' do + it 'returns all failed registries' do + expect(subject.execute).to contain_exactly(registry_verification_failed, registry_repository_verification_failed, registry_wiki_verification_failed, registry_repository_verification_failed_broken_shard, registry_wiki_verification_failed_broken_shard) + end + end + + context 'with selective sync by namespace' do + it 'returns all failed registries where projects belongs to the namespaces' do + node.update!(selective_sync_type: 'namespaces', namespaces: [group_1, nested_group_1]) + + expect(subject.execute).to contain_exactly(registry_verification_failed, registry_repository_verification_failed, registry_wiki_verification_failed) + end + end + + context 'with selective sync by shard' do + it 'returns all failed registries where projects belongs to the shards' do + node.update!(selective_sync_type: 'shards', selective_sync_shards: ['broken']) + + expect(subject.execute).to contain_exactly(registry_repository_verification_failed_broken_shard, registry_wiki_verification_failed_broken_shard) + end + end + end + end +end