Commit b5444257 authored by Manoj M J's avatar Manoj M J Committed by Bob Van Landuyt

Resolve "Allow deduplication for UserRefreshFromReplicaWorker, while still...

Resolve "Allow deduplication for UserRefreshFromReplicaWorker, while still reading from the replica database."
parent af715ba0
...@@ -37,7 +37,7 @@ ...@@ -37,7 +37,7 @@
:urgency: :low :urgency: :low
:resource_boundary: :unknown :resource_boundary: :unknown
:weight: 1 :weight: 1
:idempotent: :idempotent: true
:tags: [] :tags: []
- :name: authorized_project_update:authorized_project_update_user_refresh_over_user_range - :name: authorized_project_update:authorized_project_update_user_refresh_over_user_range
:worker_name: AuthorizedProjectUpdate::UserRefreshOverUserRangeWorker :worker_name: AuthorizedProjectUpdate::UserRefreshOverUserRangeWorker
......
# frozen_string_literal: true # frozen_string_literal: true
module AuthorizedProjectUpdate module AuthorizedProjectUpdate
class UserRefreshFromReplicaWorker # rubocop:disable Scalability/IdempotentWorker class UserRefreshFromReplicaWorker
include ApplicationWorker include ApplicationWorker
sidekiq_options retry: 3 sidekiq_options retry: 3
...@@ -9,31 +9,36 @@ module AuthorizedProjectUpdate ...@@ -9,31 +9,36 @@ module AuthorizedProjectUpdate
urgency :low urgency :low
queue_namespace :authorized_project_update queue_namespace :authorized_project_update
# This job will not be deduplicated since it is marked with idempotent!
# `data_consistency :delayed` and not `idempotent!`
# See https://gitlab.com/gitlab-org/gitlab/-/issues/325291
deduplicate :until_executing, including_scheduled: true deduplicate :until_executing, including_scheduled: true
data_consistency :delayed
def perform(user_id) def perform(user_id)
user = User.find_by_id(user_id)
return unless user
if Feature.enabled?(:user_refresh_from_replica_worker_uses_replica_db) if Feature.enabled?(:user_refresh_from_replica_worker_uses_replica_db)
enqueue_project_authorizations_refresh(user) if project_authorizations_needs_refresh?(user) use_replica_if_available do
user = User.find_by_id(user_id)
if user && project_authorizations_needs_refresh?(user)
enqueue_project_authorizations_refresh(user)
end
end
else else
use_primary_database user = User.find_by_id(user_id)
return unless user
user.refresh_authorized_projects(source: self.class.name) user.refresh_authorized_projects(source: self.class.name)
end end
end end
private private
def use_primary_database # We use this approach instead of specifying `data_consistency :delayed` because these jobs
if ::Gitlab::Database::LoadBalancing.enable? # are enqueued in large numbers, and using `data_consistency :delayed`
::Gitlab::Database::LoadBalancing::Session.current.use_primary! # does not allow us to deduplicate these jobs.
end # https://gitlab.com/gitlab-org/gitlab/-/issues/325291
def use_replica_if_available(&block)
return yield unless ::Gitlab::Database::LoadBalancing.enable?
::Gitlab::Database::LoadBalancing::Session.current.use_replicas_for_read_queries(&block)
end end
def project_authorizations_needs_refresh?(user) def project_authorizations_needs_refresh?(user)
......
...@@ -12,9 +12,9 @@ RSpec.describe AuthorizedProjectUpdate::UserRefreshFromReplicaWorker do ...@@ -12,9 +12,9 @@ RSpec.describe AuthorizedProjectUpdate::UserRefreshFromReplicaWorker do
expect(described_class.get_urgency).to eq(:low) expect(described_class.get_urgency).to eq(:low)
end end
it_behaves_like 'worker with data consistency', it_behaves_like 'an idempotent worker' do
described_class, let(:job_args) { user.id }
data_consistency: :delayed end
describe '#perform' do describe '#perform' do
it 'checks if a project_authorization refresh is needed for the user' do it 'checks if a project_authorization refresh is needed for the user' do
...@@ -44,22 +44,21 @@ RSpec.describe AuthorizedProjectUpdate::UserRefreshFromReplicaWorker do ...@@ -44,22 +44,21 @@ RSpec.describe AuthorizedProjectUpdate::UserRefreshFromReplicaWorker do
end end
end end
context 'when the feature flag `user_refresh_from_replica_worker_uses_replica_db` is disabled' do context 'with load balancing enabled' do
before do before do
stub_feature_flags(user_refresh_from_replica_worker_uses_replica_db: false) allow(Gitlab::Database::LoadBalancing).to receive(:enable?).and_return(true)
end end
context 'when load balancing is enabled' do it 'reads from the replica database' do
before do expect(Gitlab::Database::LoadBalancing::Session.current).to receive(:use_replicas_for_read_queries).and_call_original
allow(Gitlab::Database::LoadBalancing).to receive(:enable?).and_return(true)
end
it 'reads from the primary database' do execute_worker
expect(Gitlab::Database::LoadBalancing::Session.current) end
.to receive(:use_primary!) end
execute_worker context 'when the feature flag `user_refresh_from_replica_worker_uses_replica_db` is disabled' do
end before do
stub_feature_flags(user_refresh_from_replica_worker_uses_replica_db: false)
end end
it 'calls Users::RefreshAuthorizedProjectsService' do it 'calls Users::RefreshAuthorizedProjectsService' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment