Skip to content

Commit

Permalink
Fix raising in development when using sharding and background index c…
Browse files Browse the repository at this point in the history
…reation/removal was not enqueued
  • Loading branch information
fatkodima committed Apr 27, 2024
1 parent b3e4a12 commit 8b77302
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 4 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
## master (unreleased)

- Fix raising in development when using sharding and background index creation/removal was not enqueued

## 0.17.0 (2024-04-23)

- Fix background migrations `#progress` possibility to fail with zero division error
Expand Down
2 changes: 1 addition & 1 deletion lib/online_migrations/utils.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ def say(message)
end

def raise_or_say(message)
if developer_env?
if developer_env? && !multiple_databases?
raise message
else
say(message)
Expand Down
2 changes: 1 addition & 1 deletion test/background_schema_migrations/migration_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ def setup

def teardown
@connection.drop_table(:users, if_exists: true)
on_each_shard { Dog.connection.remove_index(:dogs, :name) }
OnlineMigrations::BackgroundSchemaMigrations::Migration.delete_all
# on_each_shard { Dog.delete_all }
end

def test_table_name_length_validation
Expand Down
1 change: 1 addition & 0 deletions test/background_schema_migrations/scheduler_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ module BackgroundSchemaMigrations
class SchedulerTest < Minitest::Test
def teardown
OnlineMigrations::BackgroundSchemaMigrations::Migration.delete_all
on_each_shard { Dog.connection.remove_index(:dogs, :name) }
end

def test_run
Expand Down
28 changes: 26 additions & 2 deletions test/schema_statements/indexes_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -104,12 +104,34 @@ def test_add_index_in_background
def test_add_index_in_background_when_index_already_exists
@connection.add_index(:users, :name, unique: true)
assert_raises_with_message(RuntimeError, /Index creation was not enqueued/i) do
@connection.add_index_in_background(:users, :name, unique: true, connection_class_name: "User")
OnlineMigrations::Utils.stub(:multiple_databases?, false) do
@connection.add_index_in_background(:users, :name, unique: true, connection_class_name: "User")
end
end
assert @connection.index_exists?(:users, :name)
assert_equal 0, OnlineMigrations::BackgroundSchemaMigrations::Migration.count
end

def test_add_index_in_background_when_using_multiple_databases
# Enulate migration run on a primary shard. Now index exists on both tables.
on_shard(:shard_one) do
connection = Dog.connection
assert_not connection.index_exists?(:dogs, :name)
connection.add_index_in_background(:dogs, :name, unique: true, connection_class_name: "ShardRecord")
assert connection.index_exists?(:dogs, :name)
end

on_shard(:shard_two) do
# Emulate migration running on the second shard.
connection = Dog.connection
assert connection.index_exists?(:dogs, :name)
connection.add_index_in_background(:dogs, :name, unique: true, connection_class_name: "ShardRecord")
assert connection.index_exists?(:dogs, :name)
end
ensure
on_each_shard { Dog.connection.remove_index(:dogs, :name) }
end

def test_add_index_in_background_custom_attributes
m = @connection.add_index_in_background(:users, :name, name: "my_name", max_attempts: 5, statement_timeout: 10, connection_class_name: "User")
assert_equal "my_name", m.name
Expand All @@ -132,7 +154,9 @@ def test_remove_index_in_background_raises_without_name

def test_remove_index_in_background_when_does_not_exist
assert_raises_with_message(RuntimeError, /Index deletion was not enqueued/i) do
@connection.remove_index_in_background(:users, :name, name: "index_users_on_name", connection_class_name: "User")
OnlineMigrations::Utils.stub(:multiple_databases?, false) do
@connection.remove_index_in_background(:users, :name, name: "index_users_on_name", connection_class_name: "User")
end
end

assert_not @connection.index_exists?(:users, :name)
Expand Down

0 comments on commit 8b77302

Please sign in to comment.