diff --git a/ee/app/models/concerns/geo/blob_replicator_strategy.rb b/ee/app/models/concerns/geo/blob_replicator_strategy.rb index a093da20296ae522dddc98b0c0239db73c178c17..46b6206de2938931cc011e1c3eee65b3668d98cd 100644 --- a/ee/app/models/concerns/geo/blob_replicator_strategy.rb +++ b/ee/app/models/concerns/geo/blob_replicator_strategy.rb @@ -4,7 +4,7 @@ module Geo module BlobReplicatorStrategy extend ActiveSupport::Concern - include Delay + include ::Geo::VerifiableReplicator include Gitlab::Geo::LogHelpers included do @@ -17,7 +17,8 @@ def handle_after_create_commit return unless self.class.enabled? publish(:created, **created_params) - schedule_checksum_calculation if needs_checksum? + + after_verifiable_update end # Called by Gitlab::Geo::Replicator#consume @@ -47,45 +48,8 @@ def blob_path carrierwave_uploader.path end - def calculate_checksum! - checksum = model_record.calculate_checksum! - update_verification_state!(checksum: checksum) - rescue => e - log_error('Error calculating the checksum', e) - update_verification_state!(failure: e.message) - end - - # Check if given checksum matches known one - # - # @param [String] checksum - # @return [Boolean] whether checksum matches - def matches_checksum?(checksum) - model_record.verification_checksum == checksum - end - private - # Update checksum on Geo primary database - # - # @param [String] checksum value generated by the checksum routine - # @param [String] failure (optional) stacktrace from failed execution - def update_verification_state!(checksum: nil, failure: nil) - retry_at, retry_count = calculate_next_retry_attempt if failure.present? - - model_record.update!( - verification_checksum: checksum, - verified_at: Time.current, - verification_failure: failure, - verification_retry_at: retry_at, - verification_retry_count: retry_count - ) - end - - def calculate_next_retry_attempt - retry_count = model_record.verification_retry_count.to_i + 1 - [next_retry_time(retry_count), retry_count] - end - def download ::Geo::BlobDownloadService.new(replicator: self).execute end diff --git a/ee/app/models/concerns/geo/repository_replicator_strategy.rb b/ee/app/models/concerns/geo/repository_replicator_strategy.rb index 821f315f3c9c217111bc34c0a184a3e96af872dc..3caddd139551d40cadfe50a5b3ebc98628f79e04 100644 --- a/ee/app/models/concerns/geo/repository_replicator_strategy.rb +++ b/ee/app/models/concerns/geo/repository_replicator_strategy.rb @@ -4,7 +4,7 @@ module Geo module RepositoryReplicatorStrategy extend ActiveSupport::Concern - include Delay + include ::Geo::VerifiableReplicator include Gitlab::Geo::LogHelpers included do diff --git a/ee/app/models/concerns/geo/verifiable_replicator.rb b/ee/app/models/concerns/geo/verifiable_replicator.rb new file mode 100644 index 0000000000000000000000000000000000000000..d6b166123c21efe1c8a9a67e139e409ca6d57476 --- /dev/null +++ b/ee/app/models/concerns/geo/verifiable_replicator.rb @@ -0,0 +1,87 @@ +# frozen_string_literal: true + +module Geo + module VerifiableReplicator + extend ActiveSupport::Concern + + include Delay + + class_methods do + def checksummed + model.available_replicables.checksummed + end + + def checksummed_count + model.available_replicables.checksummed.count + end + + def checksum_failed_count + model.available_replicables.checksum_failed.count + end + end + + def after_verifiable_update + schedule_checksum_calculation if needs_checksum? + end + + def calculate_checksum! + checksum = model_record.calculate_checksum! + update_verification_state!(checksum: checksum) + rescue => e + log_error('Error calculating the checksum', e) + update_verification_state!(failure: e.message) + end + + # Check if given checksum matches known one + # + # @param [String] checksum + # @return [Boolean] whether checksum matches + def matches_checksum?(checksum) + model_record.verification_checksum == checksum + end + + def needs_checksum? + return true unless model_record.respond_to?(:needs_checksum?) + + model_record.needs_checksum? + end + + # Checksum value from the main database + # + # @abstract + def primary_checksum + model_record.verification_checksum + end + + def secondary_checksum + registry.verification_checksum + end + + private + + # Update checksum on Geo primary database + # + # @param [String] checksum value generated by the checksum routine + # @param [String] failure (optional) stacktrace from failed execution + def update_verification_state!(checksum: nil, failure: nil) + retry_at, retry_count = calculate_next_retry_attempt if failure.present? + + model_record.update!( + verification_checksum: checksum, + verified_at: Time.current, + verification_failure: failure, + verification_retry_at: retry_at, + verification_retry_count: retry_count + ) + end + + def calculate_next_retry_attempt + retry_count = model_record.verification_retry_count.to_i + 1 + [next_retry_time(retry_count), retry_count] + end + + def schedule_checksum_calculation + raise NotImplementedError + end + end +end diff --git a/ee/lib/gitlab/geo/replicator.rb b/ee/lib/gitlab/geo/replicator.rb index 9b1c76a389229b34c5266bc651fae599079acde6..a85a347d324d5812065ede601072a0f24d8df9ea 100644 --- a/ee/lib/gitlab/geo/replicator.rb +++ b/ee/lib/gitlab/geo/replicator.rb @@ -134,18 +134,6 @@ def self.for_replicable_params(replicable_name:, replicable_id:) replicator_class.new(model_record_id: replicable_id) end - def self.checksummed - model.available_replicables.checksummed - end - - def self.checksummed_count - model.available_replicables.checksummed.count - end - - def self.checksum_failed_count - model.available_replicables.checksum_failed.count - end - def self.primary_total_count model.available_replicables.count end @@ -265,17 +253,6 @@ def registry registry_class.for_model_record_id(model_record_id) end - # Checksum value from the main database - # - # @abstract - def primary_checksum - model_record.verification_checksum - end - - def secondary_checksum - registry.verification_checksum - end - # Return exactly the data needed by `for_replicable_params` to # reinstantiate this Replicator elsewhere. # @@ -298,10 +275,6 @@ def handle_after_update publish(:updated, **updated_params) end - def schedule_checksum_calculation - raise NotImplementedError - end - def created_params event_params end @@ -318,12 +291,6 @@ def event_params { model_record_id: model_record.id } end - def needs_checksum? - return true unless model_record.respond_to?(:needs_checksum?) - - model_record.needs_checksum? - end - protected # Store an event on the database diff --git a/ee/spec/support/shared_examples/models/concerns/blob_replicator_strategy_shared_examples.rb b/ee/spec/support/shared_examples/models/concerns/blob_replicator_strategy_shared_examples.rb index 76c08ac92bef50e0311af364c87132df35871790..375810cc27458de2b4adcc87654008d16aaa8619 100644 --- a/ee/spec/support/shared_examples/models/concerns/blob_replicator_strategy_shared_examples.rb +++ b/ee/spec/support/shared_examples/models/concerns/blob_replicator_strategy_shared_examples.rb @@ -19,6 +19,7 @@ end it_behaves_like 'a replicator' + it_behaves_like 'a verifiable replicator' # This could be included in each model's spec, but including it here is DRYer. include_examples 'a replicable model' @@ -33,9 +34,8 @@ "replicable_name" => replicator.replicable_name, "event_name" => "created", "payload" => { "model_record_id" => replicator.model_record.id }) end - it 'schedules the checksum calculation if needed' do - expect(Geo::BlobVerificationPrimaryWorker).to receive(:perform_async) - expect(replicator).to receive(:needs_checksum?).and_return(true) + it 'calls #after_verifiable_update' do + expect(replicator).to receive(:after_verifiable_update) replicator.handle_after_create_commit end @@ -45,8 +45,8 @@ stub_feature_flags(replicator.replication_enabled_feature_key => false) end - it 'does not schedule the checksum calculation' do - expect(Geo::BlobVerificationPrimaryWorker).not_to receive(:perform_async) + it 'does not call #after_verifiable_update' do + expect(replicator).not_to receive(:after_verifiable_update) replicator.handle_after_create_commit end @@ -82,30 +82,6 @@ end end - describe '#calculate_checksum!' do - it 'calculates the checksum' do - model_record.save! - - replicator.calculate_checksum! - - expect(model_record.reload.verification_checksum).not_to be_nil - expect(model_record.reload.verified_at).not_to be_nil - end - - it 'saves the error message and increments retry counter' do - model_record.save! - - allow(model_record).to receive(:calculate_checksum!) do - raise StandardError.new('Failure to calculate checksum') - end - - replicator.calculate_checksum! - - expect(model_record.reload.verification_failure).to eq 'Failure to calculate checksum' - expect(model_record.verification_retry_count).to be 1 - end - end - describe 'created event consumption' do context "when the blob's project is in replicables for this geo node" do it 'invokes Geo::BlobDownloadService' do diff --git a/ee/spec/support/shared_examples/models/concerns/verifiable_replicator_shared_examples.rb b/ee/spec/support/shared_examples/models/concerns/verifiable_replicator_shared_examples.rb new file mode 100644 index 0000000000000000000000000000000000000000..d0818e9353c4805db7461504b6b9ac2b935da835 --- /dev/null +++ b/ee/spec/support/shared_examples/models/concerns/verifiable_replicator_shared_examples.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +# This is included by BlobReplicatorStrategy and RepositoryReplicatorStrategy. +# +RSpec.shared_examples 'a verifiable replicator' do + include EE::GeoHelpers + + describe '#after_verifiable_update' do + it 'schedules the checksum calculation if needed' do + expect(replicator).to receive(:schedule_checksum_calculation) + expect(replicator).to receive(:needs_checksum?).and_return(true) + + replicator.after_verifiable_update + end + end + + describe '#calculate_checksum!' do + it 'calculates the checksum' do + model_record.save! + + replicator.calculate_checksum! + + expect(model_record.reload.verification_checksum).not_to be_nil + expect(model_record.reload.verified_at).not_to be_nil + end + + it 'saves the error message and increments retry counter' do + model_record.save! + + allow(model_record).to receive(:calculate_checksum!) do + raise StandardError.new('Failure to calculate checksum') + end + + replicator.calculate_checksum! + + expect(model_record.reload.verification_failure).to eq 'Failure to calculate checksum' + expect(model_record.verification_retry_count).to be 1 + end + end +end