Fix: UntilAndWhileExecuting (#627)

* Fix UntilAndWhileExecuting Locking mechanism was broken in #624. * Bump appraisals * Simplify access
mhenrixon · Jul 28, 2021 · da53bb2 · da53bb2
1 parent 1da6859
commit da53bb2
Show file tree

Hide file tree

Showing 9 changed files with 18 additions and 17 deletions.
diff --git a/gemfiles/sidekiq_5.0.gemfile b/gemfiles/sidekiq_5.0.gemfile
@@ -7,7 +7,7 @@ gem "bundler"
 gem "gem-release"
 gem "github-markup"
 gem "rack-test"
-gem "rake"
+gem "rake", "13.0.3"
 gem "rspec"
 gem "rspec-html-matchers"
 gem "rspec-its"

diff --git a/gemfiles/sidekiq_5.1.gemfile b/gemfiles/sidekiq_5.1.gemfile
@@ -7,7 +7,7 @@ gem "bundler"
 gem "gem-release"
 gem "github-markup"
 gem "rack-test"
-gem "rake"
+gem "rake", "13.0.3"
 gem "rspec"
 gem "rspec-html-matchers"
 gem "rspec-its"

diff --git a/gemfiles/sidekiq_5.2.gemfile b/gemfiles/sidekiq_5.2.gemfile
@@ -7,7 +7,7 @@ gem "bundler"
 gem "gem-release"
 gem "github-markup"
 gem "rack-test"
-gem "rake"
+gem "rake", "13.0.3"
 gem "rspec"
 gem "rspec-html-matchers"
 gem "rspec-its"

diff --git a/gemfiles/sidekiq_6.0.gemfile b/gemfiles/sidekiq_6.0.gemfile
@@ -7,7 +7,7 @@ gem "bundler"
 gem "gem-release"
 gem "github-markup"
 gem "rack-test"
-gem "rake"
+gem "rake", "13.0.3"
 gem "rspec"
 gem "rspec-html-matchers"
 gem "rspec-its"

diff --git a/gemfiles/sidekiq_6.1.gemfile b/gemfiles/sidekiq_6.1.gemfile
@@ -7,7 +7,7 @@ gem "bundler"
 gem "gem-release"
 gem "github-markup"
 gem "rack-test"
-gem "rake"
+gem "rake", "13.0.3"
 gem "rspec"
 gem "rspec-html-matchers"
 gem "rspec-its"

diff --git a/gemfiles/sidekiq_develop.gemfile b/gemfiles/sidekiq_develop.gemfile
@@ -7,7 +7,7 @@ gem "bundler"
 gem "gem-release"
 gem "github-markup"
 gem "rack-test"
-gem "rake"
+gem "rake", "13.0.3"
 gem "rspec"
 gem "rspec-html-matchers"
 gem "rspec-its"

diff --git a/lib/sidekiq_unique_jobs/locksmith.rb b/lib/sidekiq_unique_jobs/locksmith.rb
@@ -102,7 +102,7 @@ def execute(&block)
       raise SidekiqUniqueJobs::InvalidArgument, "#execute needs a block" unless block
 
       redis(redis_pool) do |conn|
-        lock!(conn, method(:primed_async), config.timeout, &block)
+        lock!(conn, method(:primed_async), &block)
       end
     end
 
@@ -179,6 +179,9 @@ def ==(other)
     #
     # Used to reduce some duplication from the two methods
     #
+    # @see lock
+    # @see execute
+    #
     # @param [Sidekiq::RedisConnection, ConnectionPool] conn the redis connection
     # @param [Method] primed_method reference to the method to use for getting a primed token
     #
@@ -238,7 +241,7 @@ def enqueue(conn)
     def primed_async(conn, wait = nil, &block)
       primed_jid = Concurrent::Promises
                    .future(conn) { |red_con| pop_queued(red_con, wait) }
-                   .value(add_drift(wait || config.ttl))
+                   .value(add_drift(wait || config.timeout))
 
       handle_primed(primed_jid, &block)
     end

diff --git a/lib/sidekiq_unique_jobs/timing.rb b/lib/sidekiq_unique_jobs/timing.rb
@@ -51,7 +51,7 @@ def clock_stamp
       if Process.const_defined?("CLOCK_MONOTONIC")
         Process.clock_gettime(Process::CLOCK_MONOTONIC)
       else
-        Time.now.to_f
+        now_f
       end
     end
   end

diff --git a/spec/sidekiq_unique_jobs/locksmith_spec.rb b/spec/sidekiq_unique_jobs/locksmith_spec.rb
@@ -29,6 +29,8 @@
       "queue" => queue,
     }
   end
+  let(:key_one)  { locksmith_one.key }
+  let(:key_two)  { locksmith_two.key }
   let(:item_two) { item_one.merge("jid" => jid_two) }
 
   describe "#to_s" do
@@ -288,13 +290,6 @@
   context "when lock_timeout is 1" do
     let(:lock_timeout) { 1 }
 
-    let(:slow_brpoplpush) do
-      proc do
-        sleep(0.75)
-        locksmith_one.job_id
-      end
-    end
-
     it "blocks other locks" do
       did_we_get_in = false
 
@@ -308,7 +303,10 @@
     end
 
     it "waits for brpoplpush when resolving the promise" do
-      allow(locksmith_one).to receive(:brpoplpush).with(anything, 1, &slow_brpoplpush)
+      allow(locksmith_one).to receive(:brpoplpush) do
+        sleep(0.75)
+        jid_one
+      end
 
       did_we_get_in = false
       locksmith_one.execute do
-Original file line number
+Diff line change
@@ Expand Up / @@ -51,7 +51,7 @@ def clock_stamp @@
           if Process.const_defined?("CLOCK_MONOTONIC")
             Process.clock_gettime(Process::CLOCK_MONOTONIC)
           else
-            Time.now.to_f
+            now_f
           end
         end
       end
@@ Expand Down @@