Skip to content

Commit

Permalink
Fix setting started_at/finished_at for parents of sharded backgro…
Browse files Browse the repository at this point in the history
…und schema migrations
  • Loading branch information
fatkodima committed May 7, 2024
1 parent ca6d0df commit 89fc190
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 4 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
## master (unreleased)

- Fix setting `started_at`/`finished_at` for parents of sharded background schema migrations
- Improve retrying of failed sharded background migrations
- Fix a bug when retried background data migration can not start
- Do not run multiple background schema migrations on the same table at the same time
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,10 @@ def progress
def retry
if composite? && failed?
children.failed.each(&:retry)
running!
update!(
status: self.class.statuses[:running],
finished_at: nil
)
true
elsif failed?
update!(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,14 @@ def run
def mark_as_running
Migration.transaction do
migration.running!
migration.parent.running! if migration.parent

if (parent = migration.parent)
if parent.started_at
parent.update!(status: :running, finished_at: nil)
else
parent.update!(status: :running, started_at: Time.current, finished_at: nil)
end
end
end
end

Expand Down Expand Up @@ -90,10 +97,10 @@ def complete_parent_if_needed(migration)
parent.with_lock do
children = parent.children.select(:status)
if children.all?(&:succeeded?)
parent.succeeded!
parent.update!(status: :succeeded, finished_at: Time.current)
completed = true
elsif children.any?(&:failed?)
parent.failed!
parent.update!(status: :failed, finished_at: Time.current)
completed = true
end
end
Expand Down
5 changes: 5 additions & 0 deletions test/background_schema_migrations/migration_runner_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,12 @@ def test_run_marks_migration_and_its_parent_as_running

run_migration(child)
assert child.reload.succeeded?
assert child.started_at
assert child.finished_at

assert m.reload.running?
assert m.started_at
assert_nil m.finished_at
end

def test_run_runs_migration
Expand Down
4 changes: 4 additions & 0 deletions test/background_schema_migrations/migration_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,8 @@ def test_retry_composite
m = create_sharded_migration
run_migration(m)
assert m.succeeded?
assert m.started_at
assert m.finished_at

child1, child2, child3 = m.children.to_a
migrations = [m, child1, child2, child3]
Expand All @@ -185,6 +187,8 @@ def test_retry_composite
child1.update_column(:status, "failed")

assert m.retry
assert m.started_at
assert_nil m.finished_at

migrations.each(&:reload)
assert m.running?
Expand Down

0 comments on commit 89fc190

Please sign in to comment.