[feature request] A less invasive "Suggestion" mode

[alfuken]

Sometimes, (rarely, but still), goldiloader preloads things that it shouldn't have.

Like, for example (actual code is not quite the same, though similar):

class Item < ApplicationRecord
  has_and_belongs_to_many :item_groups
end

class ItemGroup < ApplicationRecord
  has_and_belongs_to_many :items
end

class Bucket < ApplicationRecord
  belongs_to :item_group
  has_many :items, through: :item_group
end

There's millions of items, with thousands of groups and hundreds of buckets. When I'm calling @bucket.items.destroy_all, if at least one of the items belongs to any other group than the bucket's one - all hells break loose. Dozens of groups, tens, hundreds of thousands of items are being loaded from the database... And I've discovered this completely accidentally! Of course, one can disable automatic eager-loading on a case-by-case basis, but: how many more other similar places are in the code, when it loads things it shouldn't have?..

As I said, this setup is only a vague example — it might as well be some other associations or misconfigurations or shenanigans in the project, but, nevertheless — it would be nice to have an ability to run Goldiloader in a diagnostic/suggestions mode: instead of automagically autoloading things — print a message into the Rails log:

Goldiloader suggests eager-loading the `:item_group => :items` association for the Bucket class, called at `app/views/buckets/show.html.erb:18`

(with a config option to print the full backtrace, not just caller location)

That would be a most awesome addition. :)

[jturkel]

@alfuken - Thanks for filing this issue. Have you checked out the bullet gem which does exactly what you're describing?

[alfuken]

@alfuken - Thanks for filing this issue. Have you checked out the bullet gem which does exactly what you're describing?

Yeah, I sure did, the only problem is, Bullet author doesn't recomment using it in production environment, because bullet will make your app much slower on production. Despite warning I actually tried, and had to revert it in quite a hurry.
I'd assume it's because it does much more than just catching N+1's, but 🤷‍♂️

Goldiloader, on the other hand, didn't show any slowing-down effects (except false-positive loading), and that's exactly why I'm asking ;)

[alfuken]

Do I udnerstand correctly, that actual loading happens in Goldiloader::AssociationLoader#eager_load?

So if I add between lines 17-18 something like this, that'll do the trick?

if ENV["GOLDILOADER_SUGGESTION_MODE"].present?
  Rails.logger.info{ "[GOLDILOADER] would load #{models.inspect} for association #{association_name.inspect} here:"
  Rails.logger.info{ Rails.backtrace_cleaner.clean(caller) }
  return
end

[alfuken]

@jturkel ping 👋 :)

[jturkel]

The use case makes sense @alfuken. It will certainly catch fewer scenarios than bullet but unlike bullet, it can safely run in production. I'd be will to accept a PR if you want to make the change.

Goldiloader::AssociationLoader.load is the method that should be changed and any configuration should be done on Goldiloader similar to Goldiloader.globally_enabled.