Shopify
Using cache when within a transaction if already loaded
I don't have the full context, but it is my understanding we do not want to send the cache backend destructive request while within a transaction for fear of corrupting with an operation that would never be commit on mysql.
We are hitting some code behaviours that are hard to diagnose and counter intuitive in active record callback code paths (those that imply transactions)
Here would be the current behaviour without these changes:
class Brand < ActiveRecord::Base
end
class Car < ActiveRecord::Base
include IdentityCache
belongs_to :brand
cache_belongs_to :brand
end
brand = Brand.create!(color: 'red')
car = Car.create!(brand: brand)
car.fetch_brand
# Attempt to load from cache, IDC miss then MySql
car.fetch_brand
# Attempt to load from cache, IDC hit
Car.transaction do
car.fetch_brand
end
# Attempt to load directly from MySql, even if IDC cache is present (IdentityCache.should_use_cache? is false)
This lead to extra mysql calls for no reasons.
A "worst" example would be where you want to load data from IDC to edit it.
brand = Brand.create!(color: 'red')
car = Car.create!(brand: brand)
car.fetch_brand.color = 'green'
puts car.fetch_brand.color
=> 'green'
Car.transaction do
puts car.fetch_brand.color
end
=> 'red'
It somewhat counter intuitive to "rollback" the cached relation to a state pre-transaction.
Is there any pitfalls to use the cache if already populated even within a transaction? Is there a proper work around?
Thoughts? @dylanahsmith @chrisdonaldson cc: @christianblais
I don't have the full context, but it is my understanding we do not want to send the cache backend destructive request while within a transaction for fear of corrupting with an operation that would never be commit on mysql.
No, after_commit avoids that problem.
Is there any pitfalls to use the cache if already populated even within a transaction? Is there a proper work around?
Yes, the whole point of this check to see if you are in a transaction is that we are trying to avoid using cached records as part of an operation that modifies the database, and the whole point of a transaction is to modify the database. E.g. if you read a record from the cache, modify it, then save it, you will have propagated any cache corruption to the database. Similarly, if you calculate an aggregate value from values in an association that was fetched from the cache, you will similarly have propagated cache corruption to the database.
As a workaround
- you can avoid IDC fetch methods if you know the result is only going to be used in a transaction
- you can move the IDC fetch calls into the transaction
- you could preload associations using active record, to make sure some code you are calling uses the active record loaded association
Perhaps we should also have a method like IdentityCache.with_cache(false) { ... } that could turn on identity cache around a block of code without opening a transaction.
A "worst" example would be where you want to load data from IDC to edit it.
Well that should never be a thing, for the reasons Dylan explains, I know is hard to avoid in the general case tho.
I think we should document this in the readme better; but the whole point was to make the library a noop inside explicit transactions and hook on every single transaction to follow the changes happening to the actual objects.
