Skip to content

Commit

Permalink
Add helper to cleanup delayed sidekiq timechunks (#894)
Browse files Browse the repository at this point in the history
Mainly useful to avoid flaky tests when using a real redis database
and not flushing the database in between tests
  • Loading branch information
Drowze authored Dec 16, 2023
1 parent 63d6458 commit ea0ed0a
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 0 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

### New Features

* [#894](https://github.com/toptal/chewy/pull/894): Way of cleaning redis from artifacts left by `delayed_sidekiq` strategy which could potentially cause flaky tests. ([@Drowze](https://github.com/Drowze))

### Changes

### Bugs Fixed
Expand Down
6 changes: 6 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -848,6 +848,12 @@ Explicit call of the reindex using `:delayed_sidekiq` strategy with `:update_fie
CitiesIndex.import([1, 2, 3], update_fields: [:name], strategy: :delayed_sidekiq)
```
While running tests with delayed_sidekiq strategy and Sidekiq is using a real redis instance that is NOT cleaned up in between tests (via e.g. `Sidekiq.redis(&:flushdb)`), you'll want to cleanup some redis keys in between tests to avoid state leaking and flaky tests. Chewy provides a convenience method for that:
```ruby
# it might be a good idea to also add to your testing setup, e.g.: a rspec `before` hook
Chewy::Strategy::DelayedSidekiq.clear_timechunks!
```
#### `:active_job`
This does the same thing as `:atomic`, but using ActiveJob. This will inherit the ActiveJob configuration settings including the `active_job.queue_adapter` setting for the environment. Patch `Chewy::Strategy::ActiveJob::Worker` for index updates improving.
Expand Down
13 changes: 13 additions & 0 deletions lib/chewy/strategy/delayed_sidekiq.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,19 @@ class Strategy
class DelayedSidekiq < Sidekiq
require_relative 'delayed_sidekiq/scheduler'

# cleanup the redis sets used internally. Useful mainly in tests to avoid
# leak and potential flaky tests.
def self.clear_timechunks!
::Sidekiq.redis do |redis|
timechunk_sets = redis.smembers(Chewy::Strategy::DelayedSidekiq::Scheduler::ALL_SETS_KEY)
break if timechunk_sets.empty?

redis.pipelined do |pipeline|
timechunk_sets.each { |set| pipeline.del(set) }
end
end
end

def leave
@stash.each do |type, ids|
next if ids.empty?
Expand Down
3 changes: 3 additions & 0 deletions lib/chewy/strategy/delayed_sidekiq/scheduler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ class Scheduler
DEFAULT_MARGIN = 2
DEFAULT_QUEUE = 'chewy'
KEY_PREFIX = 'chewy:delayed_sidekiq'
ALL_SETS_KEY = "#{KEY_PREFIX}:all_sets".freeze
FALLBACK_FIELDS = 'all'
FIELDS_IDS_SEPARATOR = ';'
IDS_SEPARATOR = ','
Expand Down Expand Up @@ -68,8 +69,10 @@ def postpone
::Sidekiq.redis do |redis|
# warning: Redis#sadd will always return an Integer in Redis 5.0.0. Use Redis#sadd? instead
if redis.respond_to?(:sadd?)
redis.sadd?(ALL_SETS_KEY, timechunks_key)
redis.sadd?(timechunk_key, serialize_data)
else
redis.sadd(ALL_SETS_KEY, timechunks_key)
redis.sadd(timechunk_key, serialize_data)
end

Expand Down
12 changes: 12 additions & 0 deletions spec/chewy/strategy/delayed_sidekiq_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -186,5 +186,17 @@
end
end
end

describe '#clear_delayed_sidekiq_timechunks test helper' do
it 'clears redis from the timechunk sorted sets to avoid leak between tests' do
timechunks_set = -> { Sidekiq.redis { |redis| redis.zrange('chewy:delayed_sidekiq:CitiesIndex:timechunks', 0, -1) } }

expect { CitiesIndex.import!([1], strategy: :delayed_sidekiq) }
.to change { timechunks_set.call.size }.by(1)

expect { Chewy::Strategy::DelayedSidekiq.clear_timechunks! }
.to change { timechunks_set.call.size }.to(0)
end
end
end
end

0 comments on commit ea0ed0a

Please sign in to comment.