Remove lease from Event#reset_project_activity
Per GitLab.com's performance metrics this method could take up to 5 seconds of wall time to complete, while only taking 1-2 milliseconds of CPU time. Removing the Redis lease in favour of conditional updates allows us to work around this. A slight drawback is that this allows for multiple threads/processes to try and update the same row. However, only a single thread/process will ever win since the UPDATE query uses a WHERE condition to only update rows that were not updated in the last hour. Fixes gitlab-org/gitlab-ce#22473
This commit is contained in:
parent
7887a3dafb
commit
c9bcfc631a
|
@ -35,6 +35,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)
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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)
|
||||
|
|
Loading…
Reference in New Issue