build_for_enqueue discards scheduled_at values for bulk-enqueued jobs

[julik]

When adopting the new bulk enqueue feature, we noticed that scheduled_at was getting removed from jobs. This would lead to very high spikes in load, because all the wait values we would specify would just end up getting discarded.

When I examine the code, I see that the issue is in the build_for_enqueue method. It takes the attributes of the active job verbatim, but for some reason the scheduled_at at the ActiveJob level is a Float, not a Time. This scheduled_at value (still a Float) then gets passed to Execution.new and it gets discarded - because the datatype is not correct.

For the moment I seem to be able to circumvent this with the following patch:

module OverrideBuildForEnqueue
  def build_for_enqueue(active_job, overrides = {})
    active_job.scheduled_at = Time.at(active_job.scheduled_at) if active_job.scheduled_at.is_a?(Numeric)
    super(active_job, overrides)
  end
end

ActiveSupport::Reloader.to_prepare do
  class << GoodJob::Execution
    prepend OverrideBuildForEnqueue
  end
end

but it is not entirely clear to me where the conversion from the Float (which seems to be ActiveJob-native) to the Time should take place. it is evident the conversion does need to happen somewhere though

[bensheldon]

I think I just fixed this (#960), but maybe I missed something: https://github.com/bensheldon/good_job/releases/tag/v3.15.9

[julik]

Yes definitely a duplicate of #959 thanks!

[bensheldon]

Fhew! 💖