Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[feature request] A less invasive "Suggestion" mode #153

Open
alfuken opened this issue Aug 1, 2024 · 3 comments
Open

[feature request] A less invasive "Suggestion" mode #153

alfuken opened this issue Aug 1, 2024 · 3 comments

Comments

@alfuken
Copy link

alfuken commented Aug 1, 2024

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
Copy link
Member

jturkel commented Aug 5, 2024

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

@alfuken
Copy link
Author

alfuken commented Oct 2, 2024

@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
Copy link
Author

alfuken commented Oct 2, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants