Omit BEGIN/COMMIT statements for empty transactions
If a transaction is opened and closed without any queries being run, we can safely omit the `BEGIN` and `COMMIT` statements, as they only exist to modify the connection's behaviour inside the transaction. This removes the overhead of those statements when saving a record with no changes, which makes workarounds like `save if changed?` unnecessary. This implementation buffers transactions inside the transaction manager and materializes them the next time the connection is used. For this to work, the adapter needs to guard all connection use with a call to `materialize_transactions`. Because of this, adapters must opt in to get this new behaviour by implementing `supports_lazy_transactions?`. If `raw_connection` is used to get a reference to the underlying database connection, the behaviour is disabled and transactions are opened eagerly, as we can't know how the connection will be used. However when the connection is checked back into the pool, we can assume that the application won't use the reference again and reenable lazy transactions. This prevents a single `raw_connection` call from disabling lazy transactions for the lifetime of the connection.
This commit is contained in:
parent
f2970a08b5
commit
0ac81ee6ff
@ -259,7 +259,9 @@ def transaction(requires_new: nil, isolation: nil, joinable: true)
|
||||
|
||||
attr_reader :transaction_manager #:nodoc:
|
||||
|
||||
delegate :within_new_transaction, :open_transactions, :current_transaction, :begin_transaction, :commit_transaction, :rollback_transaction, to: :transaction_manager
|
||||
delegate :within_new_transaction, :open_transactions, :current_transaction, :begin_transaction,
|
||||
:commit_transaction, :rollback_transaction, :materialize_transactions,
|
||||
:disable_lazy_transactions!, :enable_lazy_transactions!, to: :transaction_manager
|
||||
|
||||
def transaction_open?
|
||||
current_transaction.open?
|
||||
|
@ -91,12 +91,14 @@ def add_record(record); end
|
||||
end
|
||||
|
||||
class Transaction #:nodoc:
|
||||
attr_reader :connection, :state, :records, :savepoint_name
|
||||
attr_reader :connection, :state, :records, :savepoint_name, :isolation_level
|
||||
|
||||
def initialize(connection, options, run_commit_callbacks: false)
|
||||
@connection = connection
|
||||
@state = TransactionState.new
|
||||
@records = []
|
||||
@isolation_level = options[:isolation]
|
||||
@materialized = false
|
||||
@joinable = options.fetch(:joinable, true)
|
||||
@run_commit_callbacks = run_commit_callbacks
|
||||
end
|
||||
@ -105,6 +107,14 @@ def add_record(record)
|
||||
records << record
|
||||
end
|
||||
|
||||
def materialize!
|
||||
@materialized = true
|
||||
end
|
||||
|
||||
def materialized?
|
||||
@materialized
|
||||
end
|
||||
|
||||
def rollback_records
|
||||
ite = records.uniq
|
||||
while record = ite.shift
|
||||
@ -141,24 +151,30 @@ def open?; !closed?; end
|
||||
end
|
||||
|
||||
class SavepointTransaction < Transaction
|
||||
def initialize(connection, savepoint_name, parent_transaction, options, *args)
|
||||
super(connection, options, *args)
|
||||
def initialize(connection, savepoint_name, parent_transaction, *args)
|
||||
super(connection, *args)
|
||||
|
||||
parent_transaction.state.add_child(@state)
|
||||
|
||||
if options[:isolation]
|
||||
if isolation_level
|
||||
raise ActiveRecord::TransactionIsolationError, "cannot set transaction isolation in a nested transaction"
|
||||
end
|
||||
connection.create_savepoint(@savepoint_name = savepoint_name)
|
||||
|
||||
@savepoint_name = savepoint_name
|
||||
end
|
||||
|
||||
def materialize!
|
||||
connection.create_savepoint(savepoint_name)
|
||||
super
|
||||
end
|
||||
|
||||
def rollback
|
||||
connection.rollback_to_savepoint(savepoint_name)
|
||||
connection.rollback_to_savepoint(savepoint_name) if materialized?
|
||||
@state.rollback!
|
||||
end
|
||||
|
||||
def commit
|
||||
connection.release_savepoint(savepoint_name)
|
||||
connection.release_savepoint(savepoint_name) if materialized?
|
||||
@state.commit!
|
||||
end
|
||||
|
||||
@ -166,22 +182,23 @@ def full_rollback?; false; end
|
||||
end
|
||||
|
||||
class RealTransaction < Transaction
|
||||
def initialize(connection, options, *args)
|
||||
super
|
||||
if options[:isolation]
|
||||
connection.begin_isolated_db_transaction(options[:isolation])
|
||||
def materialize!
|
||||
if isolation_level
|
||||
connection.begin_isolated_db_transaction(isolation_level)
|
||||
else
|
||||
connection.begin_db_transaction
|
||||
end
|
||||
|
||||
super
|
||||
end
|
||||
|
||||
def rollback
|
||||
connection.rollback_db_transaction
|
||||
connection.rollback_db_transaction if materialized?
|
||||
@state.full_rollback!
|
||||
end
|
||||
|
||||
def commit
|
||||
connection.commit_db_transaction
|
||||
connection.commit_db_transaction if materialized?
|
||||
@state.full_commit!
|
||||
end
|
||||
end
|
||||
@ -190,6 +207,9 @@ class TransactionManager #:nodoc:
|
||||
def initialize(connection)
|
||||
@stack = []
|
||||
@connection = connection
|
||||
@has_unmaterialized_transactions = false
|
||||
@materializing_transactions = false
|
||||
@lazy_transactions_enabled = true
|
||||
end
|
||||
|
||||
def begin_transaction(options = {})
|
||||
@ -203,11 +223,41 @@ def begin_transaction(options = {})
|
||||
run_commit_callbacks: run_commit_callbacks)
|
||||
end
|
||||
|
||||
transaction.materialize! unless @connection.supports_lazy_transactions? && lazy_transactions_enabled?
|
||||
@stack.push(transaction)
|
||||
@has_unmaterialized_transactions = true if @connection.supports_lazy_transactions?
|
||||
transaction
|
||||
end
|
||||
end
|
||||
|
||||
def disable_lazy_transactions!
|
||||
materialize_transactions
|
||||
@lazy_transactions_enabled = false
|
||||
end
|
||||
|
||||
def enable_lazy_transactions!
|
||||
@lazy_transactions_enabled = true
|
||||
end
|
||||
|
||||
def lazy_transactions_enabled?
|
||||
@lazy_transactions_enabled
|
||||
end
|
||||
|
||||
def materialize_transactions
|
||||
return if @materializing_transactions
|
||||
return unless @has_unmaterialized_transactions
|
||||
|
||||
@connection.lock.synchronize do
|
||||
begin
|
||||
@materializing_transactions = true
|
||||
@stack.each { |t| t.materialize! unless t.materialized? }
|
||||
ensure
|
||||
@materializing_transactions = false
|
||||
end
|
||||
@has_unmaterialized_transactions = false
|
||||
end
|
||||
end
|
||||
|
||||
def commit_transaction
|
||||
@connection.lock.synchronize do
|
||||
transaction = @stack.last
|
||||
|
@ -80,6 +80,8 @@ class AbstractAdapter
|
||||
attr_reader :schema_cache, :owner, :logger, :prepared_statements, :lock
|
||||
alias :in_use? :owner
|
||||
|
||||
set_callback :checkin, :after, :enable_lazy_transactions!
|
||||
|
||||
def self.type_cast_config_to_integer(config)
|
||||
if config.is_a?(Integer)
|
||||
config
|
||||
@ -338,6 +340,10 @@ def supports_foreign_tables?
|
||||
false
|
||||
end
|
||||
|
||||
def supports_lazy_transactions?
|
||||
false
|
||||
end
|
||||
|
||||
# This is meant to be implemented by the adapters that support extensions
|
||||
def disable_extension(name)
|
||||
end
|
||||
@ -449,6 +455,7 @@ def verify!
|
||||
# This is useful for when you need to call a proprietary method such as
|
||||
# PostgreSQL's lo_* methods.
|
||||
def raw_connection
|
||||
disable_lazy_transactions!
|
||||
@connection
|
||||
end
|
||||
|
||||
|
@ -180,6 +180,8 @@ def explain(arel, binds = [])
|
||||
|
||||
# Executes the SQL statement in the context of this connection.
|
||||
def execute(sql, name = nil)
|
||||
materialize_transactions
|
||||
|
||||
log(sql, name) do
|
||||
ActiveSupport::Dependencies.interlock.permit_concurrent_loads do
|
||||
@connection.query(sql)
|
||||
|
@ -29,6 +29,8 @@ def execute(sql, name = nil)
|
||||
end
|
||||
|
||||
def exec_query(sql, name = "SQL", binds = [], prepare: false)
|
||||
materialize_transactions
|
||||
|
||||
if without_prepared_statement?(binds)
|
||||
execute_and_free(sql, name) do |result|
|
||||
ActiveRecord::Result.new(result.fields, result.to_a) if result
|
||||
@ -41,6 +43,8 @@ def exec_query(sql, name = "SQL", binds = [], prepare: false)
|
||||
end
|
||||
|
||||
def exec_delete(sql, name = nil, binds = [])
|
||||
materialize_transactions
|
||||
|
||||
if without_prepared_statement?(binds)
|
||||
execute_and_free(sql, name) { @connection.affected_rows }
|
||||
else
|
||||
|
@ -58,6 +58,10 @@ def supports_savepoints?
|
||||
true
|
||||
end
|
||||
|
||||
def supports_lazy_transactions?
|
||||
true
|
||||
end
|
||||
|
||||
# HELPER METHODS ===========================================
|
||||
|
||||
def each_hash(result) # :nodoc:
|
||||
|
@ -58,6 +58,8 @@ def result_as_array(res) #:nodoc:
|
||||
|
||||
# Queries the database and returns the results in an Array-like object
|
||||
def query(sql, name = nil) #:nodoc:
|
||||
materialize_transactions
|
||||
|
||||
log(sql, name) do
|
||||
ActiveSupport::Dependencies.interlock.permit_concurrent_loads do
|
||||
result_as_array @connection.async_exec(sql)
|
||||
@ -70,6 +72,8 @@ def query(sql, name = nil) #:nodoc:
|
||||
# Note: the PG::Result object is manually memory managed; if you don't
|
||||
# need it specifically, you may want consider the <tt>exec_query</tt> wrapper.
|
||||
def execute(sql, name = nil)
|
||||
materialize_transactions
|
||||
|
||||
log(sql, name) do
|
||||
ActiveSupport::Dependencies.interlock.permit_concurrent_loads do
|
||||
@connection.async_exec(sql)
|
||||
|
@ -326,6 +326,10 @@ def supports_pgcrypto_uuid?
|
||||
postgresql_version >= 90400
|
||||
end
|
||||
|
||||
def supports_lazy_transactions?
|
||||
true
|
||||
end
|
||||
|
||||
def get_advisory_lock(lock_id) # :nodoc:
|
||||
unless lock_id.is_a?(Integer) && lock_id.bit_length <= 63
|
||||
raise(ArgumentError, "PostgreSQL requires advisory lock ids to be a signed 64 bit integer")
|
||||
@ -597,6 +601,8 @@ def execute_and_clear(sql, name, binds, prepare: false)
|
||||
end
|
||||
|
||||
def exec_no_cache(sql, name, binds)
|
||||
materialize_transactions
|
||||
|
||||
type_casted_binds = type_casted_binds(binds)
|
||||
log(sql, name, binds, type_casted_binds) do
|
||||
ActiveSupport::Dependencies.interlock.permit_concurrent_loads do
|
||||
@ -606,6 +612,8 @@ def exec_no_cache(sql, name, binds)
|
||||
end
|
||||
|
||||
def exec_cache(sql, name, binds)
|
||||
materialize_transactions
|
||||
|
||||
stmt_key = prepare_statement(sql)
|
||||
type_casted_binds = type_casted_binds(binds)
|
||||
|
||||
|
@ -186,6 +186,10 @@ def supports_explain?
|
||||
true
|
||||
end
|
||||
|
||||
def supports_lazy_transactions?
|
||||
true
|
||||
end
|
||||
|
||||
# REFERENTIAL INTEGRITY ====================================
|
||||
|
||||
def disable_referential_integrity # :nodoc:
|
||||
@ -212,6 +216,8 @@ def explain(arel, binds = [])
|
||||
end
|
||||
|
||||
def exec_query(sql, name = nil, binds = [], prepare: false)
|
||||
materialize_transactions
|
||||
|
||||
type_casted_binds = type_casted_binds(binds)
|
||||
|
||||
log(sql, name, binds, type_casted_binds) do
|
||||
@ -252,6 +258,8 @@ def last_inserted_id(result)
|
||||
end
|
||||
|
||||
def execute(sql, name = nil) #:nodoc:
|
||||
materialize_transactions
|
||||
|
||||
log(sql, name) do
|
||||
ActiveSupport::Dependencies.interlock.permit_concurrent_loads do
|
||||
@connection.execute(sql)
|
||||
|
@ -10,6 +10,7 @@ module ActiveRecord
|
||||
class AdapterTest < ActiveRecord::TestCase
|
||||
def setup
|
||||
@connection = ActiveRecord::Base.connection
|
||||
@connection.materialize_transactions
|
||||
end
|
||||
|
||||
##
|
||||
|
@ -170,6 +170,8 @@ def test_mysql_set_session_variable_to_default
|
||||
end
|
||||
|
||||
def test_logs_name_show_variable
|
||||
ActiveRecord::Base.connection.materialize_transactions
|
||||
@subscriber.logged.clear
|
||||
@connection.show_variable "foo"
|
||||
assert_equal "SCHEMA", @subscriber.logged[0][1]
|
||||
end
|
||||
|
@ -4,6 +4,8 @@
|
||||
|
||||
class PostgresqlActiveSchemaTest < ActiveRecord::PostgreSQLTestCase
|
||||
def setup
|
||||
ActiveRecord::Base.connection.materialize_transactions
|
||||
|
||||
ActiveRecord::ConnectionAdapters::PostgreSQLAdapter.class_eval do
|
||||
def execute(sql, name = nil) sql end
|
||||
end
|
||||
|
@ -15,8 +15,9 @@ class NonExistentTable < ActiveRecord::Base
|
||||
def setup
|
||||
super
|
||||
@subscriber = SQLSubscriber.new
|
||||
@subscription = ActiveSupport::Notifications.subscribe("sql.active_record", @subscriber)
|
||||
@connection = ActiveRecord::Base.connection
|
||||
@connection.materialize_transactions
|
||||
@subscription = ActiveSupport::Notifications.subscribe("sql.active_record", @subscriber)
|
||||
end
|
||||
|
||||
def teardown
|
||||
|
@ -44,6 +44,7 @@ def debug(progname = nil, &block)
|
||||
def setup
|
||||
@old_logger = ActiveRecord::Base.logger
|
||||
Developer.primary_key
|
||||
ActiveRecord::Base.connection.materialize_transactions
|
||||
super
|
||||
ActiveRecord::LogSubscriber.attach_to(:active_record)
|
||||
end
|
||||
|
@ -31,6 +31,7 @@ def teardown
|
||||
end
|
||||
|
||||
def capture_sql
|
||||
ActiveRecord::Base.connection.materialize_transactions
|
||||
SQLCounter.clear_log
|
||||
yield
|
||||
SQLCounter.log_all.dup
|
||||
@ -48,6 +49,7 @@ def assert_sql(*patterns_to_match)
|
||||
|
||||
def assert_queries(num = 1, options = {})
|
||||
ignore_none = options.fetch(:ignore_none) { num == :any }
|
||||
ActiveRecord::Base.connection.materialize_transactions
|
||||
SQLCounter.clear_log
|
||||
x = yield
|
||||
the_log = ignore_none ? SQLCounter.log_all : SQLCounter.log
|
||||
|
@ -11,7 +11,7 @@ class Tag < ActiveRecord::Base
|
||||
|
||||
test "setting the isolation level raises an error" do
|
||||
assert_raises(ActiveRecord::TransactionIsolationError) do
|
||||
Tag.transaction(isolation: :serializable) {}
|
||||
Tag.transaction(isolation: :serializable) { Topic.connection.materialize_transactions }
|
||||
end
|
||||
end
|
||||
end
|
||||
|
@ -575,7 +575,7 @@ def test_rollback_when_commit_raises
|
||||
assert_called(Topic.connection, :rollback_db_transaction) do
|
||||
e = assert_raise RuntimeError do
|
||||
Topic.transaction do
|
||||
# do nothing
|
||||
Topic.connection.materialize_transactions
|
||||
end
|
||||
end
|
||||
assert_equal "OH NOES", e.message
|
||||
@ -943,6 +943,76 @@ def test_transaction_rollback_with_primarykeyless_tables
|
||||
connection.drop_table "transaction_without_primary_keys", if_exists: true
|
||||
end
|
||||
|
||||
def test_empty_transaction_is_not_materialized
|
||||
assert_no_queries do
|
||||
Topic.transaction {}
|
||||
end
|
||||
end
|
||||
|
||||
def test_unprepared_statement_materializes_transaction
|
||||
assert_sql(/BEGIN/i, /COMMIT/i) do
|
||||
Topic.transaction { Topic.where("1=1").first }
|
||||
end
|
||||
end
|
||||
|
||||
if ActiveRecord::Base.connection.prepared_statements
|
||||
def test_prepared_statement_materializes_transaction
|
||||
Topic.first
|
||||
|
||||
assert_sql(/BEGIN/i, /COMMIT/i) do
|
||||
Topic.transaction { Topic.first }
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
def test_savepoint_does_not_materialize_transaction
|
||||
assert_no_queries do
|
||||
Topic.transaction do
|
||||
Topic.transaction(requires_new: true) {}
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
def test_raising_does_not_materialize_transaction
|
||||
assert_raise(RuntimeError) do
|
||||
assert_no_queries do
|
||||
Topic.transaction { raise }
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
def test_accessing_raw_connection_materializes_transaction
|
||||
assert_sql(/BEGIN/i, /COMMIT/i) do
|
||||
Topic.transaction { Topic.connection.raw_connection }
|
||||
end
|
||||
end
|
||||
|
||||
def test_accessing_raw_connection_disables_lazy_transactions
|
||||
Topic.connection.raw_connection
|
||||
|
||||
assert_sql(/BEGIN/i, /COMMIT/i) do
|
||||
Topic.transaction {}
|
||||
end
|
||||
end
|
||||
|
||||
def test_checking_in_connection_reenables_lazy_transactions
|
||||
connection = Topic.connection_pool.checkout
|
||||
connection.raw_connection
|
||||
Topic.connection_pool.checkin connection
|
||||
|
||||
assert_no_queries do
|
||||
connection.transaction {}
|
||||
end
|
||||
end
|
||||
|
||||
def test_transactions_can_be_manually_materialized
|
||||
assert_sql(/BEGIN/i, /COMMIT/i) do
|
||||
Topic.transaction do
|
||||
Topic.connection.materialize_transactions
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
%w(validation save destroy).each do |filter|
|
||||
|
Loading…
Reference in New Issue
Block a user