diff --git a/app/models/concerns/counter_attribute.rb b/app/models/concerns/counter_attribute.rb index 058cb7752f65885455a43af5581cd4a6a703ea5e..03e062a98557a7ee6aa1a085c528713a0293d621 100644 --- a/app/models/concerns/counter_attribute.rb +++ b/app/models/concerns/counter_attribute.rb @@ -131,14 +131,14 @@ def increment_counter(attribute, increment) end def update_counters_with_lease(increments) - detect_race_on_record(log_fields: increments.merge({ caller: __method__ })) do + detect_race_on_record(log_fields: { caller: __method__, attributes: increments.keys }) do self.class.update_counters(id, increments) end end def reset_counter!(attribute) if counter_attribute_enabled?(attribute) - detect_race_on_record(log_fields: { caller: __method__ }) do + detect_race_on_record(log_fields: { caller: __method__, attributes: attribute }) do update!(attribute => 0) clear_counter!(attribute) end @@ -219,13 +219,25 @@ def with_exclusive_lease(lock_key) def detect_race_on_record(log_fields: {}) return yield unless Feature.enabled?(:counter_attribute_db_lease_for_update, project) - in_lock(database_lock_key, retries: 2) do + # Ensure attributes is always an array before we log + log_fields[:attributes] = Array(log_fields[:attributes]) + + Gitlab::AppLogger.info( + message: 'Acquiring lease for project statistics update', + project_statistics_id: id, + project_id: project.id, + **log_fields, + **Gitlab::ApplicationContext.current + ) + + in_lock(database_lock_key, retries: 0) do yield end rescue Gitlab::ExclusiveLeaseHelpers::FailedToObtainLockError Gitlab::AppLogger.warn( - message: 'Concurrent update to project statistics detected', + message: 'Concurrent project statistics update detected', project_statistics_id: id, + project_id: project.id, **log_fields, **Gitlab::ApplicationContext.current ) diff --git a/app/models/project_statistics.rb b/app/models/project_statistics.rb index b1753ce711e72b8a6e82dd83fde624b2119d7075..e13f8d28c9270f171c39b227de14037719e61205 100644 --- a/app/models/project_statistics.rb +++ b/app/models/project_statistics.rb @@ -50,17 +50,16 @@ def total_repository_size def refresh!(only: []) return if Gitlab::Database.read_only? - COLUMNS_TO_REFRESH.each do |column, generator| - if only.empty? || only.include?(column) - public_send("update_#{column}") # rubocop:disable GitlabSecurity/PublicSend - end + columns_to_update = only.empty? ? COLUMNS_TO_REFRESH : COLUMNS_TO_REFRESH & only + columns_to_update.each do |column| + public_send("update_#{column}") # rubocop:disable GitlabSecurity/PublicSend end if only.empty? || only.any? { |column| NAMESPACE_RELATABLE_COLUMNS.include?(column) } schedule_namespace_aggregation_worker end - detect_race_on_record(log_fields: { caller: __method__ }) do + detect_race_on_record(log_fields: { caller: __method__, attributes: columns_to_update }) do save! end end @@ -114,7 +113,7 @@ def update_storage_size end def refresh_storage_size! - detect_race_on_record(log_fields: { caller: __method__ }) do + detect_race_on_record(log_fields: { caller: __method__, attributes: :storage_size }) do update!(storage_size: STORAGE_SIZE_SUM) end end diff --git a/spec/services/ci/pipeline_artifacts/coverage_report_service_spec.rb b/spec/services/ci/pipeline_artifacts/coverage_report_service_spec.rb index b020479a78ea72771ba876659bd03df5c1dfd350..c4558bddc851c286ead24c2b47321a864d6c626a 100644 --- a/spec/services/ci/pipeline_artifacts/coverage_report_service_spec.rb +++ b/spec/services/ci/pipeline_artifacts/coverage_report_service_spec.rb @@ -35,6 +35,7 @@ end it 'logs relevant information' do + allow(Gitlab::AppLogger).to receive(:info).and_call_original expect(Gitlab::AppLogger).to receive(:info).with({ project_id: project.id, pipeline_id: pipeline.id, diff --git a/spec/support/shared_examples/models/concerns/counter_attribute_shared_examples.rb b/spec/support/shared_examples/models/concerns/counter_attribute_shared_examples.rb index ea4ba536403e7814b050d1610cf11b2f2387a5ba..a658d02f09a1e5f14f7c7c1cc57bc22dbcdd38a2 100644 --- a/spec/support/shared_examples/models/concerns/counter_attribute_shared_examples.rb +++ b/spec/support/shared_examples/models/concerns/counter_attribute_shared_examples.rb @@ -104,7 +104,14 @@ model.delayed_increment_counter(incremented_attribute, -3) end - it 'updates the record and logs it' do + it 'updates the record and logs it', :aggregate_failures do + expect(Gitlab::AppLogger).to receive(:info).with( + hash_including( + message: 'Acquiring lease for project statistics update', + attributes: [incremented_attribute] + ) + ) + expect(Gitlab::AppLogger).to receive(:info).with( hash_including( message: 'Flush counter attribute to database',