Fix poll interval not changing based on process count (#5513)

We should not memoize the value of `scaled_poll_interval` because this
changes based on the number of Sidekiq processes

Co-authored-by: Heinrich Lee Yu <[email protected]>

Author: engwan

lib/sidekiq/scheduled.rb

@@ -147,13 +147,16 @@ def random_poll_interval
         # As we run more processes, the scheduling interval average will approach an even spread
         # between 0 and poll interval so we don't need this artifical boost.
         #
-        if process_count < 10
+        count = process_count
+        interval = poll_interval_average(count)
+
+        if count < 10
           # For small clusters, calculate a random interval that is ±50% the desired average.
-          poll_interval_average * rand + poll_interval_average.to_f / 2
+          interval * rand + interval.to_f / 2
         else
           # With 10+ processes, we should have enough randomness to get decent polling
           # across the entire timespan
-          poll_interval_average * rand
+          interval * rand
         end
       end
 
@@ -170,14 +173,14 @@ def random_poll_interval
       # the same time: the thundering herd problem.
       #
       # We only do this if poll_interval_average is unset (the default).
-      def poll_interval_average
-        @config[:poll_interval_average] ||= scaled_poll_interval
+      def poll_interval_average(count)
+        @config[:poll_interval_average] || scaled_poll_interval(count)
       end
 
       # Calculates an average poll interval based on the number of known Sidekiq processes.
       # This minimizes a single point of failure by dispersing check-ins but without taxing
       # Redis if you run many Sidekiq processes.
-      def scaled_poll_interval
+      def scaled_poll_interval(process_count)
         process_count * @config[:average_scheduled_poll_interval]
       end
 

test/test_scheduled.rb

@@ -125,16 +125,35 @@ def with_sidekiq_option(name, value)
       end
     end
 
-    it "calculates an average poll interval based on the number of known Sidekiq processes" do
+    it "generates random intervals based on the number of known Sidekiq processes" do
       with_sidekiq_option(:average_scheduled_poll_interval, 10) do
-        3.times do |i|
+        intervals_count = 500
+
+        # Start with 10 processes
+        10.times do |i|
           Sidekiq.redis do |conn|
             conn.sadd("processes", ["process-#{i}"])
-            conn.hset("process-#{i}", "info", "")
           end
         end
 
-        assert_equal 30, @poller.send(:scaled_poll_interval)
+        intervals = Array.new(intervals_count) { @poller.send(:random_poll_interval) }
+        assert intervals.all? { |x| x.between?(0, 100) }
+
+        # Reduce to 3 processes
+        (3..9).each do |i|
+          Sidekiq.redis do |conn|
+            conn.srem("processes", ["process-#{i}"])
+          end
+        end
+
+        intervals = Array.new(intervals_count) { @poller.send(:random_poll_interval) }
+        assert intervals.all? { |x| x.between?(15, 45) }
+      end
+    end
+
+    it "calculates an average poll interval based on a given number of processes" do
+      with_sidekiq_option(:average_scheduled_poll_interval, 10) do
+        assert_equal 30, @poller.send(:scaled_poll_interval, 3)
       end
     end
   end