#2991 After Transaction Patch - Ruby on Rails

Type	To find
responsible:me	tickets assigned to you
tagged:"@high"	tickets tagged @high
milestone:next	tickets in the upcoming milestone
state:invalid	tickets with the state invalid
created:"last week"	tickets created last week
sort:number, importance, updated	tickets sorted by #, importance or updated
Combine keywords for powerful searching.
Use advanced searching »

This project is archived and is in readonly mode.

#2991 ✓committed

After Transaction Patch

Reported by Brian Durand | August 4th, 2009 @ 03:36 AM | in 3.0.2

This patch provides hooks for callbacks after a transaction rolls back or commits. It is intended to address some inconsistencies with the current Rails code as well as offer hooks for developers to execute code at the appropriate time.

Bug with creating records in transaction block

The current transaction code will roll back a newly created record to the proper state with a nil id and new_record? returning true, but only when an error is encountered while the record is being saved. If you have a multi statement transaction, records will not be rolled back properly. Take this case:

class Model < ActiveRecord::Base
  def after_save
    raise "illegal value" if title == "xxx"
  end
end

This code works fine:

model = Model.new(:title => "xxx")
assert model.new_record?
model.save
assert model.new_record?

However, wrap it in a transaction block, and it fails:

Model.transaction do
  model = Model.new(:title => "valid")
  assert model.new_record?
  model.save
  raise ActiveRecord::Rollback
end
assert model.new_record? # Fail!

This patch solves the issue by keeping track of all records saved or destroyed in a transaction and rolling back the id and new_record? for newly created records when transactions are rolled back.

Callbacks after_commit, after_rollback

In addition, the patch adds callbacks for after_commit and after_rollback which are called after a commit or rollback is sent to the database. The intention is to allow developers to hook in code to communicate with systems external to the database.

For example, a common construct with caching is to add a clear cache call to an after_save callback. However, since after_save callbacks happen within a transaction, the cache will be cleared before the changes is committed to the database. This leaves a small window where another process can regenerate the cache entry:

Process 1 -> begin transaction
Process 1 -> save record
Process 1 -> clear cache
Process 2 -> read cache
Process 2 -> regenerate cache from database
Process 1 -> commit transaction

In this case, Process 2 will generate what will be a stale cache entry out of sync with the database. This bug is most likely to occur in a transaction block or if there are additional after_save callbacks on the model.

As another example, some models may store data external to the database (i.e. on the file system) that needs to be cleaned up if the record is not successfully saved. Once again, this can fail in a transaction block where the record is rolled back after any after save has occurred.

after_transaction.diff 21.3 KB

Comments and changes to this ticket

Michael Koziarski August 5th, 2009 @ 07:01 AM
- Assigned user set to “Jeremy Kemper”
I certainly like the features of after_commit and after_rollback. The implementation is pretty intrusive (necessarily).

However another part of me thinks that once the transaction's rolled back it's much easier to just redirect / reload and start over.

