ruby-hyperloop / hyper-mesh

The project has moved to Hyperstack!! - Synchronization of active record models across multiple clients using Pusher, ActionCable, or Polling
https://hyperstack.org/
MIT License
22 stars 12 forks source link

n+1 queries lacking includes eager loading #106

Open sfcgeorge opened 6 years ago

sfcgeorge commented 6 years ago

🤔 Recap of includes / n+1 queries

Hypermesh doesn't automatically preload / eager load / includes associations (whatever you want to call it). This means big fetches lead to hundreds of individual SELECT statements per ID instead of 1 SELECT for each model type with an IN (list, of, ids). Aka n+1 queries.

Rails is smart about selecting records nested 1 level:

Pipeline.find(27).stages
  Pipeline Load (0.6ms)  SELECT  "pipelines".* FROM "pipelines" WHERE "pipelines"."id" = $1 LIMIT $2  [["id", 27], ["LIMIT", 1]]
  Stage Load (0.3ms)  SELECT "stages".* FROM "stages" WHERE "stages"."pipeline_id" = $1  [["pipeline_id", 27]]

However if you add more nesting Rails does not smartly load the records, because everything is lazy loaded so until it starts loop by loop it has no way of knowing that you're going to load a nested association. Other ORMs that aren't lazy optimise this internally (Phoenix Ecto). Rails calls lazy loading a feature. Ecto calls not lazy loading a feature. Go figure.

Pipeline.find(27).stages.each do |s| 
  puts s.name

  s.recruitments.each do |r| 
    puts r.status
  end
end
  Pipeline Load (0.4ms)  SELECT  "pipelines".* FROM "pipelines" WHERE "pipelines"."id" = $1 LIMIT $2  [["id", 27], ["LIMIT", 1]]
  Stage Load (0.4ms)  SELECT "stages".* FROM "stages" WHERE "stages"."pipeline_id" = $1  [["pipeline_id", 27]]
  Recruitment Load (8.1ms)  SELECT "recruitments".* FROM "recruitments" INNER JOIN "recruitments_stages" ON "recruitments"."id" = "recruitments_stages"."recruitment_id" WHERE "recruitments_stages"."stage_id" = $1  [["stage_id", 153]]
  Recruitment Load (1.6ms)  SELECT "recruitments".* FROM "recruitments" INNER JOIN "recruitments_stages" ON "recruitments"."id" = "recruitments_stages"."recruitment_id" WHERE "recruitments_stages"."stage_id" = $1  [["stage_id", 154]]
  Recruitment Load (0.7ms)  SELECT "recruitments".* FROM "recruitments" INNER JOIN "recruitments_stages" ON "recruitments"."id" = "recruitments_stages"."recruitment_id" WHERE "recruitments_stages"."stage_id" = $1  [["stage_id", 155]]
  Recruitment Load (0.6ms)  SELECT "recruitments".* FROM "recruitments" INNER JOIN "recruitments_stages" ON "recruitments"."id" = "recruitments_stages"."recruitment_id" WHERE "recruitments_stages"."stage_id" = $1  [["stage_id", 156]]
  Recruitment Load (0.5ms)  SELECT "recruitments".* FROM "recruitments" INNER JOIN "recruitments_stages" ON "recruitments"."id" = "recruitments_stages"."recruitment_id" WHERE "recruitments_stages"."stage_id" = $1  [["stage_id", 157]]
  Recruitment Load (0.6ms)  SELECT "recruitments".* FROM "recruitments" INNER JOIN "recruitments_stages" ON "recruitments"."id" = "recruitments_stages"."recruitment_id" WHERE "recruitments_stages"."stage_id" = $1  [["stage_id", 158]]

In a production app with more records and more layers of records being fetched, this leads to hundreds of selects.

🔨 Manual fix

So what you do to fix this is use the includes() methods to tell Rails to not be lazy and load all those associations up front:

# note the added `includes` specifying what we're about to access
Pipeline.includes(stages: :recruitments).find(27).stages.each do |s| 
  puts s.name

  s.recruitments.each do |r| 
    puts r.status
  end
end
  Pipeline Load (0.4ms)  SELECT  "pipelines".* FROM "pipelines" WHERE "pipelines"."id" = $1 LIMIT $2  [["id", 27], ["LIMIT", 1]]
  Stage Load (0.6ms)  SELECT "stages".* FROM "stages" WHERE "stages"."pipeline_id" = $1  [["pipeline_id", 27]]
  RecruitmentsStage Load (0.7ms)  SELECT "recruitments_stages".* FROM "recruitments_stages" WHERE "recruitments_stages"."stage_id" IN ($1, $2, $3, $4, $5, $6)  [["stage_id", 153], ["stage_id", 154], ["stage_id", 155], ["stage_id", 156], ["stage_id", 157], ["stage_id", 158]]
  Recruitment Load (0.8ms)  SELECT "recruitments".* FROM "recruitments" WHERE "recruitments"."id" IN ($1, $2, $3, $4, $5, $6, $7, $8, $9)  [["id", 7], ["id", 1], ["id", 9], ["id", 4], ["id", 3], ["id", 6], ["id", 2], ["id", 5], ["id", 8]]

