This project is archived and is in readonly mode.
has_many collection(force_reload=TRUE) hits cache under lock!
Reported by ebriscoe | January 30th, 2009 @ 04:16 PM | in 2.x
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!:
-
"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.
-
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).
-
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.
Comments and changes to this ticket
-
Will Bryant December 14th, 2009 @ 04:43 AM
- 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.
-
Repository December 16th, 2009 @ 04:49 PM
- State changed from “new” to “resolved”
(from [b1bbf90dffbc412670286154d9c7749d4388806b]) When passing force_reload = true to an association, don't use the query cache [#1827 state:resolved]
Signed-off-by: Joshua Peek josh@joshpeek.com
http://github.com/rails/rails/commit/b1bbf90dffbc412670286154d9c774... -
Repository December 16th, 2009 @ 04:50 PM
(from [bf6af5f71917b5615edb5905729b22772133eea4]) When passing force_reload = true to an association, don't use the query cache [#1827 state:resolved]
Signed-off-by: Joshua Peek josh@joshpeek.com
http://github.com/rails/rails/commit/bf6af5f71917b5615edb5905729b22...
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
<h2 style="font-size: 14px">Tickets have moved to Github</h2>
The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>
People watching this ticket
ebriscoe
Attachments
Referenced by
-
1827
has_many collection(force_reload=TRUE) hits cache under lock!
(from [b1bbf90dffbc412670286154d9c7749d4388806b])
When pa...
-
1827
has_many collection(force_reload=TRUE) hits cache under lock!
(from [bf6af5f71917b5615edb5905729b22772133eea4])
When pa...