はじめに

こんにちは。ミツカリエンジニアのたなしゅんです。
Backendアプリケーションにおいて、N+1問題というのは基本のキでありながらパフォーマンス影響の大きい重要な問題です。
弊社ではBackendのAPIサーバにRuby on Railsを採用しています。
本記事ではRailsにおけるN+1問題を改善していく過程とその中で派生した問題について扱います。

検知用のgem

bulletというN+1問題を検出してくれる有名なライブラリがあるのでそちらを使用しています。 github.com

Bullet.raise = true

上記のように設定することで、Bulletによる検出があった場合にテストをエラーにすることができます。
しかし、既に稼働中のアプリケーションでいきなりこれを有効化するとたくさんのエラーが出ました。
有効化するまでの過程での試行錯誤を以下に残し、供養したいと思います。

例として用いる構造

これ以降Userが複数のPostを持つといったよくある例を使って解説します。

class User < ApplicationRecord
  has_many :posts, dependent: :destroy
end

class Post < ApplicationRecord
  belongs_to :user
end

最も基本的なN+1

def hoge
  users = User.all
  users.each do |user|
    user.posts.each do |post|
      puts post.title
    end
  end
end

この場合、ループの中で各Userのpostsを取得するSQLが発行されてしまうので、Userを取得する1回とpostsを取得するN回のクエリが実行されます。合計でN+1回のクエリが実行されるので、これをN+1問題と呼びます。クエリの発行はデータベースとの間の通信を必要とします。通信のオーバーヘッドはとても重たい処理です。そのため、Nの数が増えれば増えるほどこの処理は顕著に遅くなっていきます。 bulletを用いるとこのコードに対して以下のような検出をしてくれます。

USE eager loading detected
  User => [:posts]
  Add to your query: .includes([:posts])

修正方法ですが、以下のようにループの前にpostsも事前ロードしておくことでループ数がどれほど増えたとしてもusersとpostsのSelectクエリはそれぞれ1回の計2回に固定されます。

def hoge
  users = User.includes([:posts]).all
  users.each do |user|
    user.posts.each do |post|
      puts post.title
    end
  end
end

補足

正確にはincludesは内部的にpreloadとeager_loadをRails側が戦略的に選択するので、外部結合を用いた1回のクエリになる場合もあります。
3つのロードメソッドの違いについては以下の記事がとてもわかりやすいのでオススメです。 zenn.dev

これでN+1問題は無事解消されました。めでたしめでたし。

と、これだけで終われば非常にシンプルで特に記事にするようなものでもありませんでしたが、既存コードのtest環境でのbullet検出ゼロを目指す過程でいくつかの壁にぶち当たりました。

テストデータ作成時のN+1

RSpec.describe User do
  # FactoryBotを使ってテスト用のUserを作成
  let!(:user) { create(:user) }
end

Rspecでテストコードを書く際、テストデータの作成には上記のようにFactoryBotを利用しています。弊社のサービスではcreate時にhookで何かしらの操作が行われるような実装が多くあるのですが、FactoryBot経由でのcreateを前提にはしていないので、FactoryBotでcreateするとその部分でN+1が検出されるということが多くの箇所で見つかりました。もちろん、テストデータの作成時のN+1も修正すればCIでの実行時間が短くなるなどの恩恵は得られると思いますが、そもそもテストデータはパターン別に数種類しか用意しないため、Nの数が少なく、パフォーマンス面での影響は軽微と言えます。ということでテストデータの作成時のN+1は検出しないようにします。

RSpec.describe User do
  # FactoryBotを使ってテスト用のUserを作成
  let!(:user) {
    Bullet.enable = false
    user = create(:user)
    Bullet.enable = true
    user
  }
end

最初パッと思いついて実装したコードは上記のようなコードでしたが、万が一enableをfalseにしたままtrueに戻し忘れることがあると悲惨です。そこで以下のようなWrapperメソッドを作成しました。

class CustomBullet
  # 無効化したあとに有効化を忘れることを防ぐため、スコープ内でのみ無効化を行うメソッドを提供する
  def self.skip
    if defined?(Bullet) && Bullet.enable?
      # bulletがinstallされていて有効な場合のみ
      Bullet.enable = false
      yield
      Bullet.enable = true
    else
      yield
    end
  end
end

これを用いると以下のように書くことが出来ます。

RSpec.describe User do
  # FactoryBotを使ってテスト用のUserを作成
  let!(:user) {
    user = nil
    CustomBullet.skip do
      user = create(:user)
    end
    user
  }
