Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Move while_preventing_writes from conn to handler #36469

Merged
merged 1 commit into from Jun 14, 2019
Merged

Move while_preventing_writes from conn to handler #36469

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 15 additions & 0 deletions activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
View file
Open in desktop
Expand Up @@ -999,15 +999,30 @@ def self.discard_unowned_pools(pid_map) # :nodoc:
end
end

attr_reader :prevent_writes

def initialize
# These caches are keyed by spec.name (ConnectionSpecification#name).
@owner_to_pool = ConnectionHandler.create_owner_to_pool
@prevent_writes = false

# Backup finalizer: if the forked child never needed a pool, the above
# early discard has not occurred
ObjectSpace.define_finalizer self, ConnectionHandler.unowned_pool_finalizer(@owner_to_pool)
end

# Prevent writing to the database regardless of role.
#
# In some cases you may want to prevent writes to the database
# even if you are on a database that can write. `while_preventing_writes`
# will prevent writes to the database for the duration of the block.
def while_preventing_writes
original, @prevent_writes = @prevent_writes, true
yield
ensure
@prevent_writes = original
end

def connection_pool_list
owner_to_pool.values.compact
end
Expand Down
17 changes: 2 additions & 15 deletions activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
View file
Open in desktop
Expand Up @@ -78,7 +78,7 @@ class AbstractAdapter
SIMPLE_INT = /\A\d+\z/

attr_accessor :pool
attr_reader :visitor, :owner, :logger, :lock, :prepared_statements, :prevent_writes
attr_reader :visitor, :owner, :logger, :lock, :prepared_statements
alias :in_use? :owner

set_callback :checkin, :after, :enable_lazy_transactions!
Expand Down Expand Up @@ -117,7 +117,6 @@ def initialize(connection, logger = nil, config = {}) # :nodoc:
@pool = ActiveRecord::ConnectionAdapters::NullPool.new
@idle_since = Concurrent.monotonic_time
@quoted_column_names, @quoted_table_names = {}, {}
@prevent_writes = false
@visitor = arel_visitor
@statements = build_statement_pool
@lock = ActiveSupport::Concurrency::LoadInterlockAwareMonitor.new
Expand All @@ -143,19 +142,7 @@ def replica?
# Returns true if the connection is a replica, or if +prevent_writes+
# is set to true.
def preventing_writes?
replica? || prevent_writes
end

# Prevent writing to the database regardless of role.
#
# In some cases you may want to prevent writes to the database
# even if you are on a database that can write. `while_preventing_writes`
# will prevent writes to the database for the duration of the block.
def while_preventing_writes
original, @prevent_writes = @prevent_writes, true
yield
ensure
@prevent_writes = original
replica? || ActiveRecord::Base.connection_handler.prevent_writes
Copy link
Member

Choose a reason for hiding this comment

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

I never remember this but, does each model have its different connection handler or all models always share the ActiveRecord::Base connection handler?

Copy link
Member Author

Choose a reason for hiding this comment

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

Each type of connection has it's own handler depending on what you put in connects_to.

Given the following:

class ApplicationRecord < ActiveRecord::Base
  self.abstract_class = true

  connects_to database: { writing: :primary, reading: :primary_replica }
end

class Person < ApplicationRecord
end

class AnimalsBase < ApplicationRecord
  self.abstract_class = true

  connects_to database: { writing: :animals, reading: :animals_replica }
end

class Dog < AnimalsBase
end

This will create 2 handlers a :writing handler and a :reading handler (aka role). Each of these handlers has 2 connections: one for application record (person) and one for animals base (dog). Because we use connected_to to swap the handler out:

ActiveRecord::Base.connected_to(role: :reading) do
  # do something on reading handler
end

means that ActiveRecord::Base.connection_handler.prevent_writes will ask the handler whether it should prevent writes instead of the connection. Since connected_to operates on the handler we can be confident it's passing the value down correctly.

Copy link
Member Author

Choose a reason for hiding this comment

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

I added a test that demonstrates this is working correctly here https://github.com/rails/rails/pull/36469/files#diff-3ad790de5c6c5fbd6d956e443bcc0490R1579

end

def migrations_paths # :nodoc:
Expand Down
4 changes: 2 additions & 2 deletions activerecord/lib/active_record/middleware/database_selector/resolver.rb
View file
Open in desktop
Expand Up @@ -45,8 +45,8 @@ def write(&blk)

