mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
Reduced memory leak problem in transactions by lazily updating AR objects with new transaction state. If AR object has a callback, the callback will be performed immediately (non-lazily) so the transaction still has to keep records with callbacks.
This commit is contained in:
parent
3a0b6c8e13
commit
67d8bb963d
6 changed files with 89 additions and 6 deletions
|
@ -1,5 +1,27 @@
|
||||||
## Rails 4.0.0 (unreleased) ##
|
## Rails 4.0.0 (unreleased) ##
|
||||||
|
|
||||||
|
* Fixing issue #776.
|
||||||
|
|
||||||
|
Memory bloat in transactions is handled by having the transaction hold only
|
||||||
|
the AR objects which it absolutely needs to know about. These are the AR
|
||||||
|
objects with callbacks (they need to be updated as soon as something in the
|
||||||
|
transaction occurs).
|
||||||
|
|
||||||
|
All other AR objects can be updated lazily by keeping a reference to a
|
||||||
|
TransactionState object. If an AR object gets inside a transaction, then
|
||||||
|
the transaction will add its TransactionState to the AR object. When the
|
||||||
|
user makes a call to some attribute on an AR object (which has no
|
||||||
|
callbacks) associated with a transaction, the AR object will call the
|
||||||
|
sync_with_transaction_state method and make sure it is up to date with the
|
||||||
|
transaction. After it has synced with the transaction state, the AR object
|
||||||
|
will return the attribute that was requested.
|
||||||
|
|
||||||
|
Most of the logic in the changes are used to handle multiple transactions,
|
||||||
|
in which case the AR object has to recursively follow parent pointers of
|
||||||
|
TransactionState objects.
|
||||||
|
|
||||||
|
*John Wang*
|
||||||
|
|
||||||
* Descriptive error message when the necessary AR adapter gem was not found.
|
* Descriptive error message when the necessary AR adapter gem was not found.
|
||||||
Fix #7313
|
Fix #7313
|
||||||
|
|
||||||
|
|
|
@ -8,27 +8,32 @@ module ActiveRecord
|
||||||
# Returns this record's primary key value wrapped in an Array if one is
|
# Returns this record's primary key value wrapped in an Array if one is
|
||||||
# available.
|
# available.
|
||||||
def to_key
|
def to_key
|
||||||
|
sync_with_transaction_state
|
||||||
key = self.id
|
key = self.id
|
||||||
[key] if key
|
[key] if key
|
||||||
end
|
end
|
||||||
|
|
||||||
# Returns the primary key value.
|
# Returns the primary key value.
|
||||||
def id
|
def id
|
||||||
|
sync_with_transaction_state
|
||||||
read_attribute(self.class.primary_key)
|
read_attribute(self.class.primary_key)
|
||||||
end
|
end
|
||||||
|
|
||||||
# Sets the primary key value.
|
# Sets the primary key value.
|
||||||
def id=(value)
|
def id=(value)
|
||||||
|
sync_with_transaction_state
|
||||||
write_attribute(self.class.primary_key, value) if self.class.primary_key
|
write_attribute(self.class.primary_key, value) if self.class.primary_key
|
||||||
end
|
end
|
||||||
|
|
||||||
# Queries the primary key value.
|
# Queries the primary key value.
|
||||||
def id?
|
def id?
|
||||||
|
sync_with_transaction_state
|
||||||
query_attribute(self.class.primary_key)
|
query_attribute(self.class.primary_key)
|
||||||
end
|
end
|
||||||
|
|
||||||
# Returns the primary key value before type cast.
|
# Returns the primary key value before type cast.
|
||||||
def id_before_type_cast
|
def id_before_type_cast
|
||||||
|
sync_with_transaction_state
|
||||||
read_attribute_before_type_cast(self.class.primary_key)
|
read_attribute_before_type_cast(self.class.primary_key)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
|
@ -5,7 +5,7 @@ module ActiveRecord
|
||||||
|
|
||||||
def initialize(connection)
|
def initialize(connection)
|
||||||
@connection = connection
|
@connection = connection
|
||||||
@state = TransactionState.new
|
@state = TransactionState.new
|
||||||
end
|
end
|
||||||
|
|
||||||
def state
|
def state
|
||||||
|
@ -14,11 +14,13 @@ module ActiveRecord
|
||||||
end
|
end
|
||||||
|
|
||||||
class TransactionState
|
class TransactionState
|
||||||
|
attr_accessor :parent
|
||||||
|
|
||||||
VALID_STATES = Set.new([:committed, :rolledback, nil])
|
VALID_STATES = Set.new([:committed, :rolledback, nil])
|
||||||
|
|
||||||
def initialize(state = nil)
|
def initialize(state = nil)
|
||||||
@state = state
|
@state = state
|
||||||
|
@parent = nil
|
||||||
end
|
end
|
||||||
|
|
||||||
def committed?
|
def committed?
|
||||||
|
@ -116,7 +118,11 @@ module ActiveRecord
|
||||||
end
|
end
|
||||||
|
|
||||||
def add_record(record)
|
def add_record(record)
|
||||||
records << record
|
if record.has_transactional_callbacks?
|
||||||
|
records << record
|
||||||
|
else
|
||||||
|
record.set_transaction_state(@state)
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
def rollback_records
|
def rollback_records
|
||||||
|
@ -188,8 +194,9 @@ module ActiveRecord
|
||||||
end
|
end
|
||||||
|
|
||||||
def perform_commit
|
def perform_commit
|
||||||
|
@state.set_state(:committed)
|
||||||
|
@state.parent = parent.state
|
||||||
connection.release_savepoint
|
connection.release_savepoint
|
||||||
records.each { |r| parent.add_record(r) }
|
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -347,8 +347,54 @@ module ActiveRecord
|
||||||
Hash[methods.map { |method| [method, public_send(method)] }].with_indifferent_access
|
Hash[methods.map { |method| [method, public_send(method)] }].with_indifferent_access
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def set_transaction_state(state) # :nodoc:
|
||||||
|
@transaction_state = state
|
||||||
|
end
|
||||||
|
|
||||||
|
def has_transactional_callbacks? # :nodoc:
|
||||||
|
!_rollback_callbacks.empty? || !_commit_callbacks.empty? || !_create_callbacks.empty?
|
||||||
|
end
|
||||||
|
|
||||||
private
|
private
|
||||||
|
|
||||||
|
# Updates the attributes on this particular ActiveRecord object so that
|
||||||
|
# if it is associated with a transaction, then the state of the AR object
|
||||||
|
# will be updated to reflect the current state of the transaction
|
||||||
|
#
|
||||||
|
# The @transaction_state variable stores the states of the associated
|
||||||
|
# transaction. This relies on the fact that a transaction can only be in
|
||||||
|
# one rollback or commit (otherwise a list of states would be required)
|
||||||
|
# Each AR object inside of a transaction carries that transaction's
|
||||||
|
# TransactionState.
|
||||||
|
#
|
||||||
|
# This method checks to see if the ActiveRecord object's state reflects
|
||||||
|
# the TransactionState, and rolls back or commits the ActiveRecord object
|
||||||
|
# as appropriate.
|
||||||
|
#
|
||||||
|
# Since ActiveRecord objects can be inside multiple transactions, this
|
||||||
|
# method recursively goes through the parent of the TransactionState and
|
||||||
|
# checks if the ActiveRecord object reflects the state of the object.
|
||||||
|
def sync_with_transaction_state
|
||||||
|
update_attributes_from_transaction_state(@transaction_state, 0)
|
||||||
|
end
|
||||||
|
|
||||||
|
def update_attributes_from_transaction_state(transaction_state, depth)
|
||||||
|
if transaction_state && !has_transactional_callbacks?
|
||||||
|
unless @reflects_state[depth]
|
||||||
|
if transaction_state.committed?
|
||||||
|
committed!
|
||||||
|
elsif transaction_state.rolledback?
|
||||||
|
rolledback!
|
||||||
|
end
|
||||||
|
@reflects_state[depth] = true
|
||||||
|
end
|
||||||
|
|
||||||
|
if transaction_state.parent && !@reflects_state[depth+1]
|
||||||
|
update_attributes_from_transaction_state(transaction_state.parent, depth+1)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
# Under Ruby 1.9, Array#flatten will call #to_ary (recursively) on each of the elements
|
# Under Ruby 1.9, Array#flatten will call #to_ary (recursively) on each of the elements
|
||||||
# of the array, and then rescues from the possible NoMethodError. If those elements are
|
# of the array, and then rescues from the possible NoMethodError. If those elements are
|
||||||
# ActiveRecord::Base's, then this triggers the various method_missing's that we have,
|
# ActiveRecord::Base's, then this triggers the various method_missing's that we have,
|
||||||
|
@ -376,7 +422,8 @@ module ActiveRecord
|
||||||
@new_record = true
|
@new_record = true
|
||||||
@txn = nil
|
@txn = nil
|
||||||
@_start_transaction_state = {}
|
@_start_transaction_state = {}
|
||||||
@transaction = nil
|
@transaction_state = nil
|
||||||
|
@reflects_state = [false]
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -69,11 +69,13 @@ module ActiveRecord
|
||||||
# Returns true if this object hasn't been saved yet -- that is, a record
|
# Returns true if this object hasn't been saved yet -- that is, a record
|
||||||
# for the object doesn't exist in the data store yet; otherwise, returns false.
|
# for the object doesn't exist in the data store yet; otherwise, returns false.
|
||||||
def new_record?
|
def new_record?
|
||||||
|
sync_with_transaction_state
|
||||||
@new_record
|
@new_record
|
||||||
end
|
end
|
||||||
|
|
||||||
# Returns true if this object has been destroyed, otherwise returns false.
|
# Returns true if this object has been destroyed, otherwise returns false.
|
||||||
def destroyed?
|
def destroyed?
|
||||||
|
sync_with_transaction_state
|
||||||
@destroyed
|
@destroyed
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
|
@ -460,7 +460,7 @@ class TransactionTest < ActiveRecord::TestCase
|
||||||
assert !transaction.state.committed?
|
assert !transaction.state.committed?
|
||||||
|
|
||||||
transaction.perform_rollback
|
transaction.perform_rollback
|
||||||
|
|
||||||
assert transaction.state.rolledback?
|
assert transaction.state.rolledback?
|
||||||
assert !transaction.state.committed?
|
assert !transaction.state.committed?
|
||||||
end
|
end
|
||||||
|
@ -474,7 +474,7 @@ class TransactionTest < ActiveRecord::TestCase
|
||||||
assert !transaction.state.committed?
|
assert !transaction.state.committed?
|
||||||
|
|
||||||
transaction.perform_commit
|
transaction.perform_commit
|
||||||
|
|
||||||
assert !transaction.state.rolledback?
|
assert !transaction.state.rolledback?
|
||||||
assert transaction.state.committed?
|
assert transaction.state.committed?
|
||||||
end
|
end
|
||||||
|
|
Loading…
Reference in a new issue