diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb index fb766cb3793..9749e019d3c 100644 --- a/app/models/ci/runner.rb +++ b/app/models/ci/runner.rb @@ -5,7 +5,7 @@ module Ci RUNNER_QUEUE_EXPIRY_TIME = 60.minutes ONLINE_CONTACT_TIMEOUT = 1.hour - UPDATE_DB_RUNNER_INFO_EVERY = 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 @@ -68,12 +68,20 @@ module Ci ONLINE_CONTACT_TIMEOUT.ago end - def cached_contacted_at - if runner_info_cache(:contacted_at) - Time.zone.parse(runner_info_cache(:contacted_at)) - else - self.contacted_at - end + def contacted_at + cached_attribute(:contacted_at) || read_attribute(:contacted_at) + end + + def version + cached_attribute(:version) || read_attribute(:version) + end + + def revision + cached_attribute(:revision) || read_attribute(:revision) + end + + def architecture + cached_attribute(:architecture) || read_attribute(:architecture) end def set_default_values @@ -97,7 +105,7 @@ module Ci end def online? - contacted_at && cached_contacted_at > self.class.contact_time_deadline + contacted_at && contacted_at > self.class.contact_time_deadline end def status @@ -161,16 +169,16 @@ module Ci ensure_runner_queue_value == value if value.present? end - def update_runner_info(params) - update_runner_info_cache(params) + def update_cached_info(values) + values = values.slice(:version, :revision, :platform, :architecture) + values[:contacted_at] = Time.now - # Use a 1h threshold to prevent beating DB updates. - return unless self.contacted_at.nil? || - (Time.now - self.contacted_at) >= UPDATE_DB_RUNNER_INFO_EVERY + cache_attributes(values) - self.contacted_at = Time.now - self.assign_attributes(params) - self.save if self.changed? + if persist_cached_data? + self.assign_attributes(values) + self.save + end end private @@ -185,24 +193,33 @@ module Ci "runner:build_queue:#{self.token}" end - def runner_info_redis_cache_key - "runner:info:#{self.id}" + def cache_attribute_key(key) + "runner:info:#{self.id}:#{key}" end - def update_runner_info_cache(params) - Gitlab::Redis::SharedState.with do |redis| - redis.set("#{runner_info_redis_cache_key}:contacted_at", Time.now) + def persist_cached_data? + # Use a random threshold to prevent beating DB updates. + # It generates a distribution between [40m, 80m]. - params && params.each do |key, value| - redis_key = "#{runner_info_redis_cache_key}:#{key}" - redis.set(redis_key, value) - end + contacted_at_max_age = UPDATE_DB_RUNNER_INFO_EVERY + Random.rand(UPDATE_DB_RUNNER_INFO_EVERY) + + real_contacted_at = read_attribute(:contacted_at) + real_contacted_at.nil? || + (Time.now - real_contacted_at) >= contacted_at_max_age + end + + def cached_attribute(key) + @cached_attributes = {} + @cached_attributes[key] ||= Gitlab::Redis::SharedState.with do |redis| + redis.get(cache_attribute_key(key)) end end - def runner_info_cache(attribute) + def cache_attributes(values) Gitlab::Redis::SharedState.with do |redis| - redis.get("#{runner_info_redis_cache_key}:#{attribute}") + values.each do |key, value| + redis.set(cache_attribute_key(key), value, ex: 24.hours) + end end end diff --git a/lib/api/helpers/runner.rb b/lib/api/helpers/runner.rb index 8f45cae0e60..87ba40e26e1 100644 --- a/lib/api/helpers/runner.rb +++ b/lib/api/helpers/runner.rb @@ -20,7 +20,7 @@ module API def authenticate_runner! forbidden! unless current_runner - current_runner.update_runner_info(get_runner_version_from_params) + current_runner.update_cached_info(get_runner_version_from_params) end def current_runner diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index ab931e5d43c..19a4e640090 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -146,9 +146,10 @@ describe Ci::Runner do end def stub_redis_runner_contacted_at(value) - redis = double - allow(Gitlab::Redis::SharedState).to receive(:with).and_yield(redis) - allow(redis).to receive(:get).with("#{runner.send(:runner_info_redis_cache_key)}:contacted_at").and_return(value) + Gitlab::Redis::SharedState.with do |redis| + cache_key = runner.send(:cache_attribute_key, :contacted_at) + allow(redis).to receive(:get).with(cache_key).and_return(value) + end end end @@ -393,10 +394,10 @@ describe Ci::Runner do end end - describe '#update_runner_info' do + describe '#update_cached_info' do let(:runner) { create(:ci_runner) } - subject { runner.update_runner_info(name: 'testing_runner') } + subject { runner.update_cached_info(architecture: '18-bit') } context 'when database was updated recently' do before do @@ -404,7 +405,7 @@ describe Ci::Runner do end it 'updates cache' do - expect_redis_update(:contacted_at, :name) + expect_redis_update(:architecture, :contacted_at) subject end @@ -416,26 +417,25 @@ describe Ci::Runner do end it 'updates database' do - expect_redis_update(:contacted_at, :name) + expect_redis_update(:architecture, :contacted_at) - expect { subject }.to change { runner.reload.contacted_at } - .and change { runner.reload.name } + 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(:contacted_at, :name) + expect_redis_update(:architecture, :contacted_at) subject end end def expect_redis_update(*params) - redis = double - expect(Gitlab::Redis::SharedState).to receive(:with).and_yield(redis) - - params.each do |param| - redis_key = "#{runner.send(:runner_info_redis_cache_key)}:#{param}" - expect(redis).to receive(:set).with(redis_key, anything) + 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 end end end