All saved objects kept until the end of the tests

Hi guys,

One of the odd things that's blocking one big app I work on from moving up to Rails 3.0 or 3.1 is that with these versions whenever an object is saved, a reference to it is retained until the end of the enclosing transaction block, so that its state (new_record?, etc.) can be rolled back if the transaction is rolled back. This is a nice feature, but because Ruby's WeakRef implementation is broken on 1.9, the ActiveRecord implementation uses regular strong references.

This is bearable in production code in most cases, but it's absolutely killing large test suites. When transactional fixtures is on, then the transaction in question is open for the length of the entire set of tests, so all objects ever saved at any point during the test cases are retained in memory.

Our test suite bloats up to 6GB before we give up and kill it - by that point the VM is doing almost nothing but garbage collecting. (It can run with transactional fixtures off, but that's very, very slow for an app with many tables.)

There are several ways we can fix this, and I thought it would be good to discuss on the list which is best because they will have different behavior. The options as I see them are:

1. Change the transaction code to not count the test transaction when determining whether to fire the commit/rollback hooks and discard the reference list. This is a bit of work, and will change the behavior of tests, but some might argue in a good way (if you want to test that your after_commit and after_rollback hooks work, pretending that you are not already inside a test transaction).

2. Explicitly throw away the reference list after each test completes, so that after_commit and after_rollback are not fired. A bit of a hack, but I imagine some will prefer it given what after_commit and after_rollback are typically used for.

3. Hassle someone to fix that WeakRef so that we can use it, making the issue largely moot (assuming you didn't care about those callbacks). The best long-term solution for other reasons, but difficult (1.9 needs patching). See http://redmine.ruby-lang.org/issues/4168.

If you want to see the problem yourself, just save some records (any model will do) in tests and watch them not get GCd:

class TestTest < ActiveSupport::TestCase
  self.use_transactional_fixtures = true

  1.upto(20) do |i|
    test "some test #{i}" do
      TestRecord.first.update_attributes!({})

      GC.start
      count = 0
      ObjectSpace.each_object(ActiveRecord::Base) do |b|
        count += 1
      end
      puts "#{count} ActiveRecord"
    end
  end
end

Hi Will,

One of the odd things that's blocking one big app I work on from
moving up to Rails 3.0 or 3.1 is that with these versions whenever an
object is saved, a reference to it is retained until the end of the
enclosing transaction block, so that its state (new_record?, etc.) can
be rolled back if the transaction is rolled back.

There are several ways we can fix this

What about adding a :consistent option when opening a transaction? E.g.

# Rolls back record state if the transaction fails
transaction do
  ...
end

# Doesn't roll back record state / keep references
transaction(:consistent => false) do
  ...
end

Then, we could make the transaction around test runs be non-consistent?

Jon

Consistent is possibly the wrong word to use there though, as it’s the C in ACID. I’m not sure I have a better suggestion though? :track_instance_state, :rollback_instances?

fwiw it’s not hard to construct cases where that per-instance rollback won’t work, so relying on it is pretty … brave. My hunch is that the most common use case for maintaining that state (rather than just redirecting the user somewhere else) is for rendering a form with the user’s data. I wonder if there’s a cleaner / simpler solution to be found?

Then, we could make the transaction around test runs be non-consistent?

Just bear in mind that the fixtures code does not use the #transaction method.

Consistent is possibly the wrong word to use there though, as it's the C in ACID. I'm not sure I have a better suggestion though? :track_instance_state, :rollback_instances?

:remember_state, I think.

Note though that all of these suggestions don't indicate whether or not after_commit and after_rollback will get called. If we go with one of them, then we will want to make sure that we remember objects that have after_commit or after_rollback events irrespective of this option - that seems like a reasonable idea to me though.

But the question still stands about whether they should be fired by tests. If no-one cares, I guess we can just leave it implementation specifics.

fwiw it's not hard to construct cases where that per-instance rollback won't work, so relying on it is pretty … brave.

Oh, really? I thought that the justification given for the new code that went in 3.0 that remembers all objects was that it could fix those cases. If it didn't really fix anything, I'd argue for ripping it out as it is an overhead relative to 2.3.

Could you give an example that's still not fixed?