You can see we now only have a few queries leveraging IN. In a production app that was doing hundreds of SELECTs this reduces it to tens.

🤓 More Convenient Fix with Gotchas

You can even add it to a scope / association so that you can forget about it in your controllers / views / components!

class Pipeline < ApplicationRecord
  has_many(
    :stages_includes_all, lambda do
      includes(
        recruitments_stages: { recruitment: { candidate: :profile } }
      )
    end,
    class_name: "Stage", foreign_key: "pipeline_id",
    inverse_of: :pipeline_includes_all
  )
end

This would be great but it always applies. Aka if in one place you're loading a big tree of records and you add includes to the scope but somewhere else you use the same scope and you only want the top level, Rails will load the whole tree specified by includes anyway—you've forced it not to be lazy!

‼️ Hypermesh Problem

So what about Hypermesh? Hypermesh exhibits a similar behaviour in the nested case when done in a component, but its mechanism for loading records is completely different. It's not actually doing Pipeline.stages.recruitments as one big load tree the Rails way, it is generating lots of vectors and loading them separately (I think, I'm fuzzy on how / why the Hypermesh implementation works). Anyway, the result is hundreds of SELECTS which is terrible for performance.

🌀 Hypermesh Workaround

You can use a scope / association includes as above and make sure you always load stuff through that, perhaps naming it :hyperloop_stages or :stages_includes_all so that it's clear this scope will always includes a ton of stuff and you should only use it in this one place in your Hyperloop component. But it's obviously inconvenient, detracting from the amazing automaticity of Hypermesh, and easy to forget to add (or you have many separate components loading associations and it's hard to put them all under one query scope). You also have to be very careful about traversing the association the other way within components as you have to use the inverse of this custom association not the regular version otherwise you end up with yet more SELECTs.

I expected Hypermesh would automatically detect and eliminate n+1 queries because it specifically abstracts away the querying and actually makes it very difficult to manually optimize or even know it needs optimizing (I didn't notice for a good while until I looked at the logs and saw hundreds of queries ruining fetch performance).

👩🏼 Rails Automatic Fix with Goldiloader

There is a gem Goldiloader that automatically detects n+1s and eager loads them for you at run time. You literally just add the gem. It does it by letting everything happen as usual, but as soon as the 1st query in the loop occurs it can tell that looks like an n+1 and eager load so the rest of the loop is fine. So you get 2 queries instead of the 1 you'd get with includes by hand, but still better than 10s. There are cases it can't detect, and cases it will eager load when it shouldn't, so you can turn it off on specific scopes. But in most cases it should work fine.

I've tried it out and it does seem to work fine in our big brownfield app. The gem has been going for years, seems well supported. BUT it doesn't help with Hyperloop. Nothing breaks, but it actually results in more queries (not less!), and they're pretty weird looking. I assume it detects Hyperloop is loading stuff that looks like an n+1 but because Hyperloop doesn't actually have a tree / use the same objects, the eager loading has no effect and all the SELECTS still happen on top of the Goldiloader optimized SELECT.

👨🏻‍🏭 Hypermesh Solutions

I think the serverside part of Hypermesh should really be rewritten to eliminate n+1 queries. Because Hyperloop makes it so easy and magical to load whatever data you want throughout components, but currently makes it hard to see and optimize this n+1 problem. We have an opportunity to make Hypermesh magical AND performant.

I believe vectors in server_data_cache.rb will need to be re-processed into a tree structure, but given the current complexity I'm not sure. I did some puts debugging and only got so far.

🧙🏻‍♂️ Custom Eager Loading

We have all the vectors up front. I think they're not in a tree but we could assemble them into a tree, or not split them up in the first place. Then once we have a tree we should be able to walk it and add each layer to an includes at the top level object. We'd have to re-use the same objects I think so AR can tell what has been eager loaded already. Since we know all of the associations we want to load up front, we have this great window to automatically optimize the querying in a big way, like static analysis.

🧝🏻‍♀️ Tree + Goldiloader

This may be easier than rolling our own n+1 detection and eager loading. Or it may not. If it's easier then great, Goldiloader seems well supported and tested so it makes sense to leverage if it helps. I think Hypermesh would still have to convert its vectors into a more standard tree of associations so that Goldiloader can do its magic.