diff --git a/CHANGELOG b/CHANGELOG index 9635309b052..9060cfcef0b 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -36,6 +36,7 @@ v 8.13.0 (unreleased) - Add missing values to linter !6276 (Katarzyna Kobierska Ula Budziszewska) - Fix Long commit messages overflow viewport in file tree - Revert avoid touching file system on Build#artifacts? + - Stop using a Redis lease when updating the project activity timestamp whenever a new event is created - Add broadcast messages and alerts below sub-nav - Better empty state for Groups view - Update ruby-prof to 0.16.2. !6026 (Elan Ruusamäe) diff --git a/app/models/event.rb b/app/models/event.rb index 55a76e26f3c..633019fe0af 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -328,13 +328,15 @@ class Event < ActiveRecord::Base def reset_project_activity return unless project - # Don't even bother obtaining a lock if the last update happened less than - # 60 minutes ago. + # Don't bother updating if we know the project was updated recently. return if recent_update? - return unless try_obtain_lease - - project.update_column(:last_activity_at, created_at) + # At this point it's possible for multiple threads/processes to try to + # update the project. Only one query should actually perform the update, + # hence we add the extra WHERE clause for last_activity_at. + Project.unscoped.where(id: project_id). + where('last_activity_at > ?', RESET_PROJECT_ACTIVITY_INTERVAL.ago). + update_all(last_activity_at: created_at) end private @@ -342,11 +344,4 @@ class Event < ActiveRecord::Base def recent_update? project.last_activity_at > RESET_PROJECT_ACTIVITY_INTERVAL.ago end - - def try_obtain_lease - Gitlab::ExclusiveLease. - new("project:update_last_activity_at:#{project.id}", - timeout: RESET_PROJECT_ACTIVITY_INTERVAL.to_i). - try_obtain - end end diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb index 8600eb4d2c4..af5002487cc 100644 --- a/spec/models/event_spec.rb +++ b/spec/models/event_spec.rb @@ -173,13 +173,11 @@ describe Event, models: true do it 'updates the project' do project.update(last_activity_at: 1.year.ago) - expect_any_instance_of(Gitlab::ExclusiveLease). - to receive(:try_obtain).and_return(true) - - expect(project).to receive(:update_column). - with(:last_activity_at, a_kind_of(Time)) - create_event(project, project.owner) + + project.reload + + project.last_activity_at <= 1.minute.ago end end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index ef854a25321..3ab5ac78bba 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -308,8 +308,7 @@ describe Project, models: true do end describe 'last_activity methods' do - let(:timestamp) { Time.now - 2.hours } - let(:project) { create(:project, created_at: timestamp, updated_at: timestamp) } + let(:project) { create(:project, last_activity_at: 2.hours.ago) } describe 'last_activity' do it 'alias last_activity to last_event' do @@ -321,7 +320,6 @@ describe Project, models: true do describe 'last_activity_date' do it 'returns the creation date of the project\'s last event if present' do - expect_any_instance_of(Event).to receive(:try_obtain_lease).and_return(true) new_event = create(:event, project: project, created_at: Time.now) expect(project.last_activity_at.to_i).to eq(new_event.created_at.to_i)