From 125e5cf03367e0dfd6f6d9ea43bd384f41dbb245 Mon Sep 17 00:00:00 2001 From: Rustam Sharshenov Date: Fri, 20 Sep 2024 10:23:49 +0100 Subject: [PATCH] Optimize bulk unlocking - Replace individual DEL commands with batched deletions with UNLINK - 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. --- CHANGELOG.md | 3 +++ lib/active_job/uniqueness/lock_manager.rb | 8 +++++++- spec/active_job/uniqueness/unlock_spec.rb | 22 ++++++++++++++++++++++ 3 files changed, 32 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 47da71e..6a2aaf8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - [#86](https://github.com/veeqo/activejob-uniqueness/pull/86) Add Rails 8.0 rc1 support by[@sharshenov](https://github.com/sharshenov) +### Changed +- [#82](https://github.com/veeqo/activejob-uniqueness/pull/82) Optimize bulk unlocking [@sharshenov](https://github.com/sharshenov) + ## [0.3.2](https://github.com/veeqo/activejob-uniqueness/compare/v0.3.1...v0.3.2) - 2024-08-16 ### Added diff --git a/lib/active_job/uniqueness/lock_manager.rb b/lib/active_job/uniqueness/lock_manager.rb index 96cc3dc..9b311f0 100644 --- a/lib/active_job/uniqueness/lock_manager.rb +++ b/lib/active_job/uniqueness/lock_manager.rb @@ -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('UNLINK', *keys) unless keys.empty? + end end end diff --git a/spec/active_job/uniqueness/unlock_spec.rb b/spec/active_job/uniqueness/unlock_spec.rb index ec2f668..a72cd7a 100644 --- a/spec/active_job/uniqueness/unlock_spec.rb +++ b/spec/active_job/uniqueness/unlock_spec.rb @@ -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_unlink_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 { unlink_commands_calls }.by(expected_number_of_unlink_commands) + end + + def unlink_commands_calls + info = redis.call('INFO', 'commandstats') + unlink_stats = info.split("\n").find { |line| line.start_with?('cmdstat_unlink:') } + return 0 unless unlink_stats + + unlink_stats.match(/cmdstat_unlink:calls=(\d+)/)[1].to_i + end + end end