From 36fbe1422666cb5553ccde9e534bb2a504f0f2c1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Kamil=20Trzci=C5=84ski?= <ayufan@ayufan.eu>
Date: Mon, 28 May 2018 18:17:18 +0200
Subject: [PATCH] Do not validate cached data

---
 app/models/ci/runner.rb                       |  6 ++---
 .../optimise-runner-update-cached-info.yml    |  5 ++++
 spec/models/ci/runner_spec.rb                 | 26 +++++++++++++------
 3 files changed, 25 insertions(+), 12 deletions(-)
 create mode 100644 changelogs/unreleased/optimise-runner-update-cached-info.yml

diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb
index 57edd6a4956..8c9aacca8de 100644
--- a/app/models/ci/runner.rb
+++ b/app/models/ci/runner.rb
@@ -219,10 +219,8 @@ module Ci
 
       cache_attributes(values)
 
-      if persist_cached_data?
-        self.assign_attributes(values)
-        self.save if self.changed?
-      end
+      # We save data without validation, it will always change due to `contacted_at`
+      self.update_columns(values) if persist_cached_data?
     end
 
     def pick_build!(build)
diff --git a/changelogs/unreleased/optimise-runner-update-cached-info.yml b/changelogs/unreleased/optimise-runner-update-cached-info.yml
new file mode 100644
index 00000000000..15fb9bcdf41
--- /dev/null
+++ b/changelogs/unreleased/optimise-runner-update-cached-info.yml
@@ -0,0 +1,5 @@
+---
+title: Update runner cached informations without performing validations
+merge_request:
+author:
+type: performance
diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb
index 0f072aa1719..f6433234573 100644
--- a/spec/models/ci/runner_spec.rb
+++ b/spec/models/ci/runner_spec.rb
@@ -549,7 +549,7 @@ describe Ci::Runner do
   end
 
   describe '#update_cached_info' do
-    let(:runner) { create(:ci_runner) }
+    let(:runner) { create(:ci_runner, :project) }
 
     subject { runner.update_cached_info(architecture: '18-bit') }
 
@@ -570,17 +570,22 @@ describe Ci::Runner do
         runner.contacted_at = 2.hours.ago
       end
 
-      it 'updates database' do
-        expect_redis_update
+      context 'with invalid runner' do
+        before do
+          runner.projects = []
+        end
 
-        expect { subject }.to change { runner.reload.read_attribute(:contacted_at) }
-          .and change { runner.reload.read_attribute(:architecture) }
+        it 'still updates redis cache and database' do
+          expect(runner).to be_invalid
+
+          expect_redis_update
+          does_db_update
+        end
       end
 
-      it 'updates cache' do
+      it 'updates redis cache and database' do
         expect_redis_update
-
-        subject
+        does_db_update
       end
     end
 
@@ -590,6 +595,11 @@ describe Ci::Runner do
         expect(redis).to receive(:set).with(redis_key, anything, any_args)
       end
     end
+
+    def does_db_update
+      expect { subject }.to change { runner.reload.read_attribute(:contacted_at) }
+        .and change { runner.reload.read_attribute(:architecture) }
+    end
   end
 
   describe '#destroy' do