diff options
author | Yorick Peterse <yorickpeterse@gmail.com> | 2017-09-20 14:21:15 +0200 |
---|---|---|
committer | Yorick Peterse <yorickpeterse@gmail.com> | 2017-09-20 14:24:14 +0200 |
commit | b3566a01049cdfbde2a9221319601d8949c12a5a (patch) | |
tree | 873d695caaaec68afabf8146af5f83f0983be04e /spec/services/keys | |
parent | a09d032b2a64c7b6652dcd589de2d9bcba7d9613 (diff) | |
download | gitlab-ce-b3566a01049cdfbde2a9221319601d8949c12a5a.tar.gz |
Stop using Sidekiq for updating Key#last_used_atremove-use-key-worker
This makes things simpler as no scheduling is involved. Further we
remove the need for running a SELECT + UPDATE just to get the key and
update it, whereas we only need an UPDATE when setting last_used_at
directly in a request.
The added service class takes care of updating Key#last_used_at without
using Sidekiq. Further it makes sure we only try to obtain a Redis lease
if we're confident that we actually need to do so, instead of always
obtaining it. We also make sure to _only_ update last_used_at instead of
also updating updated_at.
Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/36663
Diffstat (limited to 'spec/services/keys')
-rw-r--r-- | spec/services/keys/last_used_service_spec.rb | 68 |
1 files changed, 68 insertions, 0 deletions
diff --git a/spec/services/keys/last_used_service_spec.rb b/spec/services/keys/last_used_service_spec.rb new file mode 100644 index 00000000000..edaace293ae --- /dev/null +++ b/spec/services/keys/last_used_service_spec.rb @@ -0,0 +1,68 @@ +require 'spec_helper' + +describe Keys::LastUsedService do + describe '#execute', :clean_gitlab_redis_shared_state do + it 'updates the key when it has not been used recently' do + key = create(:key, last_used_at: 1.year.ago) + time = Time.zone.now + + Timecop.freeze(time) { described_class.new(key).execute } + + expect(key.last_used_at).to eq(time) + end + + it 'does not update the key when it has been used recently' do + time = 1.minute.ago + key = create(:key, last_used_at: time) + + described_class.new(key).execute + + expect(key.last_used_at).to eq(time) + end + + it 'does not update the updated_at field' do + # Since a lot of these updates could happen in parallel for different keys + # we want these updates to be as lightweight as possible, hence we want to + # make sure we _only_ update last_used_at and not always updated_at. + key = create(:key, last_used_at: 1.year.ago) + + recorder = ActiveRecord::QueryRecorder.new do + described_class.new(key).execute + end + + expect(recorder.count).to eq(1) + expect(recorder.log[0]).not_to include('updated_at') + end + end + + describe '#update?', :clean_gitlab_redis_shared_state do + it 'returns true when no last used timestamp is present' do + key = build(:key, last_used_at: nil) + service = described_class.new(key) + + expect(service.update?).to eq(true) + end + + it 'returns true when the key needs to be updated' do + key = build(:key, last_used_at: 1.year.ago) + service = described_class.new(key) + + expect(service.update?).to eq(true) + end + + it 'returns false when a lease has already been obtained' do + key = build(:key, last_used_at: 1.year.ago) + service = described_class.new(key) + + expect(service.update?).to eq(true) + expect(service.update?).to eq(false) + end + + it 'returns false when the key does not yet need to be updated' do + key = build(:key, last_used_at: 1.minute.ago) + service = described_class.new(key) + + expect(service.update?).to eq(false) + end + end +end |