-
Notifications
You must be signed in to change notification settings - Fork 70
Description
I explained this issue in depth in #47. The short version is that ActiveRecord caching can lead to stale query results within an advisory lock, at a time when it's critical to be working with up-to-date data.
I'm thinking that it may make sense for the with_advisory_lock
block to automatically turn off caching via ActiveRecord::QueryCache.uncached
for the duration of the block. Or perhaps there should be an argument to with_advisory_lock
to control this.
The example use case is using an advisory lock to prevent the double-creation of a record. Take this for example:
def create_or_update_user(external_user)
transaction do
with_advisory_lock("create-user-#{external_user.id}", transaction: true) do
user = User.find_or_initialize_by(external_id: external_user.id)
user.update_info(external_user)
user.save!
end
end
end
In this example users are created from 3rd party webhooks, and the goal is to create a new user if it doesn't already exist, or update the existing one.
The problem I ran into is that upstream of the lock in my code I had a User.find_by(external_id: external_user.id)
. The user didn't exist at this point, so the result was being cached as nil
. Then inside the lock find_or_initialize_by
used the cached result of nil
, even though by this time the user record had already been created, thus defeating the purpose of the lock.
I solve this in my code by using uncached
to ensure a fresh query:
def create_or_update_user(external_user)
transaction do
with_advisory_lock("create-user-#{external_user.id}", transaction: true) do
self.class.uncached do
user = User.find_or_initialize_by(external_id: external_user.id)
user.update_info(external_user)
user.save!
end
end
end
end