que-rb/que

Proposal to change run_at default to clock_timestamp

hlascelles opened this issue · 2 comments

Problem

Jobs enqueued in transactions can appear delayed as the default run_at is now(), which is a delayed time in a long transaction.

Example

We know that the que_jobs table has this column: run_at timestamp with time zone DEFAULT now() NOT NULL,

We have some jobs that perform work, and enqueue another job. eg:

class CalculateBilling < Que::Job
  def run(ref)
    ActiveRecord::Base.transaction do
      calculate_stuff(ref) # Takes 30 seconds
      SendBillingSms.enqueue(ref)
    end
  end
end

We then also have an alarm to check our jobs are running in a timely fashion by comparing when the job started with its run_at.

In the scenario above, this means the SendBillingSms job is enqueued with run_at as now(). But, now() does not mean "the time on the wall" in postgres. It means "the time the transaction started".

See: https://newcubator.com/blog/web/postgresql-timestamp-methods

Both now() and clock_timestamp() are PostgreSQL functions. The main difference between them is that now() adds the time at the start of the transactions, while clock_timestamp() inserts the time at the execution of the statement.

This means all of our SendBillingSms jobs are instantaneously "30 seconds delayed", even if they are run the split second that they are visible to workers.

Solution

We could/should change the column to have a default of clock_timestamp().

ie: run_at timestamp with time zone DEFAULT clock_timestamp() NOT NULL,

This means that the intent of what the run_at of a job "to run now" should be is more accurate (probably, ie principle of least surprise). It also means jobs can be monitored for "delayed start" more easily and accurately.

Note, we do not want to use Ruby time in a run_at as it may be out, even by a second or two. We also would not like to pass a run_at in since:

  1. We'd have to do it everywhere, or monkeypatch que
  2. We'd have to do a DB lookup for what that time is, which is an extra call

Would you accept a PR for this? Thanks!

That all seems well reasoned and perfectly reasonable to me. A PR would be welcome

PR opened #410