Remove deprecated command usage, fixes #5788

sidekiq · Feb 15, 2023 · 1aca434 · 1aca434
1 parent 0b753ff
commit 1aca434
Show file tree

Hide file tree

Showing 12 changed files with 52 additions and 26 deletions.
diff --git a/Changes.md b/Changes.md
@@ -2,6 +2,17 @@
 
 [Sidekiq Changes](https://github.com/sidekiq/sidekiq/blob/main/Changes.md) | [Sidekiq Pro Changes](https://github.com/sidekiq/sidekiq/blob/main/Pro-Changes.md) | [Sidekiq Enterprise Changes](https://github.com/sidekiq/sidekiq/blob/main/Ent-Changes.md)
 
+7.1.0
+----------
+
+- Change return value of `push_bulk` to map 1-to-1 with arguments.
+  If you call `push_bulk(args: [[1], [2], [3]])`, you will now always get
+  an array of 3 values as the result: `["jid1", nil, "jid3"]` where nil means
+  that particular job did not push successfully (possibly due to middleware
+  stopping it). Previously nil values were removed so it was impossible to tell
+  which jobs pushed successfully and which did not.
+- Migrate away from all deprecated Redis commands including `rpoplpush zrangebyscore zrevrange zrevrangebyscore getset hmset setex setnx`.
+
 7.0.5
 ----------
 

diff --git a/Ent-Changes.md b/Ent-Changes.md
@@ -4,6 +4,12 @@
 
 Please see [sidekiq.org](https://sidekiq.org) for more details and how to buy.
 
+HEAD
+---------
+
+- Add new points-based rate limiter popular with GraphQL endpoints at Shopify, GitHub, et al.
+  Thanks to Thad Sauter of NexHealth for contributing the initial skeleton. [#5757]
+
 7.0.4
 ---------
 

diff --git a/Pro-Changes.md b/Pro-Changes.md
@@ -4,6 +4,14 @@
 
 Please see [sidekiq.org](https://sidekiq.org/) for more details and how to buy.
 
+7.1.0
+---------
+
+- **SEMANTIC CHANGE**: Empty batches now automatically create an empty job.
+  This ensures callbacks are fired even if no jobs are created. The behavior
+  of empty batches has always been documented as undefined so this is not
+  considered a breaking change.
+
 7.0.7
 ---------
 

diff --git a/lib/sidekiq/api.rb b/lib/sidekiq/api.rb
@@ -548,7 +548,7 @@ def error?
     def remove_job
       Sidekiq.redis do |conn|
         results = conn.multi { |transaction|
-          transaction.zrangebyscore(parent.name, score, score)
+          transaction.zrange(parent.name, score, score, "BYSCORE")
           transaction.zremrangebyscore(parent.name, score, score)
         }.first
 
@@ -683,7 +683,7 @@ def fetch(score, jid = nil)
         end
 
       elements = Sidekiq.redis { |conn|
-        conn.zrangebyscore(name, begin_score, end_score, withscores: true)
+        conn.zrange(name, begin_score, end_score, "BYSCORE", withscores: true)
       }
 
       elements.each_with_object([]) do |element, result|
@@ -724,7 +724,7 @@ def delete_by_value(name, value)
     # @api private
     def delete_by_jid(score, jid)
       Sidekiq.redis do |conn|
-        elements = conn.zrangebyscore(name, score, score)
+        elements = conn.zrange(name, score, score, "BYSCORE")
         elements.each do |element|
           if element.index(jid)
             message = Sidekiq.load_json(element)

diff --git a/lib/sidekiq/launcher.rb b/lib/sidekiq/launcher.rb
@@ -165,7 +165,7 @@ def ❤
           conn.multi { |transaction|
             transaction.sadd("processes", [key])
             transaction.exists(key)
-            transaction.hmset(key, "info", to_json,
+            transaction.hset(key, "info", to_json,
               "busy", curstate.size,
               "beat", Time.now.to_f,
               "rtt_us", rtt,

diff --git a/lib/sidekiq/paginator.rb b/lib/sidekiq/paginator.rb
@@ -19,7 +19,7 @@ def page(key, pageidx = 1, page_size = 25, opts = nil)
           total_size, items = conn.multi { |transaction|
             transaction.zcard(key)
             if rev
-              transaction.zrevrange(key, starting, ending, withscores: true)
+              transaction.zrange(key, starting, ending, "REV", withscores: true)
             else
               transaction.zrange(key, starting, ending, withscores: true)
             end

diff --git a/lib/sidekiq/redis_client_adapter.rb b/lib/sidekiq/redis_client_adapter.rb
@@ -9,21 +9,22 @@ class RedisClientAdapter
     CommandError = RedisClient::CommandError
 
     module CompatMethods
-      # TODO Deprecate and remove this
       def info
         @client.call("INFO") { |i| i.lines(chomp: true).map { |l| l.split(":", 2) }.select { |l| l.size == 2 }.to_h }
       end
 
-      # TODO Deprecate and remove this
       def evalsha(sha, keys, argv)
         @client.call("EVALSHA", sha, keys.size, *keys, *argv)
       end
 
       private
 
+      DEPRECATED_COMMANDS = %i[rpoplpush zrangebyscore zrevrange zrevrangebyscore getset hmset setex setnx]
+
       # this allows us to use methods like `conn.hmset(...)` instead of having to use
       # redis-client's native `conn.call("hmset", ...)`
       def method_missing(*args, &block)
+        warn("Deprecated Redis command: #{args.first} at #{caller(1..1)}") if DEPRECATED_COMMANDS.include?(args.first)
         @client.call(*args, *block)
       end
       ruby2_keywords :method_missing if respond_to?(:ruby2_keywords, true)

diff --git a/lib/sidekiq/scheduled.rb b/lib/sidekiq/scheduled.rb
@@ -12,7 +12,7 @@ class Enq
 
       LUA_ZPOPBYSCORE = <<~LUA
         local key, now = KEYS[1], ARGV[1]
-        local jobs = redis.call("zrangebyscore", key, "-inf", now, "limit", 0, 1)
+        local jobs = redis.call("zrange", key, "-inf", now, "byscore", "limit", 0, 1)
         if jobs[1] then
           redis.call("zrem", key, jobs[1])
           return jobs[1]

diff --git a/lib/sidekiq/version.rb b/lib/sidekiq/version.rb
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
 module Sidekiq
-  VERSION = "7.0.5"
+  VERSION = "7.1.0"
   MAJOR = 7
 end
diff --git a/test/api_test.rb b/test/api_test.rb
@@ -541,7 +541,7 @@ class JobWithTags
       @cfg.redis do |conn|
         conn.multi do |transaction|
           transaction.sadd("processes", [odata["key"]])
-          transaction.hmset(odata["key"], "info", Sidekiq.dump_json(odata), "busy", 10, "beat", time)
+          transaction.hset(odata["key"], "info", Sidekiq.dump_json(odata), "busy", 10, "beat", time)
           transaction.sadd("processes", ["fake:pid"])
         end
       end
@@ -581,7 +581,7 @@ class JobWithTags
       @cfg.redis do |conn|
         conn.multi do |transaction|
           transaction.sadd("processes", [odata["key"]])
-          transaction.hmset(odata["key"], "info", Sidekiq.dump_json(odata), "busy", 10, "beat", time)
+          transaction.hset(odata["key"], "info", Sidekiq.dump_json(odata), "busy", 10, "beat", time)
         end
       end
 
@@ -619,13 +619,13 @@ class JobWithTags
       pdata = {"pid" => $$, "hostname" => hn, "started_at" => Time.now.to_i}
       @cfg.redis do |conn|
         conn.sadd("processes", [key])
-        conn.hmset(key, "info", Sidekiq.dump_json(pdata), "busy", 0, "beat", Time.now.to_f)
+        conn.hset(key, "info", Sidekiq.dump_json(pdata), "busy", 0, "beat", Time.now.to_f)
       end
 
       s = "#{key}:work"
       data = Sidekiq.dump_json({"payload" => "{}", "queue" => "default", "run_at" => Time.now.to_i})
       @cfg.redis do |c|
-        c.hmset(s, "1234", data)
+        c.hset(s, "1234", data)
       end
 
       w.each do |p, x, y|
@@ -640,8 +640,8 @@ class JobWithTags
       data = Sidekiq.dump_json({"payload" => {}, "queue" => "default", "run_at" => (Time.now.to_i - 2 * 60 * 60)})
       @cfg.redis do |c|
         c.multi do |transaction|
-          transaction.hmset(s, "5678", data)
-          transaction.hmset("b#{s}", "5678", data)
+          transaction.hset(s, "5678", data)
+          transaction.hset("b#{s}", "5678", data)
         end
       end
 
@@ -671,7 +671,7 @@ class JobWithTags
       key = "#{data["hostname"]}:#{data["pid"]}"
       @cfg.redis do |conn|
         conn.sadd("processes", [key])
-        conn.hmset(key, "info", Sidekiq.dump_json(data), "busy", 0, "beat", Time.now.to_f)
+        conn.hset(key, "info", Sidekiq.dump_json(data), "busy", 0, "beat", Time.now.to_f)
       end
       ps = Sidekiq::ProcessSet.new
       assert_equal 1, ps.size

diff --git a/test/web_helpers_test.rb b/test/web_helpers_test.rb
@@ -157,7 +157,7 @@ def params
 
       Sidekiq.redis do |conn|
         conn.sadd("processes", [key])
-        conn.hmset(key, "info", Sidekiq.dump_json(pdata), "busy", 0, "beat", Time.now.to_f)
+        conn.hset(key, "info", Sidekiq.dump_json(pdata), "busy", 0, "beat", Time.now.to_f)
       end
     end
 
@@ -174,7 +174,7 @@ def params
 
       Sidekiq.redis do |conn|
         conn.sadd("processes", [key])
-        conn.hmset(key, "info", Sidekiq.dump_json(pdata), "busy", 0, "beat", Time.now.to_f)
+        conn.hset(key, "info", Sidekiq.dump_json(pdata), "busy", 0, "beat", Time.now.to_f)
       end
     end
 

diff --git a/test/web_test.rb b/test/web_test.rb
@@ -69,10 +69,10 @@ def job_params(job, score)
       @config.redis do |conn|
         conn.incr("busy")
         conn.sadd("processes", ["foo:1234"])
-        conn.hmset("foo:1234", "info", Sidekiq.dump_json("hostname" => "foo", "started_at" => Time.now.to_f, "queues" => [], "concurrency" => 10), "at", Time.now.to_f, "busy", 4)
+        conn.hset("foo:1234", "info", Sidekiq.dump_json("hostname" => "foo", "started_at" => Time.now.to_f, "queues" => [], "concurrency" => 10), "at", Time.now.to_f, "busy", 4)
         identity = "foo:1234:work"
         hash = {queue: "critical", payload: {"class" => WebJob.name, "args" => [1, "abc"]}, run_at: Time.now.to_i}
-        conn.hmset(identity, 1001, Sidekiq.dump_json(hash))
+        conn.hset(identity, 1001, Sidekiq.dump_json(hash))
       end
       assert_equal ["1001"], Sidekiq::WorkSet.new.map { |pid, tid, data| tid }
 
@@ -88,7 +88,7 @@ def job_params(job, score)
       signals_key = "#{identity}-signals"
       @config.redis do |conn|
         conn.sadd("processes", [identity])
-        conn.hmset(identity, "info", Sidekiq.dump_json("hostname" => "foo", "identity" => identity), "at", Time.now.to_f, "busy", 0)
+        conn.hset(identity, "info", Sidekiq.dump_json("hostname" => "foo", "identity" => identity), "at", Time.now.to_f, "busy", 0)
       end
 
       assert_nil @config.redis { |c| c.lpop signals_key }
@@ -102,7 +102,7 @@ def job_params(job, score)
       signals_key = "#{identity}-signals"
       @config.redis do |conn|
         conn.sadd("processes", [identity])
-        conn.hmset(identity, "info", Sidekiq.dump_json("hostname" => "foo", "identity" => identity), "at", Time.now.to_f, "busy", 0)
+        conn.hset(identity, "info", Sidekiq.dump_json("hostname" => "foo", "identity" => identity), "at", Time.now.to_f, "busy", 0)
       end
 
       assert_nil @config.redis { |c| c.lpop signals_key }
@@ -468,10 +468,10 @@ def job_params(job, score)
     @config.redis do |conn|
       pro = "foo:1234"
       conn.sadd("processes", [pro])
-      conn.hmset(pro, "info", Sidekiq.dump_json("started_at" => Time.now.to_f, "labels" => ["frumduz"], "queues" => [], "concurrency" => 10), "busy", 1, "beat", Time.now.to_f)
+      conn.hset(pro, "info", Sidekiq.dump_json("started_at" => Time.now.to_f, "labels" => ["frumduz"], "queues" => [], "concurrency" => 10), "busy", 1, "beat", Time.now.to_f)
       identity = "#{pro}:work"
       hash = {queue: "critical", payload: {"class" => "FailJob", "args" => ["<a>hello</a>"]}, run_at: Time.now.to_i}
-      conn.hmset(identity, 100001, Sidekiq.dump_json(hash))
+      conn.hset(identity, 100001, Sidekiq.dump_json(hash))
       conn.incr("busy")
     end
 
@@ -740,8 +740,8 @@ def add_worker
     @config.redis do |conn|
       conn.multi do |transaction|
         transaction.sadd("processes", [key])
-        transaction.hmset(key, "info", Sidekiq.dump_json("hostname" => "foo", "started_at" => Time.now.to_f, "queues" => []), "at", Time.now.to_f, "busy", 4)
-        transaction.hmset("#{key}:work", Time.now.to_f, msg)
+        transaction.hset(key, "info", Sidekiq.dump_json("hostname" => "foo", "started_at" => Time.now.to_f, "queues" => []), "at", Time.now.to_f, "busy", 4)
+        transaction.hset("#{key}:work", Time.now.to_f, msg)
       end
     end
   end