private
def read_from_primary(&blk)
ActiveRecord::Base.connection.while_preventing_writes do
ActiveRecord::Base.connected_to(role: ActiveRecord::Base.writing_role) do
ActiveRecord::Base.connected_to(role: ActiveRecord::Base.writing_role) do
ActiveRecord::Base.connection_handler.while_preventing_writes do
instrumenter.instrument("database_selector.active_record.read_from_primary") do
yield
end
Expand Down
11 changes: 6 additions & 5 deletions activerecord/test/cases/adapter_test.rb
View file
Open in desktop
Expand Up @@ -12,6 +12,7 @@ class AdapterTest < ActiveRecord::TestCase
def setup
@connection = ActiveRecord::Base.connection
@connection.materialize_transactions
@connection_handler = ActiveRecord::Base.connection_handler
end

##
Expand Down Expand Up @@ -166,7 +167,7 @@ class << @connection
def test_preventing_writes_predicate
assert_not_predicate @connection, :preventing_writes?

@connection.while_preventing_writes do
@connection_handler.while_preventing_writes do
assert_predicate @connection, :preventing_writes?
end

Expand All @@ -176,7 +177,7 @@ def test_preventing_writes_predicate
def test_errors_when_an_insert_query_is_called_while_preventing_writes
assert_no_queries do
assert_raises(ActiveRecord::ReadOnlyError) do
@connection.while_preventing_writes do
@connection_handler.while_preventing_writes do
@connection.transaction do
@connection.insert("INSERT INTO subscribers(nick) VALUES ('138853948594')", nil, false)
end
Expand All @@ -190,7 +191,7 @@ def test_errors_when_an_update_query_is_called_while_preventing_writes

assert_no_queries do
assert_raises(ActiveRecord::ReadOnlyError) do
@connection.while_preventing_writes do
@connection_handler.while_preventing_writes do
@connection.transaction do
@connection.update("UPDATE subscribers SET nick = '9989' WHERE nick = '138853948594'")
end
Expand All @@ -204,7 +205,7 @@ def test_errors_when_a_delete_query_is_called_while_preventing_writes

assert_no_queries do
assert_raises(ActiveRecord::ReadOnlyError) do
@connection.while_preventing_writes do
@connection_handler.while_preventing_writes do
@connection.transaction do
@connection.delete("DELETE FROM subscribers WHERE nick = '138853948594'")
end
Expand All @@ -216,7 +217,7 @@ def test_errors_when_a_delete_query_is_called_while_preventing_writes
def test_doesnt_error_when_a_select_query_is_called_while_preventing_writes
@connection.insert("INSERT INTO subscribers(nick) VALUES ('138853948594')")

@connection.while_preventing_writes do
@connection_handler.while_preventing_writes do
result = @connection.select_all("SELECT subscribers.* FROM subscribers WHERE nick = '138853948594'")
assert_equal 1, result.length
end
Expand Down
17 changes: 9 additions & 8 deletions activerecord/test/cases/adapters/mysql2/mysql2_adapter_test.rb
View file
Open in desktop
Expand Up @@ -8,6 +8,7 @@ class Mysql2AdapterTest < ActiveRecord::Mysql2TestCase

def setup
@conn = ActiveRecord::Base.connection
@connection_handler = ActiveRecord::Base.connection_handler
end

def test_exec_query_nothing_raises_with_no_result_queries
Expand Down Expand Up @@ -148,7 +149,7 @@ def test_errors_for_bigint_fks_on_string_pk_table_in_create_table

def test_errors_when_an_insert_query_is_called_while_preventing_writes
assert_raises(ActiveRecord::ReadOnlyError) do
@conn.while_preventing_writes do
@connection_handler.while_preventing_writes do
@conn.insert("INSERT INTO `engines` (`car_id`) VALUES ('138853948594')")
end
end
Expand All @@ -158,7 +159,7 @@ def test_errors_when_an_update_query_is_called_while_preventing_writes
@conn.insert("INSERT INTO `engines` (`car_id`) VALUES ('138853948594')")