Assigning to jeremy as he was the last one to poke around in the txn area.
You flagged this item as spam.
Brian Durand September 14th, 2009 @ 03:57 PM
- Tag changed from “2.3.4, active_record, transaction” to “3.0, active_record, transaction”
Redirecting / reloading doesn't fix the problem and will lose any validation errors that should be displayed to the user. The bigger issue, though, is that it can lead to inconsistent or corrupt data.
You flagged this item as spam.
nicholas a. evans December 16th, 2009 @ 04:02 PM
There is also the after_commit gem (http://gemcutter.org/gems/after_commit). Perhaps the authors of that gem could work together with Brian (the author of this patch) and Jeremy Kemper, to get it cleaned up, reviewed, and pulled into rails core. As Brian states above, it is not possible to safely write cache invalidation or indexing code without hooks that run outside of the transaction.
You flagged this item as spam.
Brian Durand January 5th, 2010 @ 03:44 PM
I didn't include a before_commit or before_rollback in the patch just because of the nature of the issue.

When a transaction is ready to be committed, it should really be committed. A before_commit callback gives a chance for some code to raise an exception that will stop the commit from happening.

A before_rollback doesn't quite have the same problem, but it does have the complication that you can't do anything that needs to be persisted to the database (unless you have a separate connection open) since it will be immediately rolled back. I didn't think it was worth the added complication since both of these callbacks can be covered by the existing after_save callback.
You flagged this item as spam.
Brian Durand January 19th, 2010 @ 05:05 PM
I looked through the after_commit gem and couldn't use anything because of the number of core code changes in Rails 3.0. Specifically, this patch works with the save point support added to transactions in ActiveRecord::ConnectionAdapters::DatabaseStatements.

I have added a new patch file which supersedes the previous one and incorporates the latest method of adding and invoking callbacks. I also added the after_commit_on* and after_rollback_on* callbacks from the after_commit gem (where * = create, update, or delete). As I previously stated, I don't think the before_commit or before_rollback callbacks are safe and did not add them. Also fixed is restoring the ActiveRecord::Base#destroyed? when a transaction rolls back similar to how new_record? is restored.

I think it is important to get this patch into Rails 3.0 since the effects it addresses (corrupt data, and inconsistent errors that can't be replicated and turn in to a time suck) are severe and fixing the issue in a plugin or gem will be fragile. It's really not possible to patch the code in a gem without replicating the code ActiveRecord::ConnectionAdapters::DatabaseStatements#transaction because of the inclusion of save points requires several spots to hook in the callbacks.
- after_transaction_patch_2.diff 26.8 KB
Jeremy Kemper January 19th, 2010 @ 06:39 PM
- State changed from “new” to “open”
- Milestone cleared.
You flagged this item as spam.
Gaspard Bucher February 9th, 2010 @ 05:52 PM
In case someone wants a lightweight after_commit implementation on top of rails 2.x, here is a simple plugin that does the job. You don't need to patch rails, just require 'after_commit'.

This enables the following code, anywhere in a model:
```
after_commit do
  # write file, create cache, etc
end
```
- after_commit.tgz 1.4 KB
You flagged this item as spam.
Gaspard Bucher February 9th, 2010 @ 06:14 PM
Updated the after_commit plugin with proper init code: http://zenadmin.org/en/blog/post647.html
You flagged this item as spam.
Brian Durand April 29th, 2010 @ 04:55 PM
Any more thoughts on this patch? I still think that the hooks necessary for after_rollback functionality require this to be in the core code instead of as a plugin. The new support for rollback points in ActiveRecord 3 makes it quite a bit more sophisticated than in Rails 2.
Jeremy Kemper April 29th, 2010 @ 05:06 PM
+1 on the idea and implementation. I frequently need this and use a small monkeypatch plugin to do it. The only nit is some code style that doesn't match Rails, like the space in def foo (bar).
You flagged this item as spam.
Brian Durand April 29th, 2010 @ 06:50 PM
I fixed the offending code styles and rebased the patch with the latest master and fixed some Ruby warnings showing up in the tests.
- after_transaction_patch_3.diff 27 KB
Repository April 29th, 2010 @ 08:24 PM
- State changed from “open” to “committed”
(from [da840d13da865331297d5287391231b1ed39721b]) Add after_commit and after_rollback callbacks to ActiveRecord that are called after transactions either commit or rollback on all records saved or destroyed in the transaction.

[#2991 state:committed]

Signed-off-by: Jeremy Kemper jeremy@bitsweat.net
http://github.com/rails/rails/commit/da840d13da865331297d5287391231...
Jeremy Kemper April 29th, 2010 @ 08:36 PM
One more thing: after_commit_on_create is old-style API. Could you bump these up to after_commit :on => :create/:update/:destroy ?
You flagged this item as spam.
Gaspard Bucher April 29th, 2010 @ 09:06 PM
I understand the interest of the declarative after_commit code in the model as a class method as it has been the way to write callbacks in Rails for a long time but I think we could consider faster and more Ruby-like code with blocks triggered in the model because:
1. these retain the context and avoid filling the model with instance variable that you need to clear afterwards
2. code blocks are inserted in the transaction, not in the model's instance variables
3. if your code path does not need an after commit, the overhead is zero
The reason we start to ask for :on => :destroy, :on => :create is a good indicator that the callback solution is not the best approach to "after_commit". Examples below:

I. Usage example with block triggered after_commit:
```
before_destroy :clear_cache

def clear_cache
  # Since the object still exists, we can find the related cached
  # files.
  cached_pages = CachedPage.all(...)
  after_commit do
    cached_pages.each do |p|
      p.destroy!
    end
  end
end
```
II. Example with after_commit callback:
```
before_destroy :get_cached_pages
after_commit :clear_cached_pages, :on => :destroy

def get_cached_pages
  # Since the object still exists, we can find the related cached
  # files.
  @cached_pages = CachedPage.all(...)
end

def clear_cached_pages
  return if @cached_pages.blank?
  @cached_pages.each do |p|
    p.destroy!
  end
  @cached_pages = nil
end
```

Jeremy Kemper April 29th, 2010 @ 09:14 PM

Gaspard, that's similar to the monkeypatch I use.

I notice that most of my usage is in another callback that just declares an after_transaction block. I do like the instance-level API too, though.

module AfterTransaction
  def self.included(base)
    base.extend(ClassMethods)

    class << base
      alias_method_chain :transaction, :callbacks
    end
  end

  module ClassMethods
    @@after_transaction_hooks = []

    def transaction_with_callbacks(&block)
      clean = true
      transaction_without_callbacks(&block)
    rescue Exception
      clean = false
      raise
    ensure
      if connection.open_transactions.zero?
        after_transaction_callbacks if clean
        clear_transaction_callbacks!
      end
    end

    def after_transaction(&block)
      if connection.open_transactions.zero?
        yield
      else
        @@after_transaction_hooks << block
      end
    end

    private

      def after_transaction_callbacks
        @@after_transaction_hooks.each { |hook| hook.call }
      end

      def clear_transaction_callbacks!
        @@after_transaction_hooks.clear
      end
  end

  def after_transaction(&block)
    self.class.after_transaction(&block)
  end
end

You flagged this item as spam.
Brian Durand April 29th, 2010 @ 09:16 PM
Here's a patch to update the API to the new style in the comments and the tests.
- update_api_calls.diff 3 KB
You flagged this item as spam.
Brian Durand April 29th, 2010 @ 09:19 PM
I do like the features available with a block. I have written my fair share of truly ugly after_save monstrosities. The instance API would be great for all callbacks to support in addition to the class API.
Repository April 30th, 2010 @ 02:26 AM
(from [d2a49e4b1f30c5997e169110eed94a55aee53f56]) Update after_commit and after_rollback docs and tests to use new style API with an :on options instead of on_* suffix.

[#2991]

Signed-off-by: Jeremy Kemper jeremy@bitsweat.net
http://github.com/rails/rails/commit/d2a49e4b1f30c5997e169110eed94a...
Jeremy Kemper June 8th, 2010 @ 06:54 PM
- Milestone cleared.
- State changed from “committed” to “open”
Brian, could you take a look at the failing tests on master? Turns out this was broken but was hidden by some assert typos.
DHH June 8th, 2010 @ 07:24 PM
- Milestone cleared.
Repository June 8th, 2010 @ 07:56 PM
(from [1b2941cba1165b0721f57524645fe378bee2a950]) Temporarily revert "Update after_commit and after_rollback docs and tests to use new style API with an :on options instead of on_* suffix." and "Add after_commit and after_rollback callbacks to ActiveRecord that are called after transactions either commit or rollback on all records saved or destroyed in the transaction."

This reverts commits d2a49e4b1f30c5997e169110eed94a55aee53f56 and da840d13da865331297d5287391231b1ed39721b.

[#2991]

Conflicts:
```
activerecord/CHANGELOG
activerecord/lib/active_record/transactions.rb
activerecord/test/cases/transaction_callbacks_test.rb
```
http://github.com/rails/rails/commit/1b2941cba1165b0721f57524645fe3...
You flagged this item as spam.
Brian Durand June 8th, 2010 @ 08:48 PM
Fixed. I'd mistakenly assumed the :on option was universal for all callbacks. The callbacks are now modeled after the validation call backs which support the :on option.
- after_trasaction_fix_on_callbacks.diff 4.5 KB
Repository June 8th, 2010 @ 10:05 PM
(from [b07073924002fd56ac5b63b24cb9318b3dee45c4]) Revert "Temporarily revert "Update after_commit and after_rollback docs and tests to use new style API with an :on options instead of on_* suffix." and "Add after_commit and after_rollback callbacks to ActiveRecord that are called after transactions either commit or rollback on all records saved or destroyed in the transaction.""

This reverts commit 1b2941cba1165b0721f57524645fe378bee2a950.

[#2991] http://github.com/rails/rails/commit/b07073924002fd56ac5b63b24cb931...
Repository June 8th, 2010 @ 10:05 PM
- State changed from “open” to “committed”
(from [2500e6af6634c984f7d2b567403de1c4c544ff91]) Make logic for after_commit and after_rollback :on option work like it does for validation callbacks.

[#2991 state:committed]

Signed-off-by: Jeremy Kemper jeremy@bitsweat.net
http://github.com/rails/rails/commit/2500e6af6634c984f7d2b567403de1...
Jeremy Kemper June 8th, 2010 @ 10:31 PM
Thanks Brian!
You flagged this item as spam.
Brian Durand June 18th, 2010 @ 11:06 PM
I found another bug with this code. The rollback code is resetting the id to its previous value. However, the attributes hash is frozen. The attached patch fixes the problem by unfreezing the attributes on rollback. This problem is a blocker if you use ActiveRecord outside of Rails and don't set the logger (that's how I found the bug).
- fix_rollback_frozen_attributes.diff 4.3 KB
Repository June 18th, 2010 @ 11:13 PM
(from [237165feb3e5f7b117b05353bd64d756b9f18f74]) Fix bug with rolling back frozen attributes.

[#2991]

Signed-off-by: Jeremy Kemper jeremy@bitsweat.net
http://github.com/rails/rails/commit/237165feb3e5f7b117b05353bd64d7...
You flagged this item as spam.
grosser August 28th, 2010 @ 04:30 PM
- Importance changed from “” to “Medium”
I wrapped Jeremy Kemper initial after_transaction 'hack' into a plugin(+specs), for all rails 2.x users and whoever still needs the insttant after_transaction in Rails 3

http://github.com/grosser/ar_after_transaction
Jeremy Kemper October 15th, 2010 @ 11:01 PM
- Milestone set to “3.0.2”
[bulk edit]
You flagged this item as spam.
ssupreme11 May 10th, 2011 @ 10:33 PM
Its my first time to visit this site and as I was exploring I cant believe that this site was made up of a very informative articles that you should try to have compliment with so as what I am doing now I really love to look forward with more interesting information on this site..

Regards,
Custom Dissertation
You flagged this item as spam.
csnk May 18th, 2011 @ 08:15 AM
I frequently need this and use a small monkeypatch plugin to do it. The only nit is some code style that doesn't match Rails, like the space in def foo (bar).clothing factory