before_delayed_enqueue is called for recurring/cron jobs

[top-sigrid]

Description

When resque-scheduler fires a recurring/cron job via enqueue_recurring, it calls before_delayed_enqueue even though the job was never in the delayed queue. This hook should only be called when a job is being moved from the delayed queue to the work queue.

Expected Behavior

• before_delayed_enqueue should only be called when a job is being transferred from the delayed queue to the regular queue
• Recurring/cron jobs never enter the delayed queue, so this hook should not fire for them

Actual Behavior

When a cron job fires:

1. enqueue_recurring calls enqueue(config) which calls enqueue_from_config
2. enqueue_from_config calls run_before_delayed_enqueue_hooks unconditionally
3. The hook fires even though the job was never delayed

Root Cause

In scheduler.rb, enqueue_from_config (line ~335) calls before_delayed_enqueue unconditionally:

Reproduction Script

#!/usr/bin/env ruby
#
# Demonstrates bug: before_delayed_enqueue fires for cron jobs that never touched the delayed queue.
#
# Prerequisites: Redis running on localhost:6379
# Run with: ruby <filename.rb>

require "bundler/inline"

gemfile do
  source "https://rubygems.org"
  gem "resque", "~> 2.6"
  gem "resque-scheduler", "~> 4.11"
end

require "resque"
require "resque-scheduler"

redis = Redis.new(db: 15)
Resque.redis = redis
redis.flushdb

class CronTestJob
  @queue = :default
  @hook_call_count = 0

  class << self
    attr_accessor :hook_call_count
  end

  def self.perform(*args)
    puts "Job performed"
  end

  def self.before_delayed_enqueue_track(*args)
    @hook_call_count += 1
    puts "HOOK: before_delayed_enqueue_track called (count: #{@hook_call_count})"
  end
end

puts "=== Simulating a CRON job firing ==="
puts ""

# Simulate what happens when rufus-scheduler triggers a cron job
cron_config = {
  "class" => "CronTestJob",
  "args" => ["cron_arg"],
  "queue" => "default"
}

# This is what enqueue_recurring eventually calls
Resque::Scheduler.enqueue_from_config(cron_config)

puts ""
puts "=== Summary ==="
puts "before_delayed_enqueue hook calls: #{CronTestJob.hook_call_count} (expected: 0)"
puts ""
if CronTestJob.hook_call_count > 0
  puts "BUG CONFIRMED: before_delayed_enqueue should be 0 for cron jobs!"
else
  puts "OK: Hook behavior is correct"
end

redis.flushdb

Output:

=== Simulating a CRON job firing ===

HOOK: before_delayed_enqueue_track called (count: 1)

=== Summary ===
before_delayed_enqueue hook calls: 1 (expected: 0)

BUG CONFIRMED: before_delayed_enqueue should be 0 for cron jobs!

Suggested Fix

This is obviously related to #810. And it also tackles the same question we had there and in #812. I think it is still not easy to answer, whether the before/after_schedule hooks should also fire for recurring jobs, as they never touch the delayed queue.

After also discovering this I actually want to propose to introduce a new hook type: before/after_recurring. With #812 as a base this would not be complicated and we could have dedicated hooks for adding jobs to and moving them from the delayed queue (the existing before/after_schedule) and before/after_recurring just for recurring operations, that are a schedule but no delay.

[top-sigrid]

Hey @PatrickTulskie, tagging you directly as we talked about #810. I would have the time to add the proposed solution to #812, so we could fix both this and #810. Let me know what you think :)