Skip to content

Commit cc2bcd9

Browse files
committed
Defer tick_total calculation to the background data migration starting time instead of enqueueing time
1 parent b112627 commit cc2bcd9

5 files changed

Lines changed: 39 additions & 6 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
## master (unreleased)
22

33
- Drop support for Ruby < 3.3
4+
- Defer `tick_total` calculation to the background data migration starting time instead of enqueueing time
45
- Make "replacing an index" check independent of the live database state
56

67
## 0.34.0 (2026-05-29)

lib/online_migrations/background_data_migrations/migration.rb

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,13 @@ def stuck?
123123
# @private
124124
def start
125125
if enqueued?
126-
update!(status: :running, started_at: Time.current)
126+
# Defer `tick_total` calculation to the migration starting time
127+
# instead of the enqueueing time to avoid failed deploys.
128+
self.tick_total ||= safely_calculate_tick_total
129+
self.status = :running
130+
self.started_at = Time.current
131+
save!
132+
127133
data_migration.after_start
128134
true
129135
else
@@ -309,11 +315,18 @@ def retry
309315
def set_defaults
310316
config = ::OnlineMigrations.config.background_data_migrations
311317
self.max_attempts ||= config.max_attempts
312-
self.tick_total ||= on_shard_if_present do
318+
self.iteration_pause ||= config.iteration_pause
319+
end
320+
321+
def safely_calculate_tick_total
322+
on_shard_if_present do
313323
data_migration.count
314324
end
315-
316-
self.iteration_pause ||= config.iteration_pause
325+
rescue ActiveRecord::QueryCanceled
326+
# `tick_total` is not required and is used only for progress tracking.
327+
# Probably the `count` method was implemented in a non-efficient way.
328+
# Better to not track progress than have a failing migration.
329+
nil
317330
end
318331

319332
def instrument_status_change

test/background_data_migrations/data_migrations.rb

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,4 +192,10 @@ def count
192192
collection.count
193193
end
194194
end
195+
196+
class HeavyCountMigration < WithCountMigration
197+
def count
198+
ActiveRecord::Base.connection.execute("select pg_sleep(100)")
199+
end
200+
end
195201
end

test/background_data_migrations/migration_job_test.rb

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,16 @@ def test_uses_count_method_if_present
106106
assert_equal 3, m.reload.tick_total
107107
end
108108

109+
def test_gracefully_handles_heavy_count_method
110+
@connection.execute("set statement_timeout to '0.01s'")
111+
m = create_migration("HeavyCountMigration")
112+
MigrationJob.perform_inline(m.id)
113+
114+
assert_nil m.tick_total
115+
ensure
116+
@connection.execute("set statement_timeout to '10s'")
117+
end
118+
109119
def test_stores_metadada_about_the_data_migration
110120
m = create_migration("ArrayCollectionMigration")
111121
MigrationJob.perform_inline(m.id)

test/background_data_migrations/migration_test.rb

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ def test_sets_defaults
3737
config.stub(:iteration_pause, 10) do
3838
m = create_migration
3939
assert m.pending?
40+
assert_nil m.tick_total
4041
assert_equal config.max_attempts, m.max_attempts
4142
assert_equal 10, m.iteration_pause
4243
end
@@ -55,7 +56,8 @@ def test_progress_succeeded_migration
5556

5657
def test_progress_not_finished_migration
5758
2.times { User.create! }
58-
m = create_migration(migration_name: "MigrationWithCount")
59+
# tick_total is calculated on migration run time, so need to set explicitly.
60+
m = create_migration(migration_name: "MigrationWithCount", tick_total: 2)
5961

6062
m.update!(tick_count: 1)
6163
assert_in_delta 50.0, m.progress
@@ -66,7 +68,8 @@ def test_progress_not_finished_migration
6668

6769
def test_progress_running_migration_without_records
6870
assert_equal 0, EmptyCollection.new.count
69-
m = create_migration(migration_name: "EmptyCollection")
71+
# tick_total is calculated on migration run time, so need to set explicitly.
72+
m = create_migration(migration_name: "EmptyCollection", tick_total: 0)
7073
assert_in_delta 0.0, m.progress
7174
end
7275

0 commit comments

Comments
 (0)