magnusvk / counter_culture

Turbo-charged counter caches for your Rails app.
MIT License
1.96k stars 209 forks source link

Delay / Defer UPDATE call #299

Closed PhilCoggins closed 4 years ago

PhilCoggins commented 4 years ago

Is there a way to delay counter updates until a later time? I'm thinking in the case where records are inserted in bulk (like imports), it would be nice to wrap the inserts in a block that would prevent UPDATEs from occurring, and then when the process is complete I could just call counter_culture_fix_counts myself. Yes, I could use a bulk insert tool, but there are certain callbacks and validations that I still wish to run.

I have not seen anything in Readme and a quick look at the repo shows this isn't currently possible. Would this library be interested in having this functionality added?

magnusvk commented 4 years ago

This isn’t currently possible. My sense is this probably isn’t a super common use case, but if there’s a clean way to add this to the gem I would consider it.

If this is just a one-off process for you and you’re ok with a hackish solution, the easiest way to accomplish this is probably to empty the internal config array and then repopulate it when you’re done: https://github.com/magnusvk/counter_culture/blob/fcf5a0971cbb68142b696038c187dc80f93db909/lib/counter_culture/extensions.rb#L53

PhilCoggins commented 4 years ago

@magnusvk Thank you for the suggestion. Emptying @after_commit_counter_cache does not appear to be a threadsafe solution, so I'll do some digging, and open a PR if I can throw something reasonable together.

magnusvk commented 4 years ago

Yeah, that sounds right. If you want this to be thread-safe I’d imagine you need to modify the counter culture code and set a flag on the thread.

PhilCoggins commented 4 years ago

This is what I ended up with:

module CounterCulture
  module Extensions
    ClassMethods.module_eval do
      def defer_counter_culture_updates
        counter_culture_updates_was = Thread.current[:defer_counter_culture_updates]
        Thread.current[:defer_counter_culture_updates] = Array(counter_culture_updates_was) + [self]
        yield
      ensure
        Thread.current[:defer_counter_culture_updates] = counter_culture_updates_was
      end
    end
  end

  module DeferredUpdate
    private

    def _update_counts_after_create
      unless Array(Thread.current[:defer_counter_culture_updates]).include?(self.class)
        super
      end
    end

    # called by after_destroy callback
    def _update_counts_after_destroy
      unless Array(Thread.current[:defer_counter_culture_updates]).include?(self.class)
        super
      end
    end

    # called by after_update callback
    def _update_counts_after_update
      unless Array(Thread.current[:defer_counter_culture_updates]).include?(self.class)
        super
      end
    end
  end
end

ActiveSupport.on_load(:active_record) do
  include CounterCulture::DeferredUpdate
end

# example
Product.defer_counter_culture_updates { ... }

I was careful not to modify any many internals, and the Thread.current could probably be done better. I'd still love to clean this up and upstream this if you think it has a home in your library. Most likely I would just update the after_create / before_destroy / after_update declarations to utilize conditional methods.

magnusvk commented 4 years ago

Yeah, I mean... This looks pretty minimal. I'm not sure how widely useful this is but if it helps you to get this merged I'd say this looks small enough that I'd be happy to merge something along those lines.

ausangshukla commented 8 months ago

Is this merged into main? I have the exact same requirements. Further to confirm the code above works well, and Ive tested it in multiple areas of my production app. Would like to see it be part of main, if that is possible.

The use case is large number of imports of a model, with mutlple counters, causes many concurrent updates, which causes deadlocks.

magnusvk commented 8 months ago

It is, though you could answer your own question by clicking through to the linked PR just above your message. 😄