luckyframework / avram

A Crystal database wrapper for reading, writing, and migrating Postgres databases.
https://luckyframework.github.io/avram/
MIT License
165 stars 64 forks source link

Query cache issues with mosquito (and other single fiber code) #793

Open jwoertink opened 2 years ago

jwoertink commented 2 years ago

The query cache creates a new instance on each fiber. In the context of HTTP requests, that means each request gets its own query cache. However, Mosquito runs all of the jobs in the same fiber. This may be the case for sidekiq too 🤷‍♂️ In any case, this means that if you're running jobs less than 1.minute apart, each job after the first will pull from cache which could have some weird side affects.

For now, provided that you're booting your workers in a separate binary, you can just disable query cache for that.

require "./app"

# Disable query cache because all jobs run in the
# same fiber which means infinite cache
Avram.settings.query_cache_enabled = false

Mosquito::Runner.start

Or alternatively, you could temporarily disable cache on a specific worker:

def perform
  Avram.temp_config(query_cache_enabled: false) do
    # do job
  end
end

Something we need to consider long term is maybe a method that allows for a specific query to bypass cache. Maybe something that looks like...

UserQuery.new.without_cache do |query|
  query.email("some@email.com").unconfirmed(true)
end

which is nice, but to quote @wout

That would be great for specific queries, but not so much with validations.

Having cache in jobs isn't necessarily bad... we just have to make some considerations for how this can be handled. Another option which will fix mosquito specifically is that "hooks" are being implemented soon. A "before hook" would allow for starting each job with fresh cache.

wout commented 2 years ago

So you're quoting me now? :joy:

I'd like to add that a reverse bypass would be useful too. So, with cache disabled:

UserQuery.new.with_cache do |query|
  query.email("some@email.com").unconfirmed(true)
end

In the project I'm working on now, I decided to disable cache because most of the time I need uncached data. But sometimes I do need queries to be cached, and there's no way to achieve that right now. The closest thing would be memoizing a method.

matthewmcgarvey commented 2 years ago

Maybe to add another block to it...

Avram.with_cache do |cache|
  UserQuery.new.with_cache(cache) do |query|
    query.email(...)
  end
end
robacarp commented 2 years ago

Mosquito just released a patch increment which adds a before hook to job execution. At the very least this should provide a means to expire the query cache manually. I don't have a query-cache enabled environment at the moment but I believe adding this to config/mosquito.cr should do it:

abstract class Mosquito::Job
  macro inherited
    before do 
      Fiber.current.query_cache.flush
    end
  end
end