Don't remove the key when job is executed

CHANGELOG.md

@@ -1,3 +1,6 @@
+## [Unreleased]
+- don't remove debounce key in redis to avoid invalid debouncing
+
 ## [2.0.0] - 2023-02-28
 Complete rewrite of the library:  
 - Instead of iterating through whole schedule set, sidekiq-debouncer will now cache debounce key in redis with a reference to the job.

README.md

@@ -88,8 +88,8 @@ Keep in mind that the result of the debounce method will be converted to string,
 
 In the application, call `MyWorker.perform_async(...)` as usual. Everytime you call this function, `MyWorker`'s execution will be postponed by 5 minutes. After that time `MyWorker` will receive a method call `perform` with an array of arguments that were provided to the `MyWorker.perform_async(...)` calls.
 
-To avoid keeping leftover keys in redis (for example, when job was manually removed from schedule set), all additional keys are created with TTL.
-It's 7 days by default and should be ok in most of the cases. If you are debouncing your jobs in higher interval than that, you can overwrite this setting:
+To avoid keeping leftover keys in redis, all additional keys are created with TTL.
+It's 24 hours by default and should be ok in most of the cases. If you are debouncing your jobs in higher interval than that, you can overwrite this setting:
 
 ```ruby
 Sidekiq.configure_client do |config|

lib/sidekiq/debouncer/middleware/client.rb

@@ -17,7 +17,7 @@ class Client
         REDIS_ERROR_CLASS = defined?(RedisClient::CommandError) ? RedisClient::CommandError : Redis::CommandError
 
         def initialize(options = {})
-          @debounce_key_ttl = options.fetch(:ttl, 60 * 60 * 24 * 7) # 7 days by default
+          @debounce_key_ttl = options.fetch(:ttl, 60 * 60 * 24) # 24 hours by default
         end
 
         def call(worker_class, job, _queue, _redis_pool)

lib/sidekiq/debouncer/middleware/server.rb

@@ -3,20 +3,13 @@
 module Sidekiq
   module Debouncer
     module Middleware
-      # Server middleware removes debounce key from redis before executing the job
+      # wrap args into array because sidekiq uses splat while calling perform
       class Server
         include Sidekiq::ServerMiddleware
 
         def call(_worker, job, _queue)
           if job.key?("debounce_key")
-            # skip if job comes from dead or retry set
-            unless job.key?("failed_at")
-              redis do |connection|
-                connection.call("DEL", job["debounce_key"])
-              end
-            end
-
-            job["args"] = [job["args"]] # wrap args into array because sidekiq uses splat while calling perform
+            job["args"] = [job["args"]]
           end
 
           yield

sidekiq-debouncer.gemspec

@@ -12,6 +12,7 @@ Gem::Specification.new do |gem|
   gem.email = ["sebcioz@gmail.com", "kukicola@gmail.com"]
   gem.summary = "Sidekiq extension that adds the ability to debounce job execution"
   gem.description = <<~DESCRIPTION
+    Sidekiq extension that adds the ability to debounce job execution.
     Worker will postpone its execution after `wait time` have elapsed since the last time it was invoked.
     Useful for implementing behavior that should only happen after the input has stopped arriving.
   DESCRIPTION

spec/sidekiq/middleware/client_spec.rb

@@ -106,7 +106,7 @@
       end
     end
 
-    context "1 task, 3 minutes break, 1 task, 6 minutes break, 1 task" do
+    context "1 task, 3 minutes break, 1 task, 6 minutes break, 2 tasks" do
       it "executes two tasks after 8 minutes, the last one in 14 minutes" do
         TestWorker.perform_async("A", "job 1")
 
@@ -120,10 +120,14 @@
         expect(schedule_set.size).to eq(1)
 
         queue_job = queue.first
-        scheduled = schedule_set.first
-
         expect(queue_job.args).to eq([["A", "job 1"], ["A", "job 2"]])
-        expect(scheduled.args).to eq([["A", "job 3"]])
+
+        processor.process_one
+        TestWorker.perform_async("A", "job 4")
+        expect(schedule_set.size).to eq(1)
+
+        scheduled = schedule_set.first
+        expect(scheduled.args).to eq([["A", "job 3"], ["A", "job 4"]])
         expect(scheduled.at.to_i).to be((time_start + 14 * 60).to_i)
       end
     end

spec/sidekiq/middleware/server_spec.rb

@@ -20,34 +20,6 @@
 
       expect_any_instance_of(TestWorker).to receive(:perform).with([["A", "job 1"], ["A", "job 2"]]).and_call_original
       processor.process_one
-
-      expect(Sidekiq.redis { |con| con.call("GET", "debounce/TestWorker/A") }).to be_nil
-    end
-  end
-
-  context "retry job" do
-    it "removes debounce key and wrap arguments only on first call" do
-      TestWorker.perform_async("A", "job 1")
-
-      expect(Sidekiq.redis { |con| con.call("GET", "debounce/TestWorker/A") }).not_to be_nil
-
-      Timecop.freeze(time_start + 10 * 60)
-      puller.enqueue # job now in the queue
-
-      allow_any_instance_of(TestWorker).to receive(:perform).with([["A", "job 1"]]).and_raise("something")
-      processor.process_one rescue nil # standard:disable Style/RescueModifier
-
-      expect(Sidekiq.redis { |con| con.call("GET", "debounce/TestWorker/A") }).to be_nil
-
-      Timecop.freeze(time_start + 20 * 60)
-      puller.enqueue # job in the queue again
-
-      TestWorker.perform_async("A", "job 2")
-      expect(Sidekiq.redis { |con| con.call("GET", "debounce/TestWorker/A") }).not_to be_nil
-
-      processor.process_one rescue nil # standard:disable Style/RescueModifier
-
-      expect(Sidekiq.redis { |con| con.call("GET", "debounce/TestWorker/A") }).not_to be_nil
     end
   end