assert_raises(ActiveRecord::ReadOnlyError) do
@conn.while_preventing_writes do
@connection_handler.while_preventing_writes do
@conn.update("UPDATE `engines` SET `engines`.`car_id` = '9989' WHERE `engines`.`car_id` = '138853948594'")
end
end
Expand All @@ -168,7 +169,7 @@ def test_errors_when_a_delete_query_is_called_while_preventing_writes
@conn.execute("INSERT INTO `engines` (`car_id`) VALUES ('138853948594')")

assert_raises(ActiveRecord::ReadOnlyError) do
@conn.while_preventing_writes do
@connection_handler.while_preventing_writes do
@conn.execute("DELETE FROM `engines` where `engines`.`car_id` = '138853948594'")
end
end
Expand All @@ -178,7 +179,7 @@ def test_errors_when_a_replace_query_is_called_while_preventing_writes
@conn.execute("INSERT INTO `engines` (`car_id`) VALUES ('138853948594')")

assert_raises(ActiveRecord::ReadOnlyError) do
@conn.while_preventing_writes do
@connection_handler.while_preventing_writes do
@conn.execute("REPLACE INTO `engines` SET `engines`.`car_id` = '249823948'")
end
end
Expand All @@ -187,27 +188,27 @@ def test_errors_when_a_replace_query_is_called_while_preventing_writes
def test_doesnt_error_when_a_select_query_is_called_while_preventing_writes
@conn.execute("INSERT INTO `engines` (`car_id`) VALUES ('138853948594')")

@conn.while_preventing_writes do
@connection_handler.while_preventing_writes do
assert_equal 1, @conn.execute("SELECT `engines`.* FROM `engines` WHERE `engines`.`car_id` = '138853948594'").entries.count
end
end

def test_doesnt_error_when_a_show_query_is_called_while_preventing_writes
@conn.while_preventing_writes do
@connection_handler.while_preventing_writes do
assert_equal 2, @conn.execute("SHOW FULL FIELDS FROM `engines`").entries.count
end
end

def test_doesnt_error_when_a_set_query_is_called_while_preventing_writes
@conn.while_preventing_writes do
@connection_handler.while_preventing_writes do
assert_nil @conn.execute("SET NAMES utf8mb4 COLLATE utf8mb4_unicode_ci")
end
end

def test_doesnt_error_when_a_read_query_with_leading_chars_is_called_while_preventing_writes
@conn.execute("INSERT INTO `engines` (`car_id`) VALUES ('138853948594')")

@conn.while_preventing_writes do
@connection_handler.while_preventing_writes do
assert_equal 1, @conn.execute("(\n( SELECT `engines`.* FROM `engines` WHERE `engines`.`car_id` = '138853948594' ) )").entries.count
end
end
Expand Down
15 changes: 8 additions & 7 deletions activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb
View file
Open in desktop
Expand Up @@ -13,6 +13,7 @@ class PostgreSQLAdapterTest < ActiveRecord::PostgreSQLTestCase

def setup
@connection = ActiveRecord::Base.connection
@connection_handler = ActiveRecord::Base.connection_handler
end

def test_bad_connection
Expand Down Expand Up @@ -379,7 +380,7 @@ def test_unparsed_defaults_are_at_least_set_when_saving
def test_errors_when_an_insert_query_is_called_while_preventing_writes
with_example_table do
assert_raises(ActiveRecord::ReadOnlyError) do
@connection.while_preventing_writes do
@connection_handler.while_preventing_writes do
@connection.execute("INSERT INTO ex (data) VALUES ('138853948594')")
end
end
Expand All @@ -391,7 +392,7 @@ def test_errors_when_an_update_query_is_called_while_preventing_writes
@connection.execute("INSERT INTO ex (data) VALUES ('138853948594')")

assert_raises(ActiveRecord::ReadOnlyError) do
@connection.while_preventing_writes do
@connection_handler.while_preventing_writes do
@connection.execute("UPDATE ex SET data = '9989' WHERE data = '138853948594'")
end
end
Expand All @@ -403,7 +404,7 @@ def test_errors_when_a_delete_query_is_called_while_preventing_writes
@connection.execute("INSERT INTO ex (data) VALUES ('138853948594')")

assert_raises(ActiveRecord::ReadOnlyError) do
@connection.while_preventing_writes do
@connection_handler.while_preventing_writes do
@connection.execute("DELETE FROM ex where data = '138853948594'")
end
end
Expand All @@ -414,20 +415,20 @@ def test_doesnt_error_when_a_select_query_is_called_while_preventing_writes
with_example_table do
@connection.execute("INSERT INTO ex (data) VALUES ('138853948594')")

