RFC: Turn off ActiveRecord caching within lock blocks

[jmanian]

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

[jmanian]

I also asked for input from the Rails team at rails/rails#43634. I thought they might be able to provide some guidance based on how ActiveRecord deals with this in row-level locking (if at all).

[lucasprag]

My 2 cents on this. I had the exact same problem as described in #47 and adding a ApplicationRecord.uncached block solved my problem as well. This was a really hard issue to solve, so for the sake of others not having this issue, I'm all in favor of the proposed solution above☝️ .

[muxcmux]

+1 for this

[mceachen]

(I'm too rusty with Rails at this point to give an opinion: this seems like an innocuous enough change, but either way, the documentation should be updated to highlight this issue. @seuros would you accept a PR with this change, if @jmanian or @lucasprag or @muxcmux submitted one?)

[seuros]

Yes.

[seuros]

Any news on this RFC ? I'm planning in releasing a new version pretty soon. I dropped old version of rails/ruby, so this implementation should be easier now.

[muxcmux]

@seuros I've rolled out a PR which adds that option to with_advisory_lock and updated the README, let me know if you have any comments/suggestions!