Merge pull request sidekiq#2473 from pik/develop

Do not perform redundant job look-up on delete for SortedEntry

lib/sidekiq/api.rb

@@ -369,11 +369,15 @@ def at
     end
 
     def delete
-      @parent.delete(score, jid)
+      if @value
+        @parent.delete_by_value(@parent.name, @value)
+      else
+        @parent.delete_by_jid(score, jid)
+      end
     end
 
     def reschedule(at)
-      @parent.delete(score, jid)
+      delete
       @parent.schedule(at, item)
     end
 
@@ -519,36 +523,30 @@ def find_job(jid)
       self.detect { |j| j.jid == jid }
     end
 
-    def delete(score, jid = nil)
-      if jid
-        elements = Sidekiq.redis do |conn|
-          conn.zrangebyscore(name, score, score)
-        end
+    def delete_by_value(name, value)
+      Sidekiq.redis do |conn|
+        ret = conn.zrem(name, value)
+        @_size -= 1 if ret
+        ret
+      end
+    end
 
-        elements_with_jid = elements.map do |element|
+    def delete_by_jid(score, jid)
+      Sidekiq.redis do |conn|
+        elements = conn.zrangebyscore(name, score, score)
+        elements.each do |element|
           message = Sidekiq.load_json(element)
-
           if message["jid"] == jid
-            _, @_size = Sidekiq.redis do |conn|
-              conn.multi do
-                conn.zrem(name, element)
-                conn.zcard name
-              end
-            end
-          end
-        end
-        elements_with_jid.count != 0
-      else
-        count, @_size = Sidekiq.redis do |conn|
-          conn.multi do
-            conn.zremrangebyscore(name, score, score)
-            conn.zcard name
+            ret = conn.zrem(name, element)
+            @_size -= 1 if ret
+            break ret
           end
+          false
         end
-        count != 0
       end
     end
 
+    alias_method :delete, :delete_by_jid
   end
 
   ##

test/test_api.rb

@@ -346,14 +346,12 @@ class ApiWorker
       assert_in_delta Time.now.to_f, retri.at.to_f, 0.02
     end
 
-    it 'can delete multiple retries from score' do
-      same_time = Time.now.to_f
-      add_retry('bob1', same_time)
-      add_retry('bob2', same_time)
-      r = Sidekiq::RetrySet.new
-      assert_equal 2, r.size
-      Sidekiq::RetrySet.new.delete(same_time)
-      assert_equal 0, r.size
+    it 'requires a jid to delete an entry' do
+      start_time = Time.now.to_f
+      add_retry('bob2', Time.now.to_f)
+      assert_raises(ArgumentError) do
+        Sidekiq::RetrySet.new.delete(start_time)
+      end
     end
 
     it 'can delete a single retry from score and jid' do