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:
- We'd have to do it everywhere, or monkeypatch que
- 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