Skip to content

G
gitlab-ce
Commit 8ba2c6bd authored by Mikołaj Wawrzyniak's avatar Mikołaj Wawrzyniak
Browse files
Options

Merge branch 'remove-rubocop-rule-for-data-consistency-with-deduplication' into 'master' 

Remove rubocop rule for lb with deduplication

See merge request gitlab-org/gitlab!79241
parents ee294618 559f1ace
Show whitespace changes
Inline Side-by-side
Showing with 0 additions and 325 deletions
app/workers/authorized_project_update/user_refresh_over_user_range_worker.rb
View file @ 8ba2c6bd
......@@ -20,7 +20,6 @@ module AuthorizedProjectUpdate
urgency :low
queue_namespace :authorized_project_update
deduplicate :until_executing, including_scheduled: true
data_consistency :delayed
idempotent!
......
app/workers/expire_job_cache_worker.rb
View file @ 8ba2c6bd
......@@ -10,8 +10,6 @@ class ExpireJobCacheWorker # rubocop:disable Scalability/IdempotentWorker
queue_namespace :pipeline_cache
urgency :high
deduplicate :until_executing, including_scheduled: true
idempotent!
def perform(job_id)
......
ee/app/workers/groups/create_event_worker.rb
View file @ 8ba2c6bd
......@@ -4,9 +4,7 @@ module Groups
class CreateEventWorker
include ApplicationWorker
deduplicate :until_executing, including_scheduled: true
data_consistency :delayed
idempotent!
feature_category :onboarding
urgency :throttled
......
rubocop/cop/sidekiq_load_balancing/worker_data_consistency_with_deduplication.rb deleted 100644 → 0
View file @ ee294618
# frozen_string_literal: true
require_relative '../../code_reuse_helpers'
module RuboCop
module Cop
module SidekiqLoadBalancing
# This cop checks for including_scheduled: true option in idempotent Sidekiq workers that utilize load balancing capabilities.
#
# @example
#
# # bad
# class BadWorker
# include ApplicationWorker
#
# data_consistency :delayed
# idempotent!
#
# def perform
# end
# end
#
# # bad
# class BadWorker
# include ApplicationWorker
#
# data_consistency :delayed
#
# deduplicate :until_executing
# idempotent!
#
# def perform
# end
# end
#
# # good
# class GoodWorker
# include ApplicationWorker
#
# data_consistency :delayed
#
# deduplicate :until_executing, including_scheduled: true
# idempotent!
#
# def perform
# end
# end
#
class WorkerDataConsistencyWithDeduplication < RuboCop::Cop::Base
include CodeReuseHelpers
extend AutoCorrector
HELP_LINK = 'https://docs.gitlab.com/ee/development/sidekiq_style_guide.html#scheduling-jobs-in-the-future'
REPLACEMENT = ', including_scheduled: true'
DEFAULT_STRATEGY = ':until_executing'
MSG = <<~MSG
Workers that declare either `:sticky` or `:delayed` data consistency become eligible for database load-balancing.
In both cases, jobs are enqueued with a short delay.
If you do want to deduplicate jobs that utilize load-balancing, you need to specify including_scheduled: true
argument when defining deduplication strategy.
See #{HELP_LINK} for a more detailed explanation of these settings.
MSG
def_node_search :application_worker?, <<~PATTERN
`(send nil? :include (const nil? :ApplicationWorker))
PATTERN
def_node_search :idempotent_worker?, <<~PATTERN
`(send nil? :idempotent!)
PATTERN
def_node_search :data_consistency_defined?, <<~PATTERN
`(send nil? :data_consistency (sym {:sticky :delayed }))
PATTERN
def_node_matcher :including_scheduled?, <<~PATTERN
`(hash <(pair (sym :including_scheduled) (%1)) ...>)
PATTERN
def_node_matcher :deduplicate_strategy?, <<~PATTERN
`(send nil? :deduplicate (sym $_) $(...)?)
PATTERN
def on_class(node)
return unless in_worker?(node)
return unless application_worker?(node)
return unless idempotent_worker?(node)
return unless data_consistency_defined?(node)
@strategy, options = deduplicate_strategy?(node)
including_scheduled = false
if options
@deduplicate_options = options[0]
including_scheduled = including_scheduled?(@deduplicate_options, :true) # rubocop:disable Lint/BooleanSymbol
end
@offense = !(including_scheduled || @strategy == :none)
end
def on_send(node)
return unless offense
if node.children[1] == :deduplicate
add_offense(node.loc.expression) do |corrector|
autocorrect_deduplicate_strategy(node, corrector)
end
elsif node.children[1] == :idempotent! && !strategy
add_offense(node.loc.expression) do |corrector|
autocorrect_missing_deduplicate_strategy(node, corrector)
end
end
end
private
attr_reader :offense, :deduplicate_options, :strategy
def autocorrect_deduplicate_with_options(corrector)
if including_scheduled?(deduplicate_options, :false) # rubocop:disable Lint/BooleanSymbol
replacement = deduplicate_options.source.sub("including_scheduled: false", "including_scheduled: true")
corrector.replace(deduplicate_options.loc.expression, replacement)
else
corrector.insert_after(deduplicate_options.loc.expression, REPLACEMENT)
end
end
def autocorrect_deduplicate_without_options(node, corrector)
corrector.insert_after(node.loc.expression, REPLACEMENT)
end
def autocorrect_missing_deduplicate_strategy(node, corrector)
indent_found = node.source_range.source_line =~ /^( +)/
# Get indentation size
whitespaces = Regexp.last_match(1).size if indent_found
replacement = "deduplicate #{DEFAULT_STRATEGY}#{REPLACEMENT}\n"
# Add indentation in the end since we are inserting a whole line before idempotent!
replacement += ' ' * whitespaces.to_i
corrector.insert_before(node.source_range, replacement)
end
def autocorrect_deduplicate_strategy(node, corrector)
if deduplicate_options
autocorrect_deduplicate_with_options(corrector)
else
autocorrect_deduplicate_without_options(node, corrector)
end
end
end
end
end
end
spec/rubocop/cop/sidekiq_load_balancing/worker_data_consistency_with_deduplication_spec.rb deleted 100644 → 0
View file @ ee294618
# frozen_string_literal: true
require 'fast_spec_helper'
require 'rspec-parameterized'
require_relative '../../../../rubocop/cop/sidekiq_load_balancing/worker_data_consistency_with_deduplication'
RSpec.describe RuboCop::Cop::SidekiqLoadBalancing::WorkerDataConsistencyWithDeduplication do
using RSpec::Parameterized::TableSyntax
subject(:cop) { described_class.new }
before do
allow(cop)
.to receive(:in_worker?)
.and_return(true)
end
where(:data_consistency) { %i[delayed sticky] }
with_them do
let(:strategy) { described_class::DEFAULT_STRATEGY }
let(:corrected) do
<<~CORRECTED
class SomeWorker
include ApplicationWorker
data_consistency :#{data_consistency}
deduplicate #{strategy}, including_scheduled: true
idempotent!
end
CORRECTED
end
context 'when deduplication strategy is not explicitly set' do
it 'registers an offense and corrects using default strategy' do
expect_offense(<<~CODE)
class SomeWorker
include ApplicationWorker
data_consistency :#{data_consistency}
idempotent!
^^^^^^^^^^^ Workers that declare either `:sticky` or `:delayed` data consistency [...]
end
CODE
expect_correction(corrected)
end
context 'when identation is different' do
let(:corrected) do
<<~CORRECTED
class SomeWorker
include ApplicationWorker
data_consistency :#{data_consistency}
deduplicate #{strategy}, including_scheduled: true
idempotent!
end
CORRECTED
end
it 'registers an offense and corrects with correct identation' do
expect_offense(<<~CODE)
class SomeWorker
include ApplicationWorker
data_consistency :#{data_consistency}
idempotent!
^^^^^^^^^^^ Workers that declare either `:sticky` or `:delayed` data consistency [...]
end
CODE
expect_correction(corrected)
end
end
end
context 'when deduplication strategy does not include including_scheduling option' do
let(:strategy) { ':until_executed' }
it 'registers an offense and corrects' do
expect_offense(<<~CODE)
class SomeWorker
include ApplicationWorker
data_consistency :#{data_consistency}
deduplicate :until_executed
^^^^^^^^^^^^^^^^^^^^^^^^^^^ Workers that declare either `:sticky` or `:delayed` data consistency [...]
idempotent!
end
CODE
expect_correction(corrected)
end
end
context 'when deduplication strategy has including_scheduling option disabled' do
let(:strategy) { ':until_executed' }
it 'registers an offense and corrects' do
expect_offense(<<~CODE)
class SomeWorker
include ApplicationWorker
data_consistency :#{data_consistency}
deduplicate :until_executed, including_scheduled: false
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Workers that declare either `:sticky` or `:delayed` data consistency [...]
idempotent!
end
CODE
expect_correction(corrected)
end
end
context "when deduplication strategy is :none" do
it 'does not register an offense' do
expect_no_offenses(<<~CODE)
class SomeWorker
include ApplicationWorker
data_consistency :always
deduplicate :none
idempotent!
end
CODE
end
end
context "when deduplication strategy has including_scheduling option enabled" do
it 'does not register an offense' do
expect_no_offenses(<<~CODE)
class SomeWorker
include ApplicationWorker
data_consistency :always
deduplicate :until_executing, including_scheduled: true
idempotent!
end
CODE
end
end
end
context "data_consistency: :always" do
it 'does not register an offense' do
expect_no_offenses(<<~CODE)
class SomeWorker
include ApplicationWorker
data_consistency :always
idempotent!
end
CODE
end
end
end
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7