@connection.while_preventing_writes do
@connection_handler.while_preventing_writes do
assert_equal 1, @connection.execute("SELECT * FROM ex WHERE data = '138853948594'").entries.count
end
end
end

def test_doesnt_error_when_a_show_query_is_called_while_preventing_writes
@connection.while_preventing_writes do
@connection_handler.while_preventing_writes do
assert_equal 1, @connection.execute("SHOW TIME ZONE").entries.count
end
end

def test_doesnt_error_when_a_set_query_is_called_while_preventing_writes
@connection.while_preventing_writes do
@connection_handler.while_preventing_writes do
assert_equal [], @connection.execute("SET standard_conforming_strings = on").entries
end
end
Expand All @@ -436,7 +437,7 @@ def test_doesnt_error_when_a_read_query_with_leading_chars_is_called_while_preve
with_example_table do
@connection.execute("INSERT INTO ex (data) VALUES ('138853948594')")

@connection.while_preventing_writes do
@connection_handler.while_preventing_writes do
assert_equal 1, @connection.execute("(\n( SELECT * FROM ex WHERE data = '138853948594' ) )").entries.count
end
end
Expand Down
14 changes: 8 additions & 6 deletions activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb
View file
Open in desktop
Expand Up @@ -19,6 +19,8 @@ def setup
@conn = Base.sqlite3_connection database: ":memory:",
adapter: "sqlite3",
timeout: 100

@connection_handler = ActiveRecord::Base.connection_handler
end

def test_bad_connection
Expand Down Expand Up @@ -572,7 +574,7 @@ def test_writes_are_not_permitted_to_readonly_databases
def test_errors_when_an_insert_query_is_called_while_preventing_writes
with_example_table "id int, data string" do
assert_raises(ActiveRecord::ReadOnlyError) do
@conn.while_preventing_writes do
@connection_handler.while_preventing_writes do
@conn.execute("INSERT INTO ex (data) VALUES ('138853948594')")
end
end
Expand All @@ -584,7 +586,7 @@ def test_errors_when_an_update_query_is_called_while_preventing_writes
@conn.execute("INSERT INTO ex (data) VALUES ('138853948594')")

assert_raises(ActiveRecord::ReadOnlyError) do
@conn.while_preventing_writes do
@connection_handler.while_preventing_writes do
@conn.execute("UPDATE ex SET data = '9989' WHERE data = '138853948594'")
end
end
Expand All @@ -596,7 +598,7 @@ def test_errors_when_a_delete_query_is_called_while_preventing_writes
@conn.execute("INSERT INTO ex (data) VALUES ('138853948594')")

assert_raises(ActiveRecord::ReadOnlyError) do
@conn.while_preventing_writes do
@connection_handler.while_preventing_writes do
@conn.execute("DELETE FROM ex where data = '138853948594'")
end
end
Expand All @@ -608,7 +610,7 @@ def test_errors_when_a_replace_query_is_called_while_preventing_writes
@conn.execute("INSERT INTO ex (data) VALUES ('138853948594')")

assert_raises(ActiveRecord::ReadOnlyError) do
@conn.while_preventing_writes do
@connection_handler.while_preventing_writes do
@conn.execute("REPLACE INTO ex (data) VALUES ('249823948')")
end
end
Expand All @@ -619,7 +621,7 @@ def test_doesnt_error_when_a_select_query_is_called_while_preventing_writes
with_example_table "id int, data string" do
@conn.execute("INSERT INTO ex (data) VALUES ('138853948594')")

@conn.while_preventing_writes do
@connection_handler.while_preventing_writes do
assert_equal 1, @conn.execute("SELECT data from ex WHERE data = '138853948594'").count
end
end
Expand All @@ -629,7 +631,7 @@ def test_doesnt_error_when_a_read_query_with_leading_chars_is_called_while_preve
with_example_table "id int, data string" do
@conn.execute("INSERT INTO ex (data) VALUES ('138853948594')")

@conn.while_preventing_writes do
@connection_handler.while_preventing_writes do
assert_equal 1, @conn.execute(" SELECT data from ex WHERE data = '138853948594'").count
end
end
Expand Down