diff options
author | Rafael Mendonça França <rafaelmfranca@gmail.com> | 2013-02-20 08:20:26 -0800 |
---|---|---|
committer | Rafael Mendonça França <rafaelmfranca@gmail.com> | 2013-02-20 08:20:26 -0800 |
commit | 82a54324f615ae999c811b67701fd269b54d9fe4 (patch) | |
tree | 77784730efb77689419082290a476405d2bb5561 /activerecord | |
parent | b85f63bbd00243787387d9c32901131851e8e424 (diff) | |
parent | 67d8bb963d5d51fc644d6b1ca20164efb4cee6d7 (diff) | |
download | rails-82a54324f615ae999c811b67701fd269b54d9fe4.tar.gz rails-82a54324f615ae999c811b67701fd269b54d9fe4.tar.bz2 rails-82a54324f615ae999c811b67701fd269b54d9fe4.zip |
Merge pull request #9068 from wangjohn/transaction_callback_patch
Reduced memory leak problem in transactions by lazily updating AR objects
Diffstat (limited to 'activerecord')
-rw-r--r-- | activerecord/CHANGELOG.md | 22 | ||||
-rw-r--r-- | activerecord/lib/active_record/attribute_methods/primary_key.rb | 5 | ||||
-rw-r--r-- | activerecord/lib/active_record/connection_adapters/abstract/transaction.rb | 13 | ||||
-rw-r--r-- | activerecord/lib/active_record/core.rb | 49 | ||||
-rw-r--r-- | activerecord/lib/active_record/persistence.rb | 2 | ||||
-rw-r--r-- | activerecord/test/cases/transactions_test.rb | 4 |
6 files changed, 89 insertions, 6 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index a888b79391..e864e90e6f 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,5 +1,27 @@ ## 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. Fix #7313 diff --git a/activerecord/lib/active_record/attribute_methods/primary_key.rb b/activerecord/lib/active_record/attribute_methods/primary_key.rb index 310f1b6e75..3e454b713a 100644 --- a/activerecord/lib/active_record/attribute_methods/primary_key.rb +++ b/activerecord/lib/active_record/attribute_methods/primary_key.rb @@ -8,27 +8,32 @@ module ActiveRecord # Returns this record's primary key value wrapped in an Array if one is # available. def to_key + sync_with_transaction_state key = self.id [key] if key end # Returns the primary key value. def id + sync_with_transaction_state read_attribute(self.class.primary_key) end # Sets the primary key value. def id=(value) + sync_with_transaction_state write_attribute(self.class.primary_key, value) if self.class.primary_key end # Queries the primary key value. def id? + sync_with_transaction_state query_attribute(self.class.primary_key) end # Returns the primary key value before type cast. def id_before_type_cast + sync_with_transaction_state read_attribute_before_type_cast(self.class.primary_key) end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb b/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb index 3ecef96b10..73c80a3220 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb @@ -5,7 +5,7 @@ module ActiveRecord def initialize(connection) @connection = connection - @state = TransactionState.new + @state = TransactionState.new end def state @@ -14,11 +14,13 @@ module ActiveRecord end class TransactionState + attr_accessor :parent VALID_STATES = Set.new([:committed, :rolledback, nil]) def initialize(state = nil) @state = state + @parent = nil end def committed? @@ -116,7 +118,11 @@ module ActiveRecord end def add_record(record) - records << record + if record.has_transactional_callbacks? + records << record + else + record.set_transaction_state(@state) + end end def rollback_records @@ -188,8 +194,9 @@ module ActiveRecord end def perform_commit + @state.set_state(:committed) + @state.parent = parent.state connection.release_savepoint - records.each { |r| parent.add_record(r) } end end end diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index 63a1197a56..6510433056 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -347,8 +347,54 @@ module ActiveRecord Hash[methods.map { |method| [method, public_send(method)] }].with_indifferent_access 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 + # 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 # 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, @@ -376,7 +422,8 @@ module ActiveRecord @new_record = true @txn = nil @_start_transaction_state = {} - @transaction = nil + @transaction_state = nil + @reflects_state = [false] end end end diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index 803cae7115..066f93635a 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -69,11 +69,13 @@ module ActiveRecord # 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. def new_record? + sync_with_transaction_state @new_record end # Returns true if this object has been destroyed, otherwise returns false. def destroyed? + sync_with_transaction_state @destroyed end diff --git a/activerecord/test/cases/transactions_test.rb b/activerecord/test/cases/transactions_test.rb index 546737b398..6d66342fa5 100644 --- a/activerecord/test/cases/transactions_test.rb +++ b/activerecord/test/cases/transactions_test.rb @@ -460,7 +460,7 @@ class TransactionTest < ActiveRecord::TestCase assert !transaction.state.committed? transaction.perform_rollback - + assert transaction.state.rolledback? assert !transaction.state.committed? end @@ -474,7 +474,7 @@ class TransactionTest < ActiveRecord::TestCase assert !transaction.state.committed? transaction.perform_commit - + assert !transaction.state.rolledback? assert transaction.state.committed? end |