end

enabledを戻し忘れるといったことがないコードになりました！しかし、先にも書いた通り、これがかなりの箇所で検出されています。1行1行すべてこの対応を入れていくのは指の骨が折れてしまいます。どうにかして一括して対応したいと考えた結果、以下のようなコードに行きつきました。

FactoryBot.define do
  # テストデータの作成時は一律N+1を検出しないようにする
  to_create do |instance|
    CustomBullet.skip { instance.save! }
  end
end

FactoryBotはグローバルな設定が可能でした。 thoughtbot.github.io

これを利用すると、FactoryBotでのcreate時にBulletの検出設定をオフにするというやりたいことをDRYに実現することができました。

単体テストだと不要なロードになってしまう問題

def getAllUsers
  User.includes([:posts]).all
end

def hoge
  users = getAllUsers
  users.each do |user|
    user.posts.each do |post|
      puts post.title
    end
  end
end

上記のgetAllUsersに対して単体テストを作成してみます。

RSpec.describe User do
  let!(:users) { create_list(:user, 3) }
  let!(:posts) { create_list(:post, 3, user: users.first) }
  
  it 'returns all users' do
    # 全Userを返しているか確認
    results = described_class.getAllUsers
    expect(results).to eq(users)
  end
end

この場合、getAllUsersの単体テストではincludesに指定したpostsを参照していないのでbulletから以下のようなRemoveの警告が出てしまいます。

AVOID eager loading detected
  User => [:posts]
  Remove from your query: .includes([:posts])

テストコード側で返却されたモデルのリレーションを参照するコードを書けば警告を回避することが出来ます。

RSpec.describe User do
  let!(:users) { create_list(:user, 3) }
  let!(:posts) { create_list(:post, 3, user: users.first) }
  
  it 'returns all users' do
    # 全Userを返しているか確認
    results = described_class.getAllUsers
    expect(results).to eq(users)
    expect(results.first.posts).to eq(posts) # 参照することで不要という警告はなくなる
  end
end

しかし、getAllUsersが新たにfugaメソッドからも呼び出されるようになるとどうでしょう。

def fuga
  users = getAllUsers
  users.each do |user|
    puts user.name
  end
end

fugaではpostsは参照されないまま処理が終わるため、fugaはテスト時だけではなく、実際にアプリ上での実行コードとしてもRemove警告が出ることになります。これらが今から開発されるもので、厳密に実装するのであれば以下のようにするのがベストだと思います。

# 事前ロードさせる関連は引数で指定する
def getAllUsers(association = [])
  User.includes(association).all
end

def hoge
  users = getAllUsers(association: [:posts])
  users.each do |user|
    user.posts.each do |post|
      puts post.title
    end
  end
end

def fuga
  users = getAllUsers
  users.each do |user|
    puts user.name
  end
end

呼び出し側で必要な箇所でのみ事前ロードさせるようなコードになり、どちらのメソッドでも過不足ない状態になりました。

しかし、本対応を既存コードすべてに対応するのは工数として重くなりすぎてしまい、現在のフェーズでリファクタリングにかけられる工数を大きくはみ出してしまうため、上記の書き方は今後の新規開発部分では活かすとして、既存コードに行うのは諦めました。代わりに、以下の設定コードを追加し、Remove警告自体を発生させないようにしました。

Bullet.unused_eager_loading_enable = false

Bulletには事前ロードの過不足の過と不足をそれぞれ有効無効を設定する機能があります。
unused_eager_loading_enableは未使用(過)なロードを警告する設定です。
もちろん、これにより本当に必要なRemove警告まで失われてしまうという問題はありますが、それは以下の理由から重要ではないと判断しました。

N+1が問題になるのはNが無限に増える場合であり、Nの個数が固定であれば固定値のパフォーマンスになるため、テストして問題なければ致命的な問題ではない。
不要なロードが1つ増えている場合、2つのクエリのうち1つが消せるというだけで、Nのような不定数ではないため予期せぬ悪パフォーマンスの原因にはならない。

上記のような試行錯誤を経て、無事Bulletをtest(CI)でraiseさせることができました。

最後に

このようにまだまだ技術的な改善課題も多く、一緒に改善してくれるITエンジニアを募集しています。興味のある方はぜひお気軽にご連絡ください！

herp.careers

ミツカリ技術ブログ

株式会社ミツカリの開発チームのブログです

RailsのRspecでN+1をエラー扱いにするまでの戦い