diff options
-rw-r--r-- | app/models/ci/runner.rb | 52 | ||||
-rw-r--r-- | spec/models/ci/runner_spec.rb | 25 | ||||
-rw-r--r-- | spec/requests/api/runner_spec.rb | 2 |
3 files changed, 38 insertions, 41 deletions
diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb index 5711be3ae29..6bd90e71bf3 100644 --- a/app/models/ci/runner.rb +++ b/app/models/ci/runner.rb @@ -2,12 +2,14 @@ module Ci class Runner < ActiveRecord::Base extend Gitlab::Ci::Model include Gitlab::SQL::Pattern + include Gitlab::Utils::StrongMemoize RUNNER_QUEUE_EXPIRY_TIME = 60.minutes ONLINE_CONTACT_TIMEOUT = 1.hour UPDATE_DB_RUNNER_INFO_EVERY = 40.minutes AVAILABLE_SCOPES = %w[specific shared active paused online].freeze FORM_EDITABLE = %i[description tag_list active run_untagged locked access_level].freeze + CACHED_ATTRIBUTES_EXPIRY_TIME = 24.hours has_many :builds has_many :runner_projects, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent @@ -68,29 +70,15 @@ module Ci ONLINE_CONTACT_TIMEOUT.ago end - def name - cached_attribute(:name) || read_attribute(:name) - end - - def version - cached_attribute(:version) || read_attribute(:version) - end - - def revision - cached_attribute(:revision) || read_attribute(:revision) - end - - def platform - cached_attribute(:platform) || read_attribute(:platform) - end - - def architecture - cached_attribute(:architecture) || read_attribute(:architecture) + def self.cached_attr_reader(*attributes) + attributes.each do |attribute| + define_method("#{attribute}") do + cached_attribute(attribute) || read_attribute(attribute) + end + end end - def contacted_at - cached_attribute(:contacted_at) || read_attribute(:contacted_at) - end + cached_attr_reader :version, :revision, :platform, :architecture, :contacted_at def set_default_values self.token = SecureRandom.hex(15) if self.token.blank? @@ -178,7 +166,7 @@ module Ci end def update_cached_info(values) - values = values&.slice(:name, :version, :revision, :platform, :architecture) || {} + values = values&.slice(:version, :revision, :platform, :architecture) || {} values[:contacted_at] = Time.now cache_attributes(values) @@ -201,8 +189,8 @@ module Ci "runner:build_queue:#{self.token}" end - def cache_attribute_key(key) - "runner:info:#{self.id}:#{key}" + def cache_attribute_key + "runner:info:#{self.id}" end def persist_cached_data? @@ -217,17 +205,21 @@ module Ci end def cached_attribute(key) - @cached_attributes = {} - @cached_attributes[key] ||= Gitlab::Redis::SharedState.with do |redis| - redis.get(cache_attribute_key(key)) + (cached_attributes || {})[key] + end + + def cached_attributes + strong_memoize(:cached_attributes) do + Gitlab::Redis::SharedState.with do |redis| + data = redis.get(cache_attribute_key) + JSON.parse(data, symbolize_names: true) if data + end end end def cache_attributes(values) Gitlab::Redis::SharedState.with do |redis| - values.each do |key, value| - redis.set(cache_attribute_key(key), value, ex: 24.hours) - end + redis.set(cache_attribute_key, values.to_json, ex: CACHED_ATTRIBUTES_EXPIRY_TIME) end end diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 41e27c62bd9..ab170e6351c 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -95,6 +95,12 @@ describe Ci::Runner do subject { runner.online? } + before do + allow_any_instance_of(described_class).to receive(:cached_attribute).and_call_original + allow_any_instance_of(described_class).to receive(:cached_attribute) + .with(:platform).and_return("darwin") + end + context 'no cache value' do before do stub_redis_runner_contacted_at(nil) @@ -147,8 +153,9 @@ describe Ci::Runner do def stub_redis_runner_contacted_at(value) Gitlab::Redis::SharedState.with do |redis| - cache_key = runner.send(:cache_attribute_key, :contacted_at) - expect(redis).to receive(:get).with(cache_key).and_return(value).at_least(:once) + cache_key = runner.send(:cache_attribute_key) + expect(redis).to receive(:get).with(cache_key) + .and_return({ contacted_at: value }.to_json).at_least(:once) end end end @@ -405,7 +412,7 @@ describe Ci::Runner do end it 'updates cache' do - expect_redis_update(:architecture, :contacted_at) + expect_redis_update subject end @@ -417,25 +424,23 @@ describe Ci::Runner do end it 'updates database' do - expect_redis_update(:architecture, :contacted_at) + expect_redis_update expect { subject }.to change { runner.reload.read_attribute(:contacted_at) } .and change { runner.reload.read_attribute(:architecture) } end it 'updates cache' do - expect_redis_update(:architecture, :contacted_at) + expect_redis_update subject end end - def expect_redis_update(*params) + def expect_redis_update Gitlab::Redis::SharedState.with do |redis| - params.each do |param| - redis_key = runner.send(:cache_attribute_key, param) - expect(redis).to receive(:set).with(redis_key, anything, any_args) - end + redis_key = runner.send(:cache_attribute_key) + expect(redis).to receive(:set).with(redis_key, anything, any_args) end end end diff --git a/spec/requests/api/runner_spec.rb b/spec/requests/api/runner_spec.rb index 0adb314e51d..1040d5dd887 100644 --- a/spec/requests/api/runner_spec.rb +++ b/spec/requests/api/runner_spec.rb @@ -409,7 +409,7 @@ describe API::Runner do expect { request_job }.to change { runner.reload.contacted_at } end - %w(name version revision platform architecture).each do |param| + %w(version revision platform architecture).each do |param| context "when info parameter '#{param}' is present" do let(:value) { "#{param}_value" } |