Skip to content

Commit

Permalink
Stop creating an oban_beats table in migrations
Browse files Browse the repository at this point in the history
  • Loading branch information
sorentwo committed Apr 1, 2021
1 parent 81374c9 commit 35796e6
Show file tree
Hide file tree
Showing 5 changed files with 7 additions and 29 deletions.
1 change: 0 additions & 1 deletion bench/support/setup.exs
@@ -1,6 +1,5 @@
defmodule BenchHelper do
def reset_db do
Oban.Test.Repo.query!("TRUNCATE oban_beats", [], log: false)
Oban.Test.Repo.query!("TRUNCATE oban_jobs", [], log: false)
end

Expand Down
19 changes: 0 additions & 19 deletions lib/oban/migrations/v03.ex
Expand Up @@ -7,30 +7,11 @@ defmodule Oban.Migrations.V03 do
alter table(:oban_jobs, prefix: prefix) do
add_if_not_exists(:attempted_by, {:array, :text})
end

create_if_not_exists table(:oban_beats, primary_key: false, prefix: prefix) do
add :node, :text, null: false
add :queue, :text, null: false
add :nonce, :text, null: false
add :limit, :integer, null: false
add :paused, :boolean, null: false, default: false
add :running, {:array, :integer}, null: false, default: []

add :inserted_at, :utc_datetime_usec,
null: false,
default: fragment("timezone('UTC', now())")

add :started_at, :utc_datetime_usec, null: false
end

create_if_not_exists index(:oban_beats, [:inserted_at], prefix: prefix)
end

def down(prefix) do
alter table(:oban_jobs, prefix: prefix) do
remove_if_exists(:attempted_by, {:array, :text})
end

drop_if_exists table(:oban_beats, prefix: prefix)
end
end
8 changes: 4 additions & 4 deletions lib/oban/migrations/v06.ex
Expand Up @@ -3,11 +3,11 @@ defmodule Oban.Migrations.V06 do

use Ecto.Migration

def up(prefix) do
execute "ALTER TABLE #{prefix}.oban_beats ALTER COLUMN running TYPE bigint[]"
def up(_prefix) do
# This used to modify oban_beats, which aren't included anymore
end

def down(prefix) do
execute "ALTER TABLE #{prefix}.oban_beats ALTER COLUMN running TYPE integer[]"
def down(_prefix) do
# This used to modify oban_beats, which aren't included anymore
end
end
3 changes: 3 additions & 0 deletions mix.exs
Expand Up @@ -16,6 +16,7 @@ defmodule Oban.MixProject do
preferred_cli_env: [
bench: :test,
"test.ci": :test,
"test.reset": :test,
"test.setup": :test
],

Expand Down Expand Up @@ -86,6 +87,7 @@ defmodule Oban.MixProject do
"../oban_pro/guides/plugins/lifeline.md": [title: "Lifeline Plugin"],
"../oban_pro/guides/plugins/dynamic_cron.md": [title: "Dynamic Cron Plugin"],
"../oban_pro/guides/plugins/dynamic_pruner.md": [title: "Dynamic Pruner Plugin"],
"../oban_pro/guides/plugins/relay.md": [title: "Relay Plugin"],
"../oban_pro/guides/plugins/reprioritizer.md": [title: "Reprioritizer Plugin"],
"../oban_pro/guides/workers/batch.md": [title: "Batch Worker"],
"../oban_pro/guides/workers/workflow.md": [title: "Workflow Worker"],
Expand Down Expand Up @@ -170,6 +172,7 @@ defmodule Oban.MixProject do
defp aliases do
[
bench: "run bench/bench_helper.exs",
"test.reset": ["ecto.drop", "test.setup"],
"test.setup": ["ecto.create", "ecto.migrate"],
"test.ci": [
"format --check-formatted",
Expand Down
5 changes: 0 additions & 5 deletions test/oban/migrations_test.exs
Expand Up @@ -48,21 +48,18 @@ defmodule Oban.MigrationsTest do
end

assert table_exists?("oban_jobs")
assert table_exists?("oban_beats")
assert migrated_version() == current_version()

Application.put_env(:oban, :down_version, 2)
assert :ok = Ecto.Migrator.down(Repo, @base_version + 2, StepMigration)

assert table_exists?("oban_jobs")
refute table_exists?("oban_beats")
assert migrated_version() == 1

Application.put_env(:oban, :down_version, 1)
assert :ok = Ecto.Migrator.down(Repo, @base_version + 1, StepMigration)

refute table_exists?("oban_jobs")
refute table_exists?("oban_beats")
after
clear_migrated()
end
Expand All @@ -71,7 +68,6 @@ defmodule Oban.MigrationsTest do
assert :ok = Ecto.Migrator.up(Repo, @base_version, DefaultMigration)

assert table_exists?("oban_jobs")
assert table_exists?("oban_beats")
assert migrated_version() == current_version()

# Migrating once more to replicate multiple migrations that don't specify a version.
Expand All @@ -80,7 +76,6 @@ defmodule Oban.MigrationsTest do
assert :ok = Ecto.Migrator.down(Repo, @base_version + 1, DefaultMigration)

refute table_exists?("oban_jobs")
refute table_exists?("oban_beats")

# Migrating once more to replicate multiple migrations that don't specify a version.
assert :ok = Ecto.Migrator.down(Repo, @base_version, DefaultMigration)
Expand Down

5 comments on commit 35796e6

@esse
Copy link

@esse esse commented on 35796e6 Sep 24, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've just encountered this commit - I think it breaks one of the principles in the README :(

Migrations between versions are idempotent and will never change after a release.

@sorentwo
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair point. In a way, it does violate one of the stated principles. This was a tough spot:

  1. For the majority of users, the oban_beats table was useless.
  2. A migration that dropped the table would require extra coordination to prevent breaking production systems.

Migrations for the oban_jobs table didn't change and I felt this was an acceptable compromise 🤷‍♂️

@esse
Copy link

@esse esse commented on 35796e6 Sep 24, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add point to the README with that specific case then?

@sorentwo
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done e9208cb

@esse
Copy link

@esse esse commented on 35796e6 Sep 24, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you!

Please sign in to comment.