From 00ddc10f2ee8b89706e8626f26498189ef2d9cd4 Mon Sep 17 00:00:00 2001 From: Parker Selbert Date: Fri, 12 Jul 2024 09:43:16 -0500 Subject: [PATCH] Remove forced unique from cron plugin Using uniqueness by default prevents being able to use the cron plugin with databases that don't support uniqueness (e.g. yugabyte, because of advisory locks). Luckily, uniqueness hasn't been necessary for safe cron insertion since leadership was introduced and scheduling changed to top-of-the-minute _many_ versions ago. --- README.md | 45 +++++++++++++-------------------- lib/oban/plugins/cron.ex | 11 +------- test/oban/plugins/cron_test.exs | 25 ------------------ 3 files changed, 19 insertions(+), 62 deletions(-) diff --git a/README.md b/README.md index 9f343f36..026f881c 100644 --- a/README.md +++ b/README.md @@ -593,9 +593,8 @@ config :my_app, Oban, ## Periodic Jobs -Oban's `Cron` plugin registers workers a cron-like schedule and enqueues jobs -automatically. Periodic jobs are declared as a list of `{cron, worker}` or -`{cron, worker, options}` tuples: +Oban's `Cron` plugin registers workers a cron-like schedule and enqueues jobs automatically. +Periodic jobs are declared as a list of `{cron, worker}` or `{cron, worker, options}` tuples: ```elixir config :my_app, Oban, @@ -620,18 +619,17 @@ The crontab would insert jobs as follows: * `MyApp.MondayWorker` — Inserted at noon every Monday in the "scheduled" queue * `MyApp.AnotherDailyWorker` — Inserted at midnight every day with no retries -The crontab format respects all [standard rules][cron] and has one minute -resolution. Jobs are considered unique for most of each minute, which prevents -duplicate jobs with multiple nodes and across node restarts. +The crontab format respects all [standard rules][cron] and has one minute resolution. Jobs are +considered unique for most of each minute, which prevents duplicate jobs with multiple nodes and +across node restarts. -Like other jobs, recurring jobs will use the `:queue` specified by the worker -module (or `:default` if one is not specified). +Like other jobs, recurring jobs will use the `:queue` specified by the worker module (or +`:default` if one is not specified). ### Cron Expressions -Standard Cron expressions are composed of rules specifying the minutes, hours, -days, months and weekdays. Rules for each field are comprised of literal values, -wildcards, step values or ranges: +Standard Cron expressions are composed of rules specifying the minutes, hours, days, months and +weekdays. Rules for each field are comprised of literal values, wildcards, step values or ranges: * `*` — Wildcard, matches any value (0, 1, 2, ...) * `0` — Literal, matches only itself (only 0) @@ -639,8 +637,8 @@ wildcards, step values or ranges: * `0-5` — Range, matches any value within the range (0, 1, 2, 3, 4, 5) * `0-9/2` - Step values can be used in conjunction with ranges (0, 2, 4, 6, 8) -Each part may have multiple rules, where rules are separated by a comma. The -allowed values for each field are as follows: +Each part may have multiple rules, where rules are separated by a comma. The allowed values for +each field are as follows: * `minute` — 0-59 * `hour` — 0-23 @@ -664,25 +662,18 @@ Some specific examples that demonstrate the full range of expressions: * `0 0 * DEC *` — Once a day at midnight during December * `0 7-9,4-6 13 * FRI` — Once an hour during both rush hours on Friday the 13th -For more in depth information see the man documentation for `cron` and `crontab` -in your system. Alternatively you can experiment with various expressions -online at [Crontab Guru][guru]. +For more in depth information see the man documentation for `cron` and `crontab` in your system. +Alternatively you can experiment with various expressions online at [Crontab Guru][guru]. #### Caveats & Guidelines -* All schedules are evaluated as UTC unless a different timezone is provided. - See `Oban.Plugins.Cron` for information about configuring a timezone. +* All schedules are evaluated as UTC unless a different timezone is provided. See + `Oban.Plugins.Cron` for information about configuring a timezone. -* Workers can be used for regular _and_ scheduled jobs so long as they accept - different arguments. +* Workers can be used for regular _and_ scheduled jobs so long as they accept different arguments. -* Duplicate jobs are prevented through transactional locks and unique - constraints. Workers that are used for regular and scheduled jobs _must not_ - specify `unique` options less than `60s`. - -* Long running jobs may execute simultaneously if the scheduling interval is - shorter than it takes to execute the job. You can prevent overlap by passing - custom `unique` opts in the crontab config: +* Long running jobs may execute simultaneously if the scheduling interval is shorter than it takes + to execute the job. You can prevent overlap by passing custom `unique` opts in the crontab config: ```elixir custom_args = %{scheduled: true} diff --git a/lib/oban/plugins/cron.ex b/lib/oban/plugins/cron.ex index 28046471..6fe6fe7a 100644 --- a/lib/oban/plugins/cron.ex +++ b/lib/oban/plugins/cron.ex @@ -281,18 +281,9 @@ defmodule Oban.Plugins.Cron do opts = worker.__opts__() - |> unique_opts(opts) + |> Worker.merge_opts(opts) |> Keyword.update(:meta, meta, &Map.merge(&1, meta)) worker.new(args, opts) end - - # Make each job unique for 59 seconds to prevent double-enqueue if the node or scheduler - # crashes. The minimum resolution for our cron jobs is 1 minute, so there is potentially - # a one second window where a double enqueue can happen. - defp unique_opts(worker_opts, crontab_opts) do - [unique: [period: 59]] - |> Worker.merge_opts(worker_opts) - |> Worker.merge_opts(crontab_opts) - end end diff --git a/test/oban/plugins/cron_test.exs b/test/oban/plugins/cron_test.exs index fd9bb67b..62aa1d50 100644 --- a/test/oban/plugins/cron_test.exs +++ b/test/oban/plugins/cron_test.exs @@ -37,7 +37,6 @@ defmodule Oban.Plugins.CronTest do test ":crontab worker options are validated" do refute_valid("expected valid job options", crontab: [{"* * * * *", Worker, priority: -1}]) - refute_valid("expected valid job options", crontab: [{"* * * * *", Worker, unique: []}]) end test ":timezone is validated as a known timezone" do @@ -99,30 +98,6 @@ defmodule Oban.Plugins.CronTest do assert [1, 3] == inserted_refs() end - test "cron jobs are not enqueued twice within the same minute" do - run_with_opts(crontab: [{"* * * * *", Worker, args: worker_args(1)}]) - - assert [1] == inserted_refs() - - run_with_opts(crontab: [{"* * * * *", Worker, args: worker_args(1)}]) - - assert [1] == inserted_refs() - end - - test "cron jobs are only enqueued once between nodes" do - opts = [crontab: [{"* * * * *", Worker, args: worker_args(1)}]] - - name1 = start_supervised_oban!(plugins: [{Cron, opts}]) - name2 = start_supervised_oban!(plugins: [{Cron, opts}]) - name3 = start_supervised_oban!(plugins: [{Cron, opts}]) - - [name1, name2, name3] - |> Task.async_stream(&send_evaluate/1) - |> Stream.run() - - assert [1] == inserted_refs() - end - test "cron jobs are scheduled using the configured timezone" do {:ok, %DateTime{hour: chi_hour}} = DateTime.now("America/Chicago") {:ok, %DateTime{hour: utc_hour}} = DateTime.now("Etc/UTC")