Skip to content

Commit

Permalink
Optimize bulk unlocking
Browse files Browse the repository at this point in the history
- Replace individual DEL commands with batched deletions
- Use SCAN with COUNT parameter for better performance
- Reduce network round-trips between application and Redis
- Improve memory efficiency by processing keys in batches

This change enhances the performance of the delete_locks method, especially when dealing with a large number of keys. The new implementation uses the SCAN command with a COUNT parameter to retrieve keys in batches, reducing memory usage. It then deletes these keys in batches, significantly reducing the number of network round-trips to the Redis server.

While Lua scripting could potentially offer even greater performance benefits, we opted not to use it in this iteration. This decision maintains better readability and easier maintenance of the Ruby code, avoids potential issues with script caching across multiple Redis servers, and keeps the implementation consistent with other parts of the codebase. The current optimization strikes a balance between performance improvement and code simplicity.
  • Loading branch information
sharshenov committed Sep 20, 2024
1 parent cfbf2fe commit 3978afd
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 1 deletion.
8 changes: 7 additions & 1 deletion lib/active_job/uniqueness/lock_manager.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,17 @@ def delete_lock(resource)
true
end

DELETE_LOCKS_SCAN_COUNT = 1000

# Unlocks multiple resources by key wildcard.
def delete_locks(wildcard)
@servers.each do |server|
synced_redis_connection(server) do |conn|
conn.scan('MATCH', wildcard).each { |key| conn.call('DEL', key) }
cursor = 0
while cursor != '0'
cursor, keys = conn.call('SCAN', cursor, 'MATCH', wildcard, 'COUNT', DELETE_LOCKS_SCAN_COUNT)
conn.call('DEL', *keys) unless keys.empty?
end
end
end

Expand Down
22 changes: 22 additions & 0 deletions spec/active_job/uniqueness/unlock_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,4 +68,26 @@
end
end
end

describe 'bulk deletion' do
subject(:unlock!) { described_class.unlock! }

let(:expected_initial_number_of_locks) { 1_103 } # 1_100 + 2 + 1
let(:expected_number_of_delete_commands) { 2 } # 1103 / 1000 (ActiveJob::Uniqueness::LockManager::DELETE_LOCKS_SCAN_COUNT)

before { 1_100.times.each { |i| job_class.perform_later(3, i) } }

it 'removes locks efficiently' do
expect { unlock! }.to change { locks_count }.from(expected_initial_number_of_locks).to(0)
.and change { delete_commands_calls }.by(expected_number_of_delete_commands)
end

def delete_commands_calls
info = redis.call('INFO', 'commandstats')
del_stats = info.split("\n").find { |line| line.start_with?('cmdstat_del:') }
return 0 unless del_stats

del_stats.match(/cmdstat_del:calls=(\d+)/)[1].to_i
end
end
end

0 comments on commit 3978afd

Please sign in to comment.