Restrict last_activity_at updates to one per hourthrottle-last-activity-at

The lock in turn is only obtained when actually needed, reducing some
load on Redis.

Fixes gitlab-org/gitlab-ce#22213

3 files changed, 47 insertions, 13 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 6093cb09880..0e9c5df09eb 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -15,6 +15,7 @@ v 8.12.0 (unreleased)
   - Make push events have equal vertical spacing.
   - Add two-factor recovery endpoint to internal API !5510
   - Pass the "Remember me" value to the U2F authentication form
+  - Only update projects.last_activity_at once per hour when creating a new event
   - Remove vendor prefixes for linear-gradient CSS (ClemMakesApps)
   - Move pushes_since_gc from the database to Redis
   - Add font color contrast to external label in admin area (ClemMakesApps)
diff --git a/app/models/event.rb b/app/models/event.rb
index a0b7b0dc2b5..03f5c904e96 100644
--- a/app/models/event.rb
+++ b/app/models/event.rb
@@ -324,8 +324,18 @@ class Event < ActiveRecord::Base
   end
 
   def reset_project_activity
-    if project && Gitlab::ExclusiveLease.new("project:update_last_activity_at:#{project.id}", timeout: 60).try_obtain
-      project.update_column(:last_activity_at, self.created_at)
-    end
+    return unless project
+
+    interval = 1.hour
+
+    # Don't even bother obtaining a lock if the last update happened less than
+    # 60 minutes ago.
+    return if (Time.now - project.last_activity_at) <= interval
+
+    return unless Gitlab::ExclusiveLease.
+      new("project:update_last_activity_at:#{project.id}", timeout: interval.to_i).
+      try_obtain
+
+    project.update_column(:last_activity_at, created_at)
   end
 end
diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb
index b5d0d79e14e..8600eb4d2c4 100644
--- a/spec/models/event_spec.rb
+++ b/spec/models/event_spec.rb
@@ -16,18 +16,12 @@ describe Event, models: true do
 
   describe 'Callbacks' do
     describe 'after_create :reset_project_activity' do
-      let(:project) { create(:project) }
+      let(:project) { create(:empty_project) }
 
-      context "project's last activity was less than 5 minutes ago" do
-        it 'does not update project.last_activity_at if it has been touched less than 5 minutes ago' do
-          create_event(project, project.owner)
-          project.update_column(:last_activity_at, 5.minutes.ago)
-          project_last_activity_at = project.last_activity_at
+      it 'calls the reset_project_activity method' do
+        expect_any_instance_of(Event).to receive(:reset_project_activity)
 
-          create_event(project, project.owner)
-
-          expect(project.last_activity_at).to eq(project_last_activity_at)
-        end
+        create_event(project, project.owner)
       end
     end
   end
@@ -161,6 +155,35 @@ describe Event, models: true do
     end
   end
 
+  describe '#reset_project_activity' do
+    let(:project) { create(:empty_project) }
+
+    context 'when a project was updated less than 1 hour ago' do
+      it 'does not update the project' do
+        project.update(last_activity_at: Time.now)
+
+        expect(project).not_to receive(:update_column).
+          with(:last_activity_at, a_kind_of(Time))
+
+        create_event(project, project.owner)
+      end
+    end
+
+    context 'when a project was updated more than 1 hour ago' 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)
+      end
+    end
+  end
+
   def create_event(project, user, attrs = {})
     data = {
       before: Gitlab::Git::BLANK_SHA,