Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

This project is archived and is in readonly mode.

has_many collection(force_reload=TRUE) hits cache under lock!

#1827

In a has_many association, collection(true) failed to execute a fresh SQL command, despite force_reload being set to true, and instead hit the cache, inside a transaction / lock!. The problem only occurred with concurrent requests and multiple mongrels, in the 2nd concurrence/lock!. Here's the simplified/offending code...

where as... Order has_many :order_items And :order_items has_many :quantities

Class Order

def update

Order.transaction do
  self.lock!
  order_item = self.order_items.first
  order_item.quantities(true).clear
  order_item.quantities << OrderItemQuantity(...stuff here...)
end

end end

The problem was that in the 2nd concurrence, "order_item.quantities(true).clear" was failing to delete the quantities created by the 1st concurrence. The solution was to replace:

order_item.quantities(true).clear

...with...

OrderItemQuantities.delete_all(["order_item_id = ?", order_item.id])

So here are some facts, in the 2nd instance/concurrence/lock!:

  1. "order_item.quantities(true).clear" was hitting the cache (according to the log), despite the (true) which (as far as I understand) should have forced a reload.

  2. The quantities cache was stale (returning quantities from before the 1st instance/concurrence/lock!. This caused the clear to do nothing (because the id's returned from quantities(true) no longer existed because of the 1st concurrence).

  3. OrderItemQuantities.all(["order_item_id = ?", order_item.id]), when inserted before the "order_item.quantities(true).clear", issued fresh SQL and returned the correct list of quantities (where "correct" means the quantities created by the 1st concurrence/lock!).

Using Rails 2.1.1, MySQL, storage engine is InnoDB for all tables.

Unless I misunderstand the documentation, order_item.quantities(true) should have issued fresh SQL rather than hitting the cache.

Reported by ebriscoe · January 30th, 2009 @ 04:16 PM

State: resolved
Milestone: 2.x
Assigned to: josh josh
Importance: none

Activity

  1. Will Bryant
    Will Bryant
    • Assigned user set to josh
    • Tag changed from 2.1.1, activerecord, collection, has_many, lock, transaction to 2.1.1, 2.3.x, activerecord, collection, has_many, lock, query_cache, transaction

    This affects one of my projects too - we're not using clear, but we are locking and using (true) to check for the current record - very bad thing happened when it used the cached result...

    Patch attached. Josh, does this look OK to you?

    It's surprising that this hasn't come up before - but from examining the projects I have access to it seems most times when people use (true) they've already dirtied the cache by executing a CUD operation on another instance, so the cache doesn't get used anyway. On a more positive note, that means that the performance impact of punching through the cache for force_reload=true is pretty minimal for most apps - they already don't cache most calls.

    This patch is against 2-3-stable.

    December 14th, 2009 @ 04:43 AM

  2. Repository
    Repository
    • State changed from new to resolved

    (from [b1bbf90dffbc412670286154d9c7749d4388806b]) When passing force_reload = true to an association, don't use the query cache [#1827 has_many collection(force_reload=TRUE) hits cache under lock! state:resolved]

    Signed-off-by: Joshua Peek josh@joshpeek.com
    http://github.com/rails/rails/commit/b1bbf90dffbc412670286154d9c774...

    December 16th, 2009 @ 04:49 PM

  3. Repository