Skip to content
代码片段 群组 项目
未验证 提交 64ce9ab6 编辑于 作者: Sylvester Chin's avatar Sylvester Chin 提交者: GitLab
浏览文件

Guard PATs `last_used_at` updates with exclusive lease

上级 b72dba0c
No related branches found
No related tags found
无相关合并请求
...@@ -2,6 +2,10 @@ ...@@ -2,6 +2,10 @@
module PersonalAccessTokens module PersonalAccessTokens
class LastUsedService class LastUsedService
include ExclusiveLeaseGuard
LEASE_TIMEOUT = 60.seconds.to_i
def initialize(personal_access_token) def initialize(personal_access_token)
@personal_access_token = personal_access_token @personal_access_token = personal_access_token
end end
...@@ -14,13 +18,35 @@ def execute ...@@ -14,13 +18,35 @@ def execute
# would be updated when using #touch). # would be updated when using #touch).
return unless update? return unless update?
::Gitlab::Database::LoadBalancing::Session.without_sticky_writes do with_lease do
@personal_access_token.update_column(:last_used_at, Time.zone.now) ::Gitlab::Database::LoadBalancing::Session.without_sticky_writes do
@personal_access_token.update_column(:last_used_at, Time.zone.now)
end
end end
end end
private private
def lease_timeout
LEASE_TIMEOUT
end
def lease_key
@lease_key ||= "pat:last_used_update_lock:#{@personal_access_token.id}"
end
def with_lease
return yield unless Feature.enabled?(
:use_lease_for_pat_last_used_update,
Feature.current_request,
type: :gitlab_com_derisk
)
try_obtain_lease do
yield
end
end
def update? def update?
return false if ::Gitlab::Database.read_only? return false if ::Gitlab::Database.read_only?
......
---
name: use_lease_for_pat_last_used_update
feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/468851
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/158577
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/471276
milestone: '17.2'
group: group::scalability
type: gitlab_com_derisk
default_enabled: false
...@@ -3,6 +3,8 @@ ...@@ -3,6 +3,8 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe PersonalAccessTokens::LastUsedService, feature_category: :system_access do RSpec.describe PersonalAccessTokens::LastUsedService, feature_category: :system_access do
include ExclusiveLeaseHelpers
describe '#execute' do describe '#execute' do
subject { described_class.new(personal_access_token).execute } subject { described_class.new(personal_access_token).execute }
...@@ -13,12 +15,56 @@ ...@@ -13,12 +15,56 @@
expect { subject }.to change { personal_access_token.last_used_at } expect { subject }.to change { personal_access_token.last_used_at }
end end
it 'obtains an exclusive lease before updating' do
Gitlab::Redis::SharedState.with do |redis|
expect(redis).to receive(:set).with(
"#{Gitlab::ExclusiveLease::PREFIX}:pat:last_used_update_lock:#{personal_access_token.id}",
anything,
nx: true,
ex: described_class::LEASE_TIMEOUT
).and_call_original
end
expect { subject }.to change { personal_access_token.last_used_at }
end
it 'does not run on read-only GitLab instances' do it 'does not run on read-only GitLab instances' do
allow(::Gitlab::Database).to receive(:read_only?).and_return(true) allow(::Gitlab::Database).to receive(:read_only?).and_return(true)
expect { subject }.not_to change { personal_access_token.last_used_at } expect { subject }.not_to change { personal_access_token.last_used_at }
end end
context 'when lease is already acquired by another process' do
let(:lease_key) { "pat:last_used_update_lock:#{personal_access_token.id}" }
before do
stub_exclusive_lease_taken(lease_key, timeout: described_class::LEASE_TIMEOUT)
end
it 'does not update last_used_at' do
expect { subject }.not_to change { personal_access_token.last_used_at }
end
end
context 'when use_lease_for_pat_last_used_update flag is disabled' do
before do
stub_feature_flags(use_lease_for_pat_last_used_update: false)
end
it 'does not obtain an exclusive lease before updating' do
Gitlab::Redis::SharedState.with do |redis|
expect(redis).not_to receive(:set).with(
"#{Gitlab::ExclusiveLease::PREFIX}:pat:last_used_update_lock:#{personal_access_token.id}",
anything,
nx: true,
ex: described_class::LEASE_TIMEOUT
)
end
expect { subject }.to change { personal_access_token.last_used_at }
end
end
context 'when database load balancing is configured' do context 'when database load balancing is configured' do
let!(:service) { described_class.new(personal_access_token) } let!(:service) { described_class.new(personal_access_token) }
......
0% 加载